linux-kernel - Re: [PATCH v2 1/2] mm: alloc_pages_bulk_noprof: drop page

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: <84b5ad54-8765-4a38-a032-ca7fb9978c2d@redhat.com>
Date: Fri, 3 Jan 2025 09:12:52 -0500
From: Luiz Capitulino <luizcap@...hat.com>
To: Yunsheng Lin <linyunsheng@...wei.com>, linux-mm@...ck.org,
 mgorman@...hsingularity.net, willy@...radead.org
Cc: david@...hat.com, linux-kernel@...r.kernel.org, lcapitulino@...il.com
Subject: Re: [PATCH v2 1/2] mm: alloc_pages_bulk_noprof: drop page_list
 argument

On 2025-01-03 06:21, Yunsheng Lin wrote:
> On 2025/1/3 0:38, Luiz Capitulino wrote:
>> On 2024-12-25 07:36, Yunsheng Lin wrote:
>>> On 2024/12/24 6:00, Luiz Capitulino wrote:
>>>
>>>>    /*
>>>> - * __alloc_pages_bulk - Allocate a number of order-0 pages to a list or array
>>>> + * __alloc_pages_bulk - Allocate a number of order-0 pages to an array
>>>>     * @gfp: GFP flags for the allocation
>>>>     * @preferred_nid: The preferred NUMA node ID to allocate from
>>>>     * @nodemask: Set of nodes to allocate from, may be NULL
>>>> - * @nr_pages: The number of pages desired on the list or array
>>>> - * @page_list: Optional list to store the allocated pages
>>>> - * @page_array: Optional array to store the pages
>>>> + * @nr_pages: The number of pages desired in the array
>>>> + * @page_array: Array to store the pages
>>>>     *
>>>>     * This is a batched version of the page allocator that attempts to
>>>> - * allocate nr_pages quickly. Pages are added to page_list if page_list
>>>> - * is not NULL, otherwise it is assumed that the page_array is valid.
>>>> + * allocate nr_pages quickly. Pages are added to the page_array.
>>>>     *
>>>> - * For lists, nr_pages is the number of pages that should be allocated.
>>>> - *
>>>> - * For arrays, only NULL elements are populated with pages and nr_pages
>>>> + * Note that only NULL elements are populated with pages and nr_pages
>>>
>>> It is not really related to this patch, but while we are at this, the above
>>> seems like an odd behavior. By roughly looking at all the callers of that
>>> API, it seems like only the below callers rely on that?
>>> fs/erofs/zutil.c: z_erofs_gbuf_growsize()
>>> fs/xfs/xfs_buf.c: xfs_buf_alloc_pages()
>>>
>>> It seems it is quite straight forward to change the above callers to not
>>> rely on the above behavior, and we might be able to avoid more checking
>>> by removing the above behavior?
>>
>> Hi Yunsheng,
>>
>> Assuming the use-case is valid, I think we might want to keep common code
>> in the API vs. duplicating it in callers?
> 
> I was thinking maybe adding a wrapper/helper around the __alloc_pages_bulk()
> to avoid the overhead for other usecase and make the semantics more obvious
> if it is an valid use-case.

Yeah, that might be a good idea.

> 
>>
>> In any case, even if we decide to go for your suggestion, I'd prefer not
>> to grow the scope of this series since this could delay its inclusion.
>> Dropping the list-API (and dead code) is actually important so IMHO it
>> should go in first.
> 
> Sure.
> 
>>
>