netdev - Re: [PATCH v2] mm: alloc_pages_bulk: remove assumption of populating only NULL elements

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: <7abb0e8c-f565-48f0-a393-8dabbabc3fe2@gmail.com>
Date: Sun, 9 Mar 2025 21:23:35 +0800
From: Yunsheng Lin <yunshenglin0825@...il.com>
To: NeilBrown <neilb@...e.de>, Yunsheng Lin <linyunsheng@...wei.com>
Cc: Qu Wenruo <wqu@...e.com>, Yishai Hadas <yishaih@...dia.com>,
 Jason Gunthorpe <jgg@...pe.ca>,
 Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>,
 Kevin Tian <kevin.tian@...el.com>,
 Alex Williamson <alex.williamson@...hat.com>, Chris Mason <clm@...com>,
 Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>,
 Gao Xiang <xiang@...nel.org>, Chao Yu <chao@...nel.org>,
 Yue Hu <zbestahu@...il.com>, Jeffle Xu <jefflexu@...ux.alibaba.com>,
 Sandeep Dhavale <dhavale@...gle.com>, Carlos Maiolino <cem@...nel.org>,
 "Darrick J. Wong" <djwong@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 Ilias Apalodimas <ilias.apalodimas@...aro.org>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Trond Myklebust <trondmy@...nel.org>,
 Anna Schumaker <anna@...nel.org>, Chuck Lever <chuck.lever@...cle.com>,
 Jeff Layton <jlayton@...nel.org>, Olga Kornievskaia <okorniev@...hat.com>,
 Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
 Luiz Capitulino <luizcap@...hat.com>,
 Mel Gorman <mgorman@...hsingularity.net>, Dave Chinner
 <david@...morbit.com>, kvm@...r.kernel.org, virtualization@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-btrfs@...r.kernel.org,
 linux-erofs@...ts.ozlabs.org, linux-xfs@...r.kernel.org, linux-mm@...ck.org,
 netdev@...r.kernel.org, linux-nfs@...r.kernel.org
Subject: Re: [PATCH v2] mm: alloc_pages_bulk: remove assumption of populating
 only NULL elements

On 3/8/2025 5:02 AM, NeilBrown wrote:

...

>>
>>>    allocated pages in the array - just like the current
>>>    alloc_pages_bulk().
>>
>> I guess 'the total number of allocated pages in the array ' include
>> the pages which are already in the array before calling the above
>> API?
> 
> Yes - just what the current function does.
> Though I don't know that we really need that detail.
> I think there are three interesting return values:
> 
> - hard failure - don't bother trying again soon:   maybe -ENOMEM
> - success - all pages are allocated:  maybe 0 (or 1?)
> - partial success - at least one page allocated, ok to try again
>    immediately - maybe -EAGAIN (or 0).

Yes, the above makes sense. And I guess returning '-ENOMEM' & '0' &
'-EAGAIN' seems like a more explicit value.

> 
>>

...

>>
> 
> If I were do work on this (and I'm not, so you don't have to follow my
> ideas) I would separate the bulk_alloc into several inline functions and
> combine them into the different interfaces that you want.  This will
> result in duplicated object code without duplicated source code.  The
> object code should be optimal.

Thanks for the detailed suggestion, it seems feasible.
If the 'add to a linked list' dispose was not removed in the [1],
I guess it is worth trying.
But I am not sure if it is still worth it at the cost of the above
mentioned 'duplicated object code' considering the array defragmenting
seem to be able to unify the dispose of 'add to end of array' and
'add to next hole in array'.

I guess I can try with the easier one using array defragmenting first,
and try below if there is more complicated use case.

1. 
https://lore.kernel.org/all/f1c75db91d08cafd211eca6a3b199b629d4ffe16.1734991165.git.luizcap@redhat.com/

> 
> The parts of the function are:
>   - validity checks - fallback to single page allocation
>   - select zone - fallback to single page allocation
>   - allocate multiple pages in the zone and dispose of them
>   - allocate a single page
> 
> The "dispose of them" is one of
>    - add to a linked list
>    - add to end of array
>    - add to next hole in array
> 
> These three could be inline functions that the "allocate multiple pages"
> and "allocate single page" functions call.  We can pass these as
> function arguments and the compile will inline them.
> I imagine these little function would take one page and return
> a bool indicating if any more are wanted.
> 
> The three functions: alloc_bulk_array alloc_bulk_list
> alloc_bulk_refill_array would each look like:
> 
>    validity checks: do we need to allocate anything?
> 
>    if want more than one page &&
>       am allowed to do mulipage (e.g. not __GFP_ACCOUNT) &&
>       zone = choose_zone() {
>          alloc_multi_from_zone(zone, dispose_function)
>    }
>    if nothing allocated
>       alloc_single_page(dispose_function)
> 
> Each would have a different dispose_function and the initial checks
> would be quite different, as would the return value.
> 
> Thanks for working on this.
> 
> NeilBrown
>