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 041F2C3DA4A for ; Mon, 19 Aug 2024 15:55:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 87C2B6B0088; Mon, 19 Aug 2024 11:55:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 82B5D6B008A; Mon, 19 Aug 2024 11:55:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6CBD46B008C; Mon, 19 Aug 2024 11:55:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 4E8E26B0088 for ; Mon, 19 Aug 2024 11:55:14 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id B5465121418 for ; Mon, 19 Aug 2024 15:55:13 +0000 (UTC) X-FDA: 82469444106.20.581C201 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf07.hostedemail.com (Postfix) with ESMTP id BE5D340013 for ; Mon, 19 Aug 2024 15:55:10 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=SskuBagj; spf=pass (imf07.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724082849; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=sIOAt7qcIh7yk+Ak6kXM6rwImTNnnUyR1orHAIYs4r4=; b=5czpcK88115GTTFUvC6tBVIirmPqftmXaW4eNiigtEi170G/NagBcR8eOXe3mkTfRnEkUB dQGCLCFXUSnNL45VLP5cj0WrC/eTciAny5fyKUaZPffoAT4LYkbDTt69lm46MI1l95vJXO hoLekPS31aqcRyRI7jU0nivSZh6vllI= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=SskuBagj; spf=pass (imf07.hostedemail.com: domain of alexander.duyck@gmail.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=alexander.duyck@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724082849; a=rsa-sha256; cv=none; b=BagFzQ1GtNu+qwNfyn3O0+R9SCvThC0phMP5Iwetjax6fkpRCiX5AvhE9J3fN3H7fPw1yB DtlusdTijTFAgrgkPLye8eV2yZ+JpzpARmkU5isA1YUgrKkmjQ4dFxgEYnS+hxSp71mGM+ M6BXPBnC2F0y6qRmVntrTvb/88YcpIs= Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-4280b3a7efaso35323785e9.0 for ; Mon, 19 Aug 2024 08:55:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724082909; x=1724687709; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=sIOAt7qcIh7yk+Ak6kXM6rwImTNnnUyR1orHAIYs4r4=; b=SskuBagjpqEKsYrh52+7nTbDthY+TP30zgrXhOXtp+ukAJdKkKfPXRFcov+gS1QvvC xOCkqguUm2/5qCyGy5Zx9fbf4ivaSQ5Qpehf1u8xbn8eekoww4pvehXI05oRzT8ZDg3s CaGkuCy580jjAWpwkdvU0UwGzLpSsgXlxTl0U6gtzzP4E0zEhmyfzBd5WUK2qq0DYQsT X4OIk2MxkQd3qOzbHsvw+sU6OC/+UrFx0wxLBSj9AnaKNAN2856S0q+z41YLLXJomK4b EyvWMYhguJ83cfSPTSMM8LblArViiu2nr2uLO6yZGyxDGEwlGfYnjxrNOPbe9iIabRCK 97nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724082909; x=1724687709; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=sIOAt7qcIh7yk+Ak6kXM6rwImTNnnUyR1orHAIYs4r4=; b=XmEadGgb4IGcjYThxtAVxNYUVqY1BwE61Xy60g1O1uj6de96DNPfD2gdcGkVRbasS6 C1rcaCUqVRPONjgBBpMnWtmzYJkk2vzWvZZN1El29AS2L5csNzOEa7dU/palsA6Obqnt HjEaO2VqKqjH5x5LswZqy09Xh0zmtSSRpT5Rc1dZ18PuLjWAedsgKNBrccehcs6cRCze 3BjZCnNsF6+r6lw1ktBcaIpjGXDNROoHi6EThcnMqBQCIzVXo53EUFucvWMVMsrbn/L1 b7IcaipVvCPWsFOJmv4NdcN4UpOobGPi0rAjyxAC23/sGmBfurCwHNg1JQYhbk69iImw w7bQ== X-Forwarded-Encrypted: i=1; AJvYcCWvp/fZQ40p7GTF/grhiVf06zpJxtPL4xzMTZgE0MyjegVeYHd8YGRoGhO+CZ6lrSYL8biMDEJT/T6KUw8S1F+t9G0= X-Gm-Message-State: AOJu0YzpijaVnGx1Cf0o/WiFvPVjasqMf31dAYtPPKC4uyyKNerWS8QV R4oe1oyT/V9EbgAMnk5uChPpOm12ITRBV/I54y6ivOM4FtjnMlyxzZd0B3V0qsTFW2oh4ceXOi7 XjqGpkXK+ZkUfUuKqeSEJc8D8ocs= X-Google-Smtp-Source: AGHT+IH+5SdMMmvczHHbCQS3d2lV+42NHIBp97s6EuIfp9v1lsrp5Tb1uRlaaTz2L+/Rjsor4cGm4qQ4+md5bQzOvmo= X-Received: by 2002:a05:6000:1006:b0:371:8eaf:3c49 with SMTP id ffacd0b85a97d-371946a32a9mr7016301f8f.40.1724082909035; Mon, 19 Aug 2024 08:55:09 -0700 (PDT) MIME-Version: 1.0 References: <20240808123714.462740-1-linyunsheng@huawei.com> <20240808123714.462740-5-linyunsheng@huawei.com> <676a2a15-d390-48a7-a8d7-6e491c89e200@huawei.com> <3e069c81-a728-4d72-a5bb-3be00d182107@huawei.com> In-Reply-To: <3e069c81-a728-4d72-a5bb-3be00d182107@huawei.com> From: Alexander Duyck Date: Mon, 19 Aug 2024 08:54:32 -0700 Message-ID: Subject: Re: [PATCH net-next v13 04/14] mm: page_frag: add '_va' suffix to page_frag API To: Yunsheng Lin Cc: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Subbaraya Sundeep , Chuck Lever , Sagi Grimberg , Jeroen de Borst , Praveen Kaligineedi , Shailend Chand , Eric Dumazet , Tony Nguyen , Przemek Kitszel , Sunil Goutham , Geetha sowjanya , hariprasad , Felix Fietkau , Sean Wang , Mark Lee , Lorenzo Bianconi , Matthias Brugger , AngeloGioacchino Del Regno , Keith Busch , Jens Axboe , Christoph Hellwig , Chaitanya Kulkarni , "Michael S. Tsirkin" , Jason Wang , =?UTF-8?Q?Eugenio_P=C3=A9rez?= , Andrew Morton , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Song Liu , Yonghong Song , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , David Howells , Marc Dionne , Jeff Layton , Neil Brown , Olga Kornievskaia , Dai Ngo , Tom Talpey , Trond Myklebust , Anna Schumaker , Shuah Khan , intel-wired-lan@lists.osuosl.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, linux-nvme@lists.infradead.org, kvm@vger.kernel.org, virtualization@lists.linux.dev, linux-mm@kvack.org, bpf@vger.kernel.org, linux-afs@lists.infradead.org, linux-nfs@vger.kernel.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: mji3sk1iqa6484tsyruk659h7ijpcn63 X-Rspam-User: X-Rspamd-Queue-Id: BE5D340013 X-Rspamd-Server: rspam02 X-HE-Tag: 1724082910-470219 X-HE-Meta: U2FsdGVkX19L+pB8Qrm+641s4mDaixFhBHpAepEKIrfIUBqEij6+qPQeCzV1UUbbrWyU8eQWVztzAv1PDmLlD+vow/NxMrWIl3QUZJCNQdl/Ta3vIoYHcVZLF2u0TjpPMz2P870knFVkTPWTMkF/IkpV2pZ5REs+6wvm/nGDRuOQrdqd4uiZ10Cz09+/HCIjIClKVrUQWQ9ZXjPxDaUJmtsYM9nJgVy65si3JpDmX1Q5zlUyDQ5Nr+MilpY5oGBOTpKIEbfBfjiuIUYgWayjzLULBBv4XM9f3vXSonf63UyvkZAfHisLkrkBxRbtg2lFnrhDEg1tiigcBFEiWWXDDjjprc5cImSDCs9Z88g3zli1T2WPtpSuYs/l21D5mIFuofkfDOfgYb/Sx8Jj1U/mo+iaNmbfw2OwT/afgkA0uQlVsOVQ+JXiajjMbTt8N5i5YcVpRHKjyhBeodQ84a+DAahdjbjlCOCLDUFPrmSVPel7bTwOr00VTC3t28KDqpJkfCvo4nWF5rTM67FeYPK9e7W923c60gQp+OaLfKmd7yFHaPheNh9vsJ9+5l21Dm2gOUvMRHXNXXQsFhJU2PidtGcbpBTOIyQoX6/YjrWUtciPRa4hxN4po6BlXFhArglCYol0+68PphVKLqGm9VC6YoWCJnzTE+dcGwQE2UTCVeRWCVo82c8+A+W0bO+GyPDmcqZF0y49qbI9zObEcFaJJxXYmXW359Rf06xETGrqQgFZ38BB7G07oA1c7cI9Nhpcm+ciE9G3WOuVzaOjZHgCAYV7xylfoiMBFcsNfl+WDxGzKHMAspB2T0gid3XWuQZyFp1pYDm7zBWkgCyC/NTe/YtBvk7JXGc8EESWvqbykQvSktwCw9BYyAjvPmhfPXd2uQphrAn6O07nixs3UsT7d1qvVRPN7+dtDOW/aXWxRpCRIRO4Qs2B0GurRY134r+fgmFah1KRyOfjoFtw0He lsUjKr/s IWJP8h3Omi4c2L8CkKJIQUDCRTMXqj0sjLfHNlhL3+OgbiYzKnef0NppZy3gZoLWn/3VOXmvHdfP1xZ2mDGH0ja1TEgy7yxEdGfatggU8fzcO6lnKh6xKzsH0R2HWShK7o6jXDonRG/ifuJyTKdzmoQbkXwXKBdmi6sU2+enCJJcC2I2FzwSaeyDVKth+Gs8MkgB7XAISGrFqNm7bkd1m0OheX9F6dPgXuRe6KdaEXjKj9DqLUFW6GcMg+dZ80YwVgdqNg6Y8IAG+fcfG8hc1EPWrpkVc+HsHLDTy3TdJg5lf/391DXve+O520YbalDeydJC14rnCb+KGOPxIym7YWDl9dSg/Vgt6WHwIxdp3T4zIFrO1xyNxUJwhTU1smt3oKRGGPSo5CqUzhZBPbPUgvq0f3S/Pr1UGfMC3tWj/6E76haf97atvCeF4mfY+ydu8gHg6fsp4MgGSjpZkNB/SkslpqGLIDNLJra9oQFJOsL7AK2VsTN3LEOgaCoSCEBZoaeiSjZ1KCBb3BnzFUVa/sNiH5D32SL1cw8iH1GECR+ReExTcl9zOeqEZOwk0hqBIh5sh 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, Aug 16, 2024 at 4:55=E2=80=AFAM Yunsheng Lin wrote: > > On 2024/8/15 23:00, Alexander Duyck wrote: > > On Wed, Aug 14, 2024 at 8:00=E2=80=AFPM Yunsheng Lin wrote: > >> > >> On 2024/8/14 23:49, Alexander H Duyck wrote: > >>> On Thu, 2024-08-08 at 20:37 +0800, Yunsheng Lin wrote: > >>>> Currently the page_frag API is returning 'virtual address' > >>>> or 'va' when allocing and expecting 'virtual address' or > >>>> 'va' as input when freeing. > >>>> > >>>> As we are about to support new use cases that the caller > >>>> need to deal with 'struct page' or need to deal with both > >>>> 'va' and 'struct page'. In order to differentiate the API > >>>> handling between 'va' and 'struct page', add '_va' suffix > >>>> to the corresponding API mirroring the page_pool_alloc_va() > >>>> API of the page_pool. So that callers expecting to deal with > >>>> va, page or both va and page may call page_frag_alloc_va*, > >>>> page_frag_alloc_pg*, or page_frag_alloc* API accordingly. > >>>> > >>>> CC: Alexander Duyck > >>>> Signed-off-by: Yunsheng Lin > >>>> Reviewed-by: Subbaraya Sundeep > >>>> Acked-by: Chuck Lever > >>>> Acked-by: Sagi Grimberg > >>>> --- > >>>> drivers/net/ethernet/google/gve/gve_rx.c | 4 ++-- > >>>> drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +- > >>>> drivers/net/ethernet/intel/ice/ice_txrx.h | 2 +- > >>>> drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 2 +- > >>>> .../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++-- > >>>> .../marvell/octeontx2/nic/otx2_common.c | 2 +- > >>>> drivers/net/ethernet/mediatek/mtk_wed_wo.c | 4 ++-- > >>>> drivers/nvme/host/tcp.c | 8 +++---- > >>>> drivers/nvme/target/tcp.c | 22 +++++++++-------= --- > >>>> drivers/vhost/net.c | 6 ++--- > >>>> include/linux/page_frag_cache.h | 21 +++++++++-------= -- > >>>> include/linux/skbuff.h | 2 +- > >>>> kernel/bpf/cpumap.c | 2 +- > >>>> mm/page_frag_cache.c | 12 +++++----- > >>>> net/core/skbuff.c | 16 +++++++------- > >>>> net/core/xdp.c | 2 +- > >>>> net/rxrpc/txbuf.c | 15 +++++++------ > >>>> net/sunrpc/svcsock.c | 6 ++--- > >>>> .../selftests/mm/page_frag/page_frag_test.c | 13 ++++++----- > >>>> 19 files changed, 75 insertions(+), 70 deletions(-) > >>>> > >>> > >>> I still say no to this patch. It is an unnecessary name change and ad= ds > >>> no value. If you insist on this patch I will reject the set every tim= e. > >>> > >>> The fact is it is polluting the git history and just makes things > >>> harder to maintain without adding any value as you aren't changing wh= at > >>> the function does and there is no need for this. In addition it just > >> > >> I guess I have to disagree with the above 'no need for this' part for > >> now, as mentioned in [1]: > >> > >> "There are three types of API as proposed in this patchset instead of > >> two types of API: > >> 1. page_frag_alloc_va() returns [va]. > >> 2. page_frag_alloc_pg() returns [page, offset]. > >> 3. page_frag_alloc() returns [va] & [page, offset]. > >> > >> You seemed to miss that we need a third naming for the type 3 API. > >> Do you see type 3 API as a valid API? if yes, what naming are you > >> suggesting for it? if no, why it is not a valid API?" > > > > I didn't. I just don't see the point in pushing out the existing API > > to support that. In reality 2 and 3 are redundant. You probably only > > need 3. Like I mentioned earlier you can essentially just pass a > > If the caller just expect [page, offset], do you expect the caller also > type 3 API, which return both [va] and [page, offset]? > > I am not sure if I understand why you think 2 and 3 are redundant here? > If you think 2 and 3 are redundant here, aren't 1 and 3 also redundant > as the similar agrument? The big difference is the need to return page and offset. Basically to support returning page and offset you need to pass at least one value as a pointer so you can store the return there. The reason why 3 is just a redundant form of 2 is that you will normally just be converting from a va to a page and offset so the va should already be easily accessible. > > page_frag via pointer to the function. With that you could also look > > at just returning a virtual address as well if you insist on having > > something that returns all of the above. No point in having 2 and 3 be > > seperate functions. > > Let's be more specific about what are your suggestion here: which way > is the prefer way to return the virtual address. It seems there are two > options: > > 1. Return the virtual address by function returning as below: > void *page_frag_alloc_bio(struct page_frag_cache *nc, struct bio_vec *bio= ); > > 2. Return the virtual address by double pointer as below: > int page_frag_alloc_bio(struct page_frag_cache *nc, struct bio_vec *bio, > void **va); I was thinking more of option 1. Basically this is a superset of page_frag_alloc_va that is also returning the page and offset via a page frag. However instead of bio_vec I would be good with "struct page_frag *" being the value passed to the function to play the role of container. Basically the big difference between 1 and 2/3 if I am not mistaken is the fact that for 1 you pass the size, whereas with 2/3 you are peeling off the page frag from the larger page frag cache after the fact via a commit type action.