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: <0edbfcf5-db26-441f-ae9c-f85985cb8b68@huawei.com>
Date: Fri, 3 Jan 2025 19:21:58 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Luiz Capitulino <luizcap@...hat.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/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.

> 
> 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.

>