From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71F18E77188 for ; Fri, 3 Jan 2025 14:27:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E4A4F6B007B; Fri, 3 Jan 2025 09:27:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DFA036B0082; Fri, 3 Jan 2025 09:27:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CC1976B0083; Fri, 3 Jan 2025 09:27:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id AD0F56B007B for ; Fri, 3 Jan 2025 09:27:08 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3C6A9160854 for ; Fri, 3 Jan 2025 14:27:08 +0000 (UTC) X-FDA: 82966366098.30.CC95E7A Received: from mail43.out.titan.email (mail43.out.titan.email [18.196.231.60]) by imf13.hostedemail.com (Postfix) with ESMTP id EAFAE20010 for ; Fri, 3 Jan 2025 14:26:15 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=techsingularity.net header.s=titan1 header.b=dI9WfIaR; dmarc=none; spf=pass (imf13.hostedemail.com: domain of mgorman@techsingularity.net designates 18.196.231.60 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1735914377; a=rsa-sha256; cv=none; b=41FGAOhtdiMAvcuH5nyS2LVA6AdL7AnvT3zuQ+/mHrciAIlCfgZjB4oYlz7iUhX85WwgV0 nMbTVEiqqmrrXbMJHiyjBN2a8I2q6Dm1g5olmsdpwTT1S80phPXdt9C+UBYXm1kLbeIIjX j/BHOfvBPP3A+oDy3iPBmu1Qo+GXdwY= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=techsingularity.net header.s=titan1 header.b=dI9WfIaR; dmarc=none; spf=pass (imf13.hostedemail.com: domain of mgorman@techsingularity.net designates 18.196.231.60 as permitted sender) smtp.mailfrom=mgorman@techsingularity.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1735914377; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=W/S14W5TiJ8BOYqnWxSP7vfT/OzkIkh6ZOhhyeEwrag=; b=iRXcG7ZYIPSwyqQCijm7HQu7sm47yWTkLlzP06rd3CGc8wDdpmyG0rrq3O0Gu0d/mxzjVn exnLC51+tucKJncjNPEAGzJZwY/1DJbDXAXK/f20q25nCcof0NzWHWFL/Gpaw+Gw+BUkjO vC1cL3HweQArEEh9oN48FKiwsRxs938= Received: from smtp-out0101.titan.email (localhost [127.0.0.1]) by smtp-out0101.titan.email (Postfix) with ESMTP id A3C59100020; Fri, 3 Jan 2025 14:27:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=W/S14W5TiJ8BOYqnWxSP7vfT/OzkIkh6ZOhhyeEwrag=; c=relaxed/relaxed; d=techsingularity.net; h=message-id:references:to:cc:subject:mime-version:in-reply-to:date:from:from:to:cc:subject:date:message-id:in-reply-to:references:reply-to; q=dns/txt; s=titan1; t=1735914423; v=1; b=dI9WfIaR2u+F4IQMQeT779bUFP+thZ1KXOTZj5EzNWe5dzTRArwHq3TViW5I7LTv7harqk7D 5aKUSr2jovKd6TggTJYYStJGVccq/RHa+AHvQ6+7kA8nDmkaPmtn423OJExV9ttfNSUiRYev5lz nN8OBASHDNKUvSTMOOmSi2v4= Received: from mail.blacknight.com (ip-84-203-196-66.broadband.digiweb.ie [84.203.196.66]) by smtp-out0101.titan.email (Postfix) with ESMTPA id 0BBEB100051; Fri, 3 Jan 2025 14:27:02 +0000 (UTC) Date: Fri, 3 Jan 2025 14:27:02 +0000 Feedback-ID: :mgorman@techsingularity.net:techsingularity.net:flockmailId From: Mel Gorman To: Yunsheng Lin Cc: Luiz Capitulino , linux-mm@kvack.org, willy@infradead.org, david@redhat.com, linux-kernel@vger.kernel.org, lcapitulino@gmail.com Subject: Re: [PATCH v2 1/2] mm: alloc_pages_bulk_noprof: drop page_list argument Message-ID: References: <6b56af9b-2670-4a15-84e8-314443ae590c@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <6b56af9b-2670-4a15-84e8-314443ae590c@huawei.com> X-F-Verdict: SPFVALID X-Titan-Src-Out: 1735914423542067052.14999.5811168919653270471@prod-euc1-smtp-out1001. X-Virus-Scanned: ClamAV using ClamSMTP X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: EAFAE20010 X-Stat-Signature: 4ek63anys5wsxdc3o7sxm18ijqxyw4uf X-Rspam-User: X-CMAE-Score: 0 X-CMAE-Analysis: v=2.4 cv=Co25cG4D c=1 sm=1 tr=0 ts=6777f3ba a=YP3kMIKohITrmHkbULZXRA==:117 a=jU4EnjUUC1PH4wSjvv7Pww==:17 a=Q9fys5e9bTEA:10 a=VdSt8ZQiCzkA:10 a=CEWIc4RMnpUA:10 a=CDkG8U_cDq8A:10 a=wFHMvWF0vP_0gFGfqAsA:9 a=PUjeQqilurYA:10 X-HE-Tag: 1735914375-107944 X-HE-Meta: U2FsdGVkX1+YAJMuzHitORqwldaDZaSRkPRyFnsS5G/M1yBhdLyvik8XSQTKhWxuZT4x1hAry8Y5Yq7S4CbvXOm/Z3T73JxVtb06ljqLwGMyeD5W8Op1JXnSVpzXYDY7/cok/d4ZIVvATyiJv3IzkJG11kpCNx1IVIViYubJElWUm/Bk20t6WVdGuqAKSIwc3TK35lByHNSmMCXQRJGkzgfMcznbUii/U4HEmPjXs8VmMsLNuaewbhJlwntjvygKH5/BHjofwkuBsNoZq8oKNEFOL+F7hmKBruGy61XOFLaw4KsLJqeSTOiWRl2tiAo+0oeLFv+dYKlHJN9G6AHHmoGpUoISNz+PWnhQFgPMe05ge/hCBPInZOS9g1m9NJ2ythnRHjgcknrcLMg7zAv3eeQ761VHCsd6eGgha6jPZm4uEE/384dm94kJ0CMVoqTvDmc6kRFM1B8KEraoA5dWQQ0Sg5II3pz53fpeCvvYcjkNit276QLeXiRGE8WjYnix9GIFIngCPLvgRWYOBxl3BGOiYAqltz2z7TJr9n7DGVy6nfchAXzAw1A7iXbRVseFtqb4BN/FzY6bo3EYYtLhsK6ZkBBZ98Emd7GB9HeSRZB6xm1T0sXb65za9Ueqc55ccXFgn0voXHN7HaKBqFOCoX4je9pMn0K4PGnvS3oh03cDFriCWpVYqFVBOzP1sikb93sNPhthtO8hx7jrk3cycmDpPQJVTO1eVKgRkCqSr2jmLQ6JPRqKYqsiegKxowuat9yW/w1Q3SOuvgbbFyYMlteIALLEBoJNc06qdowtwtrMWm180WPDK/Fo2ozRZ6cvUEX9sKSkObfp9bWklc2+twMrRv8Ah3vlwif9N4wH5/TOe+4Xf2BmdXkPxo8WvfYiQGza3Y99KpaZ4Ii/7gJsLTYncEh3KpI8tJ0iongTja64qArSsGa//FERlgcHGgLb90u7RCiFqMLryzduJHK yIhE0zbE jzBx6Vg0+6jIU0tMzQB9Q70KvuTgthLR7GkkK3I7FGJZQvF5wgHiXLMLJHn0w+ewO+mUZuFKZNV19klE967X46k4fjChtEpwD/7AEVeVuzKycYOkv8gtEG5ii2/f815ZbO2wQNetmDhzld4emyyoDcfwAa7qSnIdJYoXMyuI3EE5rEeYFay6KYk6XGeluzqd9hVBKZXQDjSVyVKHljPzbjvzVfwLD7Y3sBGm2sXWatSadJEI44BqKJEijBaxjLPE++DAgMCdRDjUZuu5ASnN8rR1ACIXtTGMjP27J4pBhYJQDChG/Fyzu67/VXjqimLHEVVXYJ09mIAyHX3ZKumad9V1BUGSuMlpE96JQR5U0IbqeHwJheXFTbee49REKJIP40iszJLTZIXwOALR4Zya2m12n76//viRzpNw577TgsqTKFGxwJ3KUF8vdSqSlb3MEIQ+v0zeR3gZZH2210ltYjTtm/fEYpFsi81S8+UYJCLmNqn3OSfpigRKQQJdj7fLSVe6H7j+Fidxhwb0rOW2H+x+qi5JGkzlLIYfgOy8tpiISFLOp6dJY2E92BFUCbHxkailDr0GImVdW3UUocQfOiW/YUrQIhTTajp1nd5fuesR80YxKlpRGpHLRXX0+Js6S7XcrtOBrVlKXKtj0b07bW3RbNSStPnj16B1YSXqkT0olyjDWsDnhXxFscA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Jan 03, 2025 at 07:29:30PM +0800, Yunsheng Lin wrote: > On 2025/1/3 4:00, Mel Gorman wrote: > > On Wed, Dec 25, 2024 at 08:36:04PM +0800, 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? > >> > > > > It was implemented that way for an early user, net/sunrpc/svc_xprt.c. > > The behaviour removes a burden from the caller to track the number of > > populated elements and then pass the exact number of pages that must be > > allocated. If the API does not handle that detail, each caller needs > > similar state tracking implementations. As the overhead is going to be > > the same whether the API implements it once or each caller implements > > there own, it is simplier if there is just one implementation. > > It seems it is quite straight forward to change the above use case to > not rely on that by something like below? > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > index 43c57124de52..52800bfddc86 100644 > --- a/net/sunrpc/svc_xprt.c > +++ b/net/sunrpc/svc_xprt.c > @@ -670,19 +670,21 @@ static bool svc_alloc_arg(struct svc_rqst *rqstp) > pages = RPCSVC_MAXPAGES; > } > > - for (filled = 0; filled < pages; filled = ret) { > - ret = alloc_pages_bulk_array(GFP_KERNEL, pages, > - rqstp->rq_pages); > - if (ret > filled) > + for (filled = 0; filled < pages;) { > + ret = alloc_pages_bulk_array(GFP_KERNEL, pages - filled, > + rqstp->rq_pages + filled); > + if (ret) { > + filled += ret; > /* Made progress, don't sleep yet */ > continue; > + } > > set_current_state(TASK_IDLE); > if (svc_thread_should_stop(rqstp)) { > set_current_state(TASK_RUNNING); > return false; > } > - trace_svc_alloc_arg_err(pages, ret); > + trace_svc_alloc_arg_err(pages, filled); > memalloc_retry_wait(GFP_KERNEL); > } > rqstp->rq_page_end = &rqstp->rq_pages[pages]; > The API implementation would also need to change to make this work as the return value is a number of pages that are on the array, not the number of new pages allocated. Even if fixed, it still moves cost and complexity to the caller and the API is harder to use and easier to make mistakes. That shift in responsibility and the maintenance burden would need to be justified. While it is possible to use wrappers to allow callers to decide whether to manage the tracking or let the API handle it, the requirement then is to show that there is a performance gain for a common use case. This is outside the scope of a serise that removes the page_list argument. Even if a series was proposed to shift responsibility to the caller, I would not expect it to be submitted with a page_list removal. -- Mel Gorman SUSE Labs