On Thu, Jun 02, 2016 at 02:04:42PM +0200, Vlastimil Babka wrote: > On 06/02/2016 12:39 PM, Mel Gorman wrote: > >On Wed, Jun 01, 2016 at 12:01:24PM +0200, Vlastimil Babka wrote: > >>>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. > >---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 <mgorman@xxxxxxxxxxxxxxxxxxx> > > Acked-by: Vlastimil Babka <vbabka@xxxxxxx> > Thanks. -- Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>