Re: [RESEND PATCH 1/2 -mm] mm: account lazy free pages separately

[Yang Shi <yang.shi@linux.alibaba.com>]

On 8/9/19 11:26 AM, Yang Shi wrote:
>
>
> On 8/9/19 11:02 AM, Michal Hocko wrote:
>> On Fri 09-08-19 09:19:13, Yang Shi wrote:
>>>
>>> On 8/9/19 1:32 AM, Michal Hocko wrote:
>>>> On Fri 09-08-19 07:57:44, Yang Shi wrote:
>>>>> When doing partial unmap to THP, the pages in the affected range 
>>>>> would
>>>>> be considered to be reclaimable when memory pressure comes in.  And,
>>>>> such pages would be put on deferred split queue and get minus from 
>>>>> the
>>>>> memory statistics (i.e. /proc/meminfo).
>>>>>
>>>>> For example, when doing THP split test, /proc/meminfo would show:
>>>>>
>>>>> Before put on lazy free list:
>>>>> MemTotal:       45288336 kB
>>>>> MemFree:        43281376 kB
>>>>> MemAvailable:   43254048 kB
>>>>> ...
>>>>> Active(anon):    1096296 kB
>>>>> Inactive(anon):     8372 kB
>>>>> ...
>>>>> AnonPages:       1096264 kB
>>>>> ...
>>>>> AnonHugePages:   1056768 kB
>>>>>
>>>>> After put on lazy free list:
>>>>> MemTotal:       45288336 kB
>>>>> MemFree:        43282612 kB
>>>>> MemAvailable:   43255284 kB
>>>>> ...
>>>>> Active(anon):    1094228 kB
>>>>> Inactive(anon):     8372 kB
>>>>> ...
>>>>> AnonPages:         49668 kB
>>>>> ...
>>>>> AnonHugePages:     10240 kB
>>>>>
>>>>> The THPs confusingly look disappeared although they are still on 
>>>>> LRU if
>>>>> you are not familair the tricks done by kernel.
>>>> Is this a fallout of the recent deferred freeing work?
>>> This series follows up the discussion happened when reviewing "Make 
>>> deferred
>>> split shrinker memcg aware".
>> OK, so it is a pre-existing problem. Thanks!
>>
>>> David Rientjes suggested deferred split THP should be accounted into
>>> available memory since they would be shrunk when memory pressure 
>>> comes in,
>>> just like MADV_FREE pages. For the discussion, please refer to:
>>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2010115.html 
>>>
>> Thanks for the reference.
>>
>>>>> Accounted the lazy free pages to NR_LAZYFREE, and show them in 
>>>>> meminfo
>>>>> and other places.  With the change the /proc/meminfo would look like:
>>>>> Before put on lazy free list:
>>>> The name is really confusing because I have thought of MADV_FREE 
>>>> immediately.
>>> Yes, I agree. We may use a more specific name, i.e. DeferredSplitTHP.
>>>
>>>>> +LazyFreePages: Cleanly freeable pages under memory pressure (i.e. 
>>>>> deferred
>>>>> +               split THP).
>>>> What does that mean actually? I have hard time imagine what cleanly
>>>> freeable pages mean.
>>> Like deferred split THP and MADV_FREE pages, they could be reclaimed 
>>> during
>>> memory pressure.
>>>
>>> If you just go with "DeferredSplitTHP", these ambiguity would go away.
>> I have to study the code some more but is there any reason why those
>> pages are not accounted as proper THPs anymore? Sure they are partially
>> unmaped but they are still THPs so why cannot we keep them accounted
>> like that. Having a new counter to reflect that sounds like papering
>> over the problem to me. But as I've said I might be missing something
>> important here.
>
> I think we could keep those pages accounted for NR_ANON_THPS since 
> they are still THP although they are unmapped as you mentioned if we 
> just want to fix the improper accounting.

By double checking what NR_ANON_THPS really means, 
Documentation/filesystems/proc.txt says "Non-file backed huge pages 
mapped into userspace page tables". Then it makes some sense to dec 
NR_ANON_THPS when removing rmap even though they are still THPs.

I don't think we would like to change the definition, if so a new 
counter may make more sense.

>
> Here the new counter is introduced for patch 2/2 to account deferred 
> split THPs into available memory since NR_ANON_THPS may contain 
> non-deferred split THPs.
>
> I could use an internal counter for deferred split THPs, but if it is 
> accounted by mod_node_page_state, why not just show it in 
> /proc/meminfo? Or we fix NR_ANON_THPS and show deferred split THPs in 
> /proc/meminfo?
>
>>
>