Re: [PATCH] mm, thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings

From: "Zi Yan" <zi.yan@cs.rutgers.edu>
To: Michal Hocko <mhocko@suse.com>, Vlastimil Babka <vbabka@suse.cz>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Alex Williamson <alex.williamson@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Stefan Priebe - Profihost AG <s.priebe@profihost.ag>
Subject: Re: [PATCH] mm, thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings
Date: Thu, 06 Sep 2018 08:35:22 -0400	[thread overview]
Message-ID: <87733522-2EFD-4A17-AE8B-6DABE8B9BFAB@cs.rutgers.edu> (raw)
In-Reply-To: <20180906112546.GP14951@dhcp22.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

On 6 Sep 2018, at 7:25, Michal Hocko wrote:

> On Thu 06-09-18 13:16:00, Vlastimil Babka wrote:
>> On 09/06/2018 01:10 PM, Vlastimil Babka wrote:
>>>> We can and should think about this much more but I would like to have
>>>> this regression closed. So can we address GFP_THISNODE part first and
>>>> build more complex solution on top?
>>>>
>>>> Is there any objection to my patch which does the similar thing to your
>>>> patch v2 in a different location?
>>>
>>> Similar but not the same. It fixes the madvise case, but I wonder about
>>> the no-madvise defrag=defer case, where Zi Yan reports it still causes
>>> swapping.
>>
>> Ah, but that should be the same with Andrea's variant 2) patch. There
>> should only be difference with defrag=always, which is direct reclaim
>> with __GFP_NORETRY, Andrea's patch would drop __GFP_THISNODE and your
>> not. Maybe Zi Yan can do the same kind of tests with Andrea's patch [1]
>> to confirm?
>
> Yes, that is the only difference and that is why I've said those patches
> are mostly similar. I do not want to touch defrag=always case because
> this one has always been stall prone and we have replaced it as a
> default just because of that. We should discuss what should be done with
> that case separately IMHO.

Vlastimil, my test using Andrea’s patch confirms your statement.
My test result of Andrea’s patch shows that it gives the same outcomes as
Michal’s patch except that when no madvise is used, THP is on by default
+ defrag = {always}, instead of swapping pages to disk, Adndrea’s patch
causes no swapping and THPs are allocated in the fallback node.

As I said before, the fundamental issue that causes swapping pages to disk
when allocating THPs in a filled node is __GFP_THISNODE removes all fallback
zone/node options, thus,  __GFP_KSWAPD_RECLAIM or __GFP_DIRECT_RECLAIM
can only swap pages out to satisfy the THP allocation request.

__GFP_THISNODE can be seen as a kernel-version MPOL_BIND policy, which
overwrites any user space memory policy and should be removed or limited
to kernel-only page allocations. But, as Michal said, we could discuss
this further but do not make this discussion on the critical path of merging
the patch.

—
Best Regards,
Yan Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]