linux-kernel - Re: [PATCH hotfix 1/2] mm/thp: fix deferred split queue not partially

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 for Android: free password hash cracker in your pocket

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

Message-ID: <947f23e2-c817-b714-57d7-c893aad5002f@google.com>
Date: Sat, 26 Oct 2024 21:43:09 -0700 (PDT)
From: Hugh Dickins <hughd@...gle.com>
To: Zi Yan <ziy@...dia.com>
cc: Hugh Dickins <hughd@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, 
    Usama Arif <usamaarif642@...il.com>, Yang Shi <shy828301@...il.com>, 
    Wei Yang <richard.weiyang@...il.com>, 
    "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>, 
    Matthew Wilcox <willy@...radead.org>, David Hildenbrand <david@...hat.com>, 
    Johannes Weiner <hannes@...xchg.org>, 
    Baolin Wang <baolin.wang@...ux.alibaba.com>, 
    Barry Song <baohua@...nel.org>, Kefeng Wang <wangkefeng.wang@...wei.com>, 
    Ryan Roberts <ryan.roberts@....com>, Nhat Pham <nphamcs@...il.com>, 
    Chris Li <chrisl@...nel.org>, linux-kernel@...r.kernel.org, 
    linux-mm@...ck.org
Subject: Re: [PATCH hotfix 1/2] mm/thp: fix deferred split queue not
 partially_mapped

On Fri, 25 Oct 2024, Zi Yan wrote:
> 
> Thank you a lot for taking the time to check the locked version. Looking
> forward to the result.

The locked version worked fine (I did see some unusual filesystem on loop
errors on this laptop, but very much doubt that was related to the extra
deferred split queue locking).  But I do still prefer the original version.

> BTW, I am not going to block this patch since it fixes the bug.

Thanks!  I'll put out the same as v2 1/2.

> 
> The tricky part in deferred_list_scan() is always the use of
> folio->_deferred_list without taking split_queue_lock. I am thinking about
> use folio_batch to store the out-of-split_queue folios, so that _deferred_list
> will not be touched when these folios are tried to be split. Basically,
> 
> 1. loop through split_queue and move folios to a folio_batch until the
>    folio_batch is full;
> 2. loop through the folio_batch to try to split each folio;
> 3. move the remaining folios back to split_queue.
> 
> With this approach, split_queue_lock might be taken more if there are
> more than 31 (folio_batch max size) folios on split_queue and split_queue_lock
> will be held longer in step 3, since the remaining folios need to be
> added back to split_queue one by one instead of a single list splice.
> 
> Let me know your thoughts. I can look into this if this approach sounds
> promising. Thanks.

TBH, I just don't see the point: we have a working mechanism, and
complicating it to rely on more infrastructure, just to reach the
same endpoint, seems a waste of developer time to me.  It's not as
if a folio_batch there would make the split handling more efficient.

It would disambiguate the list_empty(), sure; but as it stands,
there's nothing in the handling which needs that disambiguated.

I can half-imagine that a folio_batch might become a better technique,
if we go on to need less restricted use of the deferred split queue:
if for some reason we grow to want to unqueue a THP while it's still
in use (as memcg move wanted in 2/2, but was not worth recoding for).

But I'm not sure whether a folio_batch would actually help there,
and I wouldn't worry about it unless there's a need.

Hugh