linux-kernel - Re: [PATCH net-next v13 04/14] mm: page_frag: add '_va' suffix to 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: <d1a23116d054e2ebb00067227f0cffecefe33e11.camel@gmail.com>
Date: Wed, 14 Aug 2024 08:49:53 -0700
From: Alexander H Duyck <alexander.duyck@...il.com>
To: Yunsheng Lin <linyunsheng@...wei.com>, davem@...emloft.net,
 kuba@...nel.org,  pabeni@...hat.com
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Subbaraya Sundeep
 <sbhatta@...vell.com>, Chuck Lever <chuck.lever@...cle.com>, Sagi Grimberg
 <sagi@...mberg.me>, Jeroen de Borst <jeroendb@...gle.com>, Praveen
 Kaligineedi <pkaligineedi@...gle.com>, Shailend Chand
 <shailend@...gle.com>, Eric Dumazet <edumazet@...gle.com>, Tony Nguyen
 <anthony.l.nguyen@...el.com>, Przemek Kitszel
 <przemyslaw.kitszel@...el.com>, Sunil Goutham <sgoutham@...vell.com>,
 Geetha sowjanya <gakula@...vell.com>, hariprasad <hkelam@...vell.com>,
 Felix Fietkau <nbd@....name>, Sean Wang <sean.wang@...iatek.com>, Mark Lee
 <Mark-MC.Lee@...iatek.com>, Lorenzo Bianconi <lorenzo@...nel.org>, Matthias
 Brugger <matthias.bgg@...il.com>, AngeloGioacchino Del Regno
 <angelogioacchino.delregno@...labora.com>, Keith Busch <kbusch@...nel.org>,
  Jens Axboe <axboe@...nel.dk>, Christoph Hellwig <hch@....de>, Chaitanya
 Kulkarni <kch@...dia.com>,  "Michael S. Tsirkin" <mst@...hat.com>, Jason
 Wang <jasowang@...hat.com>, Eugenio Pérez
 <eperezma@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>, Alexei
 Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend
 <john.fastabend@...il.com>, Andrii Nakryiko <andrii@...nel.org>, Martin
 KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, KP
 Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo
 <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,  David Howells
 <dhowells@...hat.com>, Marc Dionne <marc.dionne@...istor.com>, Jeff Layton
 <jlayton@...nel.org>,  Neil Brown <neilb@...e.de>, Olga Kornievskaia
 <kolga@...app.com>, Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey
 <tom@...pey.com>, Trond Myklebust <trondmy@...nel.org>, Anna Schumaker
 <anna@...nel.org>,  Shuah Khan <shuah@...nel.org>,
 intel-wired-lan@...ts.osuosl.org, linux-arm-kernel@...ts.infradead.org, 
 linux-mediatek@...ts.infradead.org, linux-nvme@...ts.infradead.org, 
 kvm@...r.kernel.org, virtualization@...ts.linux.dev, linux-mm@...ck.org, 
 bpf@...r.kernel.org, linux-afs@...ts.infradead.org,
 linux-nfs@...r.kernel.org,  linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next v13 04/14] mm: page_frag: add '_va' suffix to
 page_frag API

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 <alexander.duyck@...il.com>
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> Reviewed-by: Subbaraya Sundeep <sbhatta@...vell.com>
> Acked-by: Chuck Lever <chuck.lever@...cle.com>
> Acked-by: Sagi Grimberg <sagi@...mberg.me>
> ---
>  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
makes it that much harder to backport fixes in the future as people
will have to work around the rename.