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 C5298C52D7D for ; Fri, 16 Aug 2024 11:55:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4E1AC8D0077; Fri, 16 Aug 2024 07:55:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 46B088D0075; Fri, 16 Aug 2024 07:55:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2E4A88D0077; Fri, 16 Aug 2024 07:55:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 0CBF58D0075 for ; Fri, 16 Aug 2024 07:55:28 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id B6243C1A49 for ; Fri, 16 Aug 2024 11:55:27 +0000 (UTC) X-FDA: 82457953494.18.67EA61A Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf30.hostedemail.com (Postfix) with ESMTP id 4D59480021 for ; Fri, 16 Aug 2024 11:55:23 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf30.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723809243; 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; bh=6MF+Hys4URJmfz1fejfZxR/uz81I1wSZn2lhLw7Q2dA=; b=QbRUfcbmVXm4xqq/i+nIaEJl4k6T3ti4bu2Mz/H/uv+32V2PFZDsg//IGZPxab4wCleYxG Ggj/c4oyv8pEr8Zq+SKGfp4hVjOmEsjp0+W25l/GrpGU9EhUJjFBjSA3+WDMW3l+BoH4bh nE2UXoR2i7qtiUwLxEWRV98ZyEq4S50= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723809243; a=rsa-sha256; cv=none; b=TVqScnre808u8SLAA4B5oLO5xftTcwLeF9uAquuBpnreYezxoebgpU8Og2HuoBTfWejnU4 btKif8W//dvmIazSukdJIBAMmbghGN0lrM/NzatkhnqTf263vPnRfHuSf71KAWWiV5aHG7 o5CsTM9OquNyqNEKUdUEZa6YiEb2VlY= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf30.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com Received: from mail.maildlp.com (unknown [172.19.163.174]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4WlgQt2dSLzcdVh; Fri, 16 Aug 2024 19:55:02 +0800 (CST) Received: from dggpemf200006.china.huawei.com (unknown [7.185.36.61]) by mail.maildlp.com (Postfix) with ESMTPS id 879EA1403D5; Fri, 16 Aug 2024 19:55:19 +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, 16 Aug 2024 19:55:18 +0800 Message-ID: <3e069c81-a728-4d72-a5bb-3be00d182107@huawei.com> Date: Fri, 16 Aug 2024 19:55:18 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net-next v13 04/14] mm: page_frag: add '_va' suffix to page_frag API To: Alexander Duyck CC: , , , , , 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 , , , , , , , , , , , References: <20240808123714.462740-1-linyunsheng@huawei.com> <20240808123714.462740-5-linyunsheng@huawei.com> <676a2a15-d390-48a7-a8d7-6e491c89e200@huawei.com> Content-Language: en-US From: Yunsheng Lin In-Reply-To: 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-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 4D59480021 X-Stat-Signature: 8437azpip1gfr4kemk37sgor8dcgy4wz X-Rspam-User: X-HE-Tag: 1723809323-264969 X-HE-Meta: U2FsdGVkX18+A5u45Z/oMZLFJsYWy+Laq5tOqGO4DJfCgYA/5CHsll8j4yZ572eiQpQMxuTmmiiwvckFN8TRBP3VUOMyAYzfWCHjAS4cw9AqMe4vMUxSybRwK33CXfu3RsvHw/2w8xyONEexKb2TSnvtQygZAsthpGnwCIH4/qAU+jCJCMGJldiuiKZ83yoIracGABO7WvBHwp/whgqmUxnuQQdTa3yrwDC9mw2D9/m7KmMAmccVvHTXFADIJ5HAMB5YbLVO9tjY6OaWvuXAdrT0n9Ua+CmmxlMHy/Z9TV5FIwRD19U66qcSHM0mH4AIxGgxP3tACDtEFE/w3SYTdOrPAgsq6ib3LZIvqwHf5074pYK4Dqjd6IffycjhtLImoXY9uOJDUHhkxMVPZYIdjbnDpmf/5FOu5LCyMTXnmGJPivmUSlosbF37OJB5Gfqlzq0XOYW+y9pMQW2DK0lcHoTJ0dDgJF2cp+G54mDhYiCEuzd4d4VvZPd0RIXFPDAFxTok3rgMZhK635dU7i1ODoVvI8Nkd737fUvu2VZG/1RehmyVGENX1eS0z6hmi2DA5LSxZ810jVCKCX7ygJclbevpzIahqTtSjkCEwl+8cbyNIdv85cTLWfJw7r/D9Socc/bo8x8qMxes/U8fNKgHPol4FkIASn067m40j5cPgzvmzOGBfGuIcodPP8Gooa04n1547lNQLSWyE2Gv2JG8UVzTgPLiKHSWKf/Zr2+hwPCNy6+9XSSB1h/iSecLj1v6QbPeztt9PedtqyeRI/63pKgGhYan9ealPcJc9xfLzwgsZdBBG+PdoOSW21cKY26YdUxBfxfCkFXvebBZTRGuKdzhvwE7zj55OzPkEtY/yqa/IMv6lP6uWvGZj4RV5x8hoc1iOMpW0jH0fDtFqUQrs0VOPppnPXEF5gZkm2duPlrhYxTAkEaC2gc4VB1yiXMdjwQCVyDasgXRttIzUIZ wZsM9Hh2 g0U6Ciri7vt2PYjDCVj1oJqNcdp4dMlj1kW1h4BSryIUvqFblUzT64YNt01HikTp6QV1xln2eK8Usi1AvDObAjIcVucX7HWlyLdddpVhiWPsB9/778Ur+U8cPIlzvJfIAVAr8F2S1syvK1AG93QliNpNs0AzetB2Dt7GN4ERqXZpO5th1qXgTXckEMqdRik7FBOaqtr/4tRy/8w9XH7eriZK0O61Q9dIXAW3yIyLvylVklmP/HuE0TWFxL0J2GnRhzccMnuxxKtvX7FwrEHLLogAs2XDRQn1CSqykHomVbm3xVyxdJDsE7yG0u9mpS1NpwAH1BF4/vIwjtcORSYeOgFuef9TOgQPgQRH8TJqrVGNuKlMY1Mi9Bte0Y9nZIS8BUU9BI3Ej46bi4QtqIl5rB/KrFablTYyFMdy766T1HPgio/e8nXenn7jzDzxepsHn/ArE 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 2024/8/15 23:00, Alexander Duyck wrote: > On Wed, Aug 14, 2024 at 8:00 PM 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 adds >>> no value. If you insist on this patch I will reject the set every time. >>> >>> 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 what >>> 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? > 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); If the above options is what you have in mind, please be more specific which one is the prefer option, and why it is the prefer option. If the above options is not what you have in mind, please list out the declaration of API in your mind. > > I am going to nack this patch set if you insist on this pointless > renaming. The fact is it is just adding noise that adds no value.