Re: [PATCH v2 16/16] mt76: use netmem descriptor and APIs for page pool

[Mina Almasry <almasrymina@google.com>]

On Tue, May 27, 2025 at 7:29 PM Byungchul Park <byungchul@sk.com> wrote:
>
> To simplify struct page, the effort to separate its own descriptor from
> struct page is required and the work for page pool is on going.
>
> Use netmem descriptor and APIs for page pool in mt76 code.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>

This patch looks fine to me... but this is converting 1 driver to
netmem. There are hundreds of other drivers in the tree. Are we
converting all of them to netmem...?

The motivating reason for existing drivers to use netmem is because
it's an abstract memory type that lets the driver support dma-buf
memory and io_uring memory:

https://www.kernel.org/doc/Documentation/networking/netmem.rst

This driver does not use the page_pool, doesn't support dma-buf
memory, or io_uring memory. Moving it to netmem I think will add
overhead due to the netmem_is_net_iov checks while providing no
tangible benefit AFAICT. Is your long term plan to convert all drivers
to netmem? That maybe thousands of lines of code that need to be
touched :(

> ---
>  drivers/net/wireless/mediatek/mt76/dma.c      |  6 ++---
>  drivers/net/wireless/mediatek/mt76/mt76.h     | 12 +++++-----
>  .../net/wireless/mediatek/mt76/sdio_txrx.c    | 24 +++++++++----------
>  drivers/net/wireless/mediatek/mt76/usb.c      | 10 ++++----
>  4 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
> index 35b4ec91979e..cceff435ec4a 100644
> --- a/drivers/net/wireless/mediatek/mt76/dma.c
> +++ b/drivers/net/wireless/mediatek/mt76/dma.c
> @@ -820,10 +820,10 @@ mt76_add_fragment(struct mt76_dev *dev, struct mt76_queue *q, void *data,
>         int nr_frags = shinfo->nr_frags;
>
>         if (nr_frags < ARRAY_SIZE(shinfo->frags)) {
> -               struct page *page = virt_to_head_page(data);
> -               int offset = data - page_address(page) + q->buf_offset;
> +               netmem_ref netmem = netmem_compound_head(virt_to_netmem(data));

It may be worth adding virt_to_head_netmem helper instead of doing
these 2 calls together everywhere.

-- 
Thanks,
Mina