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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190812093430.GD5117@dhcp22.suse.cz>
Date:   Mon, 12 Aug 2019 11:34:30 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Yang Shi <yang.shi@...ux.alibaba.com>
Cc:     kirill.shutemov@...ux.intel.com, hannes@...xchg.org,
        vbabka@...e.cz, rientjes@...gle.com, akpm@...ux-foundation.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH 1/2 -mm] mm: account lazy free pages separately

On Fri 09-08-19 16:54:43, Yang Shi wrote:
> 
> 
> On 8/9/19 11:26 AM, Yang Shi wrote:
> > 
> > 
> > On 8/9/19 11:02 AM, Michal Hocko wrote:
[...]
> > > 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.

Yes, changing NR_ANON_THPS semantic sounds like a bad idea. Let
me try whether I understand the problem. So we have some THP in
limbo waiting for them to be split and unmapped parts to be freed,
right? I can see that page_remove_anon_compound_rmap does correctly
decrement NR_ANON_MAPPED for sub pages that are no longer mapped by
anybody. LRU pages seem to be accounted properly as well.  As you've
said NR_ANON_THPS reflects the number of THPs mapped and that should be
reflecting the reality already IIUC.

So the only problem seems to be that deferred THP might aggregate a lot
of immediately freeable memory (if none of the subpages are mapped) and
that can confuse MemAvailable because it doesn't know about the fact.
Has an skewed counter resulted in a user observable behavior/failures?
I can see that memcg rss size was the primary problem David was looking
at. But MemAvailable will not help with that, right? Moreover is
accounting the full THP correct? What if subpages are still mapped?

-- 
Michal Hocko
SUSE Labs