On Thu, Jun 02, 2016 at 02:04:42PM +0200, Vlastimil Babka wrote:

On 06/02/2016 12:39 PM, Mel Gorman wrote:

quoted

On Wed, Jun 01, 2016 at 12:01:24PM +0200, Vlastimil Babka wrote:

quoted

quoted

Why?

The comment is fine but I do not see why the recalculation would occur.

In the original code, the preferred_zoneref for statistics is calculated
based on either the supplied nodemask or cpuset_current_mems_allowed during
the initial attempt. It then relies on the cpuset checks in the slowpath
to encorce mems_allowed but the preferred zone doesn't change.

With your proposed change, it's possible that the
preferred_zoneref recalculation points to a zoneref disallowed by
cpuset_current_mems_sllowed. While it'll be skipped during allocation,
the statistics will still be against a zone that is potentially outside
what is allowed.

Hmm that's true and I was ready to agree. But then I noticed  that
gfp_to_alloc_flags() can mask out ALLOC_CPUSET for GFP_ATOMIC. So it's
like a lighter version of the ALLOC_NO_WATERMARKS situation. In that
case it's wrong if we leave ac->preferred_zoneref at a position that has
skipped some zones due to mempolicies?

So both options are wrong then. How about this?

I wonder if the original patch we're fixing was worth all this trouble (and
more
for my compaction priority series :), but yeah this should work.

I considered that option when the bug report first came in. It was a 2%
hit to the page allocator microbenchmark to revert it. More than I expected
but enough to care. If this causes another problem, I'll revert it as
there will be other options later.

quoted

---8<---
mm, page_alloc: Recalculate the preferred zoneref if the context can ignore memory policies

The optimistic fast path may use cpuset_current_mems_allowed instead of
of a NULL nodemask supplied by the caller for cpuset allocations. The
preferred zone is calculated on this basis for statistic purposes and
as a starting point in the zonelist iterator.

However, if the context can ignore memory policies due to being atomic or
being able to ignore watermarks then the starting point in the zonelist
iterator is no longer correct. This patch resets the zonelist iterator in
the allocator slowpath if the context can ignore memory policies. This will
alter the zone used for statistics but only after it is known that it makes
sense for that context. Resetting it before entering the slowpath would
potentially allow an ALLOC_CPUSET allocation to be accounted for against
the wrong zone. Note that while nodemask is not explicitly set to the
original nodemask, it would only have been overwritten if cpuset_enabled()
and it was reset before the slowpath was entered.

Signed-off-by: Mel Gorman <redacted>

Acked-by: Vlastimil Babka <redacted>

Thanks.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>