From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EBFBD2045BC; Fri, 21 Feb 2025 09:34:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.187 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740130475; cv=none; b=nkn5hnSjvnb1hnRAD/aQEN5qR+Sl0X80VAslbPS4y4el6CvUu9wJ/BtRzeVCUw59TD9+PPr2SSzcwqkpLtxF3m0f8ThvfhyIUxK64PFI5d/9OhtMq6BAUwEpFzFV3TAdCVT8QiYcm/SIzZCkAEMswIKhba583JUjU9fVDwF2yko= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740130475; c=relaxed/simple; bh=UrE4zNeA6rGUajNEitvvWzIh6MglysOpeI2P5JxANP8=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=WU55k9ffti/1mCydPUJ82xGOQpXMrMkL5tnQreBqwBJyw/yr5pNBA/VfG6kTwdGxpgmd+hwh+OoTse7BVfPFREkMsWM7KmPnya/X3cLlI1v7OoD4ttF1EeXO8+KXe9RXjdijPcO5BNcAROYNCi1I7irotd+LMukh7XNENgOq3mo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.187 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.88.105]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4YzlG03WHwzdb9B; Fri, 21 Feb 2025 17:29:44 +0800 (CST) Received: from dggpemf200006.china.huawei.com (unknown [7.185.36.61]) by mail.maildlp.com (Postfix) with ESMTPS id 57A03140158; Fri, 21 Feb 2025 17:34:23 +0800 (CST) Received: from [10.67.120.129] (10.67.120.129) by dggpemf200006.china.huawei.com (7.185.36.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Fri, 21 Feb 2025 17:34:22 +0800 Message-ID: Date: Fri, 21 Feb 2025 17:34:22 +0800 Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC] mm: alloc_pages_bulk: remove assumption of populating only NULL elements To: Chuck Lever , Yishai Hadas , Jason Gunthorpe , Shameer Kolothum , Kevin Tian , Alex Williamson , Chris Mason , Josef Bacik , David Sterba , Gao Xiang , Chao Yu , Yue Hu , Jeffle Xu , Sandeep Dhavale , Carlos Maiolino , "Darrick J. Wong" , Andrew Morton , Jesper Dangaard Brouer , Ilias Apalodimas , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Trond Myklebust , Anna Schumaker , Jeff Layton , Neil Brown , Olga Kornievskaia , Dai Ngo , Tom Talpey CC: Luiz Capitulino , Mel Gorman , , , , , , , , , References: <20250217123127.3674033-1-linyunsheng@huawei.com> <7b7492c0-a3a7-470b-b7aa-697ac790a94b@oracle.com> Content-Language: en-US From: Yunsheng Lin In-Reply-To: <7b7492c0-a3a7-470b-b7aa-697ac790a94b@oracle.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpemf200006.china.huawei.com (7.185.36.61) On 2025/2/18 22:17, Chuck Lever wrote: > On 2/18/25 4:16 AM, Yunsheng Lin wrote: >> On 2025/2/17 22:20, Chuck Lever wrote: >>> On 2/17/25 7:31 AM, Yunsheng Lin wrote: >>>> As mentioned in [1], it seems odd to check NULL elements in >>>> the middle of page bulk allocating, >>> >>> I think I requested that check to be added to the bulk page allocator. >>> >>> When sending an RPC reply, NFSD might release pages in the middle of >> >> It seems there is no usage of the page bulk allocation API in fs/nfsd/ >> or fs/nfs/, which specific fs the above 'NFSD' is referring to? > > NFSD is in fs/nfsd/, and it is the major consumer of > net/sunrpc/svc_xprt.c. > > >>> the rq_pages array, marking each of those array entries with a NULL >>> pointer. We want to ensure that the array is refilled completely in this >>> case. >>> >> >> I did some researching, it seems you requested that in [1]? >> It seems the 'holes are always at the start' for the case in that >> discussion too, I am not sure if the case is referring to the caller >> in net/sunrpc/svc_xprt.c? If yes, it seems caller can do a better >> job of bulk allocating pages into a whole array sequentially without >> checking NULL elements first before doing the page bulk allocation >> as something below: >> >> +++ b/net/sunrpc/svc_xprt.c >> @@ -663,9 +663,10 @@ static bool svc_alloc_arg(struct svc_rqst *rqstp) >> pages = RPCSVC_MAXPAGES; >> } >> >> - for (filled = 0; filled < pages; filled = ret) { >> - ret = alloc_pages_bulk(GFP_KERNEL, pages, rqstp->rq_pages); >> - if (ret > filled) >> + for (filled = 0; filled < pages; filled += ret) { >> + ret = alloc_pages_bulk(GFP_KERNEL, pages - filled, >> + rqstp->rq_pages + filled); >> + if (ret) >> /* Made progress, don't sleep yet */ >> continue; >> >> @@ -674,7 +675,7 @@ static bool svc_alloc_arg(struct svc_rqst *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]; >> >> >> 1. https://lkml.iu.edu/hypermail/linux/kernel/2103.2/09060.html > > I still don't see what is broken about the current API. As mentioned in [1], the page bulk alloc API before this patch may have some space for improvement from performance and easy-to-use perspective as the most existing calllers of page bulk alloc API are trying to bulk allocate the page for the whole array sequentially. 1. https://lore.kernel.org/all/c9950a79-7bcb-41c2-a59e-af315dc6d7ff@huawei.com/ > > Anyway, any changes in svc_alloc_arg() will need to be run through the > upstream NFSD CI suite before they are merged. Is there any web link pointing to the above NFSD CI suite, so that I can test it if removing assumption of populating only NULL elements is indeed possible? Look more closely, it seems svc_rqst_release_pages()/svc_rdma_save_io_pages() does set rqstp->rq_respages[i] to NULL based on rqstp->rq_next_page, and the original code before using the page bulk alloc API does seem to only allocate page for NULL elements as can see from the below patch： https://lore.kernel.org/all/20210325114228.27719-8-mgorman@techsingularity.net/T/#u The clearing of rqstp->rq_respages[] to NULL does seems sequentially, is it possible to only pass NULL elements in rqstp->rq_respages[] to alloc_pages_bulk() so that bulk alloc API does not have to do the NULL checking and use the array only as output parameter? > > 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 39643C021AA for ; Fri, 21 Feb 2025 09:35:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lists.ozlabs.org; s=201707; t=1740130506; bh=KV6/ZMu6u/scLjwfN4QKh8XL+w11Fjmxpk6VPBV1oRc=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=OaLa2eSLDWEM/MTKOUK6K585BTMBbKwR4q73y7YzFtG10KidBVDRUBxH3Rqg0VB83 PuB35TNNzdoJ0jrGRH567BxSS7Hu21bEqFSI+4SGb0q5vDTWNkFxKa15zeK15A8PaW z6W7todPiVoOwT3Hp+kR+hMbX3/+NQfXYHhdajGAI2Buvd2kFuZMzwI3f4+nu864wq 1J/rxHfUpao//AnWXXh6lVZ9iOoTpR/hPgWZ60hwungG9BJqGyT+nngvqAQe/ueZA/ IJhiA7rWHeVIh0PoZczii+yhnh5boqKTYPdjvEIrHzKnnTXhEfEDrvKIiO0QXDf3+o ZN4hFar3JzT3A== Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4YzlNB6LKmz30YZ for ; Fri, 21 Feb 2025 20:35:06 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; arc=none smtp.remote-ip=45.249.212.187 ARC-Seal: i=1; a=rsa-sha256; d=lists.ozlabs.org; s=201707; t=1740130503; cv=none; b=Kj1TceCT/jJ5ZkKlpNqRJC7gdwE0wlSQiMLxeXI3AIeQHiEsOCTqzV+ZRPHsDb0EYP1sOUuOk1PDHH1e/sX6Cx6lluz19pehLN2OCZ/IaawXarYrD7D3ovFDpdFXzIx3rMcewHaOtaTr0x3xcrHa0IY61Jdb9XzldbgPJR3JluYI3bNGoFu0rpMKU4tKEWhl+70iJHIlfSzw23avYpsQgYQUu8bb3EZfRDrrkM6LBoP7IB8YKVihYJAu1t4KAQMbQEqREWH50qhZPL/9pcx3IyxyE5mOTUJJAblGqXX1nXlF37dK0sP7wpHIPCLJ5o3eOmndArWJ5EyDkH4/tioYjg== ARC-Message-Signature: i=1; a=rsa-sha256; d=lists.ozlabs.org; s=201707; t=1740130503; c=relaxed/relaxed; bh=KV6/ZMu6u/scLjwfN4QKh8XL+w11Fjmxpk6VPBV1oRc=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=OUzkt2p6NwQFmcUdeCYlBDMSMDK53LNED3OAaAj9r8+uyUQK/1dgPVMwAMfGstT4SRVGK23TKJciDxVYOZOjImvcvq+6VENvIdaGa/UDd/U+uLk9RtECT2jGIM9HnCQX61ZOpD1yPUKoG+41txT7ZaFZo8WNubF5WIqoZE/OXa/K9r813lZ870WhqIw8p/auDt5V9O8voeFYX7UtPRVbKaaiVIBgNvLcVL3eERHBxEGGQ2VSOS7KC3YPxoUBGX/Mq+YVcRft7mb8yoWKtUy8KWlSGChtqH3Q46965swJxd2J66D7WHHoD47SIOCvT+baWaoTYAzeHU970f55CD6YGw== ARC-Authentication-Results: i=1; lists.ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass (client-ip=45.249.212.187; helo=szxga01-in.huawei.com; envelope-from=linyunsheng@huawei.com; receiver=lists.ozlabs.org) smtp.mailfrom=huawei.com Authentication-Results: lists.ozlabs.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=huawei.com (client-ip=45.249.212.187; helo=szxga01-in.huawei.com; envelope-from=linyunsheng@huawei.com; receiver=lists.ozlabs.org) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4YzlN51jsDz30T7 for ; Fri, 21 Feb 2025 20:34:59 +1100 (AEDT) Received: from mail.maildlp.com (unknown [172.19.88.105]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4YzlG03WHwzdb9B; Fri, 21 Feb 2025 17:29:44 +0800 (CST) Received: from dggpemf200006.china.huawei.com (unknown [7.185.36.61]) by mail.maildlp.com (Postfix) with ESMTPS id 57A03140158; Fri, 21 Feb 2025 17:34:23 +0800 (CST) Received: from [10.67.120.129] (10.67.120.129) by dggpemf200006.china.huawei.com (7.185.36.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Fri, 21 Feb 2025 17:34:22 +0800 Message-ID: Date: Fri, 21 Feb 2025 17:34:22 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC] mm: alloc_pages_bulk: remove assumption of populating only NULL elements To: Chuck Lever , Yishai Hadas , Jason Gunthorpe , Shameer Kolothum , Kevin Tian , Alex Williamson , Chris Mason , Josef Bacik , David Sterba , Gao Xiang , Chao Yu , Yue Hu , Jeffle Xu , Sandeep Dhavale , Carlos Maiolino , "Darrick J. Wong" , Andrew Morton , Jesper Dangaard Brouer , Ilias Apalodimas , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Trond Myklebust , Anna Schumaker , Jeff Layton , Neil Brown , Olga Kornievskaia , Dai Ngo , Tom Talpey References: <20250217123127.3674033-1-linyunsheng@huawei.com> <7b7492c0-a3a7-470b-b7aa-697ac790a94b@oracle.com> Content-Language: en-US In-Reply-To: <7b7492c0-a3a7-470b-b7aa-697ac790a94b@oracle.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.120.129] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggpemf200006.china.huawei.com (7.185.36.61) X-BeenThere: linux-erofs@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development of Linux EROFS file system List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Yunsheng Lin via Linux-erofs Reply-To: Yunsheng Lin Cc: linux-nfs@vger.kernel.org, kvm@vger.kernel.org, linux-erofs@lists.ozlabs.org, linux-kernel@vger.kernel.org, virtualization@lists.linux.dev, linux-xfs@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, Mel Gorman , linux-btrfs@vger.kernel.org, Luiz Capitulino Errors-To: linux-erofs-bounces+linux-erofs=archiver.kernel.org@lists.ozlabs.org Sender: "Linux-erofs" On 2025/2/18 22:17, Chuck Lever wrote: > On 2/18/25 4:16 AM, Yunsheng Lin wrote: >> On 2025/2/17 22:20, Chuck Lever wrote: >>> On 2/17/25 7:31 AM, Yunsheng Lin wrote: >>>> As mentioned in [1], it seems odd to check NULL elements in >>>> the middle of page bulk allocating, >>> >>> I think I requested that check to be added to the bulk page allocator. >>> >>> When sending an RPC reply, NFSD might release pages in the middle of >> >> It seems there is no usage of the page bulk allocation API in fs/nfsd/ >> or fs/nfs/, which specific fs the above 'NFSD' is referring to? > > NFSD is in fs/nfsd/, and it is the major consumer of > net/sunrpc/svc_xprt.c. > > >>> the rq_pages array, marking each of those array entries with a NULL >>> pointer. We want to ensure that the array is refilled completely in this >>> case. >>> >> >> I did some researching, it seems you requested that in [1]? >> It seems the 'holes are always at the start' for the case in that >> discussion too, I am not sure if the case is referring to the caller >> in net/sunrpc/svc_xprt.c? If yes, it seems caller can do a better >> job of bulk allocating pages into a whole array sequentially without >> checking NULL elements first before doing the page bulk allocation >> as something below: >> >> +++ b/net/sunrpc/svc_xprt.c >> @@ -663,9 +663,10 @@ static bool svc_alloc_arg(struct svc_rqst *rqstp) >> pages = RPCSVC_MAXPAGES; >> } >> >> - for (filled = 0; filled < pages; filled = ret) { >> - ret = alloc_pages_bulk(GFP_KERNEL, pages, rqstp->rq_pages); >> - if (ret > filled) >> + for (filled = 0; filled < pages; filled += ret) { >> + ret = alloc_pages_bulk(GFP_KERNEL, pages - filled, >> + rqstp->rq_pages + filled); >> + if (ret) >> /* Made progress, don't sleep yet */ >> continue; >> >> @@ -674,7 +675,7 @@ static bool svc_alloc_arg(struct svc_rqst *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]; >> >> >> 1. https://lkml.iu.edu/hypermail/linux/kernel/2103.2/09060.html > > I still don't see what is broken about the current API. As mentioned in [1], the page bulk alloc API before this patch may have some space for improvement from performance and easy-to-use perspective as the most existing calllers of page bulk alloc API are trying to bulk allocate the page for the whole array sequentially. 1. https://lore.kernel.org/all/c9950a79-7bcb-41c2-a59e-af315dc6d7ff@huawei.com/ > > Anyway, any changes in svc_alloc_arg() will need to be run through the > upstream NFSD CI suite before they are merged. Is there any web link pointing to the above NFSD CI suite, so that I can test it if removing assumption of populating only NULL elements is indeed possible? Look more closely, it seems svc_rqst_release_pages()/svc_rdma_save_io_pages() does set rqstp->rq_respages[i] to NULL based on rqstp->rq_next_page, and the original code before using the page bulk alloc API does seem to only allocate page for NULL elements as can see from the below patch： https://lore.kernel.org/all/20210325114228.27719-8-mgorman@techsingularity.net/T/#u The clearing of rqstp->rq_respages[] to NULL does seems sequentially, is it possible to only pass NULL elements in rqstp->rq_respages[] to alloc_pages_bulk() so that bulk alloc API does not have to do the NULL checking and use the array only as output parameter? > >