Re: [PATCH net-next v9 8/8] mt76: use netmem descriptor and APIs for page pool

[Byungchul Park <byungchul@sk.com>]

On Sat, Jul 12, 2025 at 03:22:17PM +0100, Pavel Begunkov wrote:
> On 7/10/25 09:28, Byungchul Park 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>
> > Reviewed-by: Mina Almasry <almasrymina@google.com>
> > ---
> ...>   static inline void mt76_set_tx_blocked(struct mt76_dev *dev, bool blocked)
> > diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > index 0a927a7313a6..b1d89b6f663d 100644
> > --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > @@ -68,14 +68,14 @@ mt76s_build_rx_skb(void *data, int data_len, int buf_len)
> > 
> >       skb_put_data(skb, data, len);
> >       if (data_len > len) {
> > -             struct page *page;
> > +             netmem_ref netmem;
> > 
> >               data += len;
> > -             page = virt_to_head_page(data);
> > -             skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> > -                             page, data - page_address(page),
> > -                             data_len - len, buf_len);
> > -             get_page(page);
> > +             netmem = virt_to_head_netmem(data);
> > +             skb_add_rx_frag_netmem(skb, skb_shinfo(skb)->nr_frags,
> > +                                    netmem, data - netmem_address(netmem),
> > +                                    data_len - len, buf_len);
> > +             get_netmem(netmem);
> >       }
> > 
> >       return skb;
> > @@ -88,7 +88,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> >       struct mt76_queue *q = &dev->q_rx[qid];
> >       struct mt76_sdio *sdio = &dev->sdio;
> >       int len = 0, err, i;
> > -     struct page *page;
> > +     netmem_ref netmem;
> >       u8 *buf, *end;
> > 
> >       for (i = 0; i < intr->rx.num[qid]; i++)
> > @@ -100,11 +100,11 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> >       if (len > sdio->func->cur_blksize)
> >               len = roundup(len, sdio->func->cur_blksize);
> > 
> > -     page = __dev_alloc_pages(GFP_KERNEL, get_order(len));
> > -     if (!page)
> > +     netmem = page_to_netmem(__dev_alloc_pages(GFP_KERNEL, get_order(len)));
> > +     if (!netmem)
> >               return -ENOMEM;
> > 
> > -     buf = page_address(page);
> > +     buf = netmem_address(netmem);
> 
> We shouldn't just blindly convert everything to netmem just for the purpose
> of creating a type casting hell. It's allocating a page, and continues to
> use it as a page, e.g. netmem_address() will fail otherwise. So just leave
> it to be a page, and convert it to netmem and the very last moment when
> the api expects a netmem. There are likely many chunks like that.

Thanks for the feedback.

Unon reconsideration, focusing on the conversion between page and
netmem_desc, plus small modification on user side code e.i. driver are
sufficient to achieve my objectives.  I won't change a lot on user side
code like this from the next spin.

	Byungchul

> > 
> >       sdio_claim_host(sdio->func);
> >       err = sdio_readsb(sdio->func, buf, MCR_WRDR(qid), len);
> > @@ -112,7 +112,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> > 
> >       if (err < 0) {
> >               dev_err(dev->dev, "sdio read data failed:%d\n", err);
> > -             put_page(page);
> > +             put_netmem(netmem);
> >               return err;
> >       }
> > 
> > @@ -140,7 +140,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> >               }
> >               buf += round_up(len + 4, 4);
> >       }
> > -     put_page(page);
> > +     put_netmem(netmem);
> > 
> >       spin_lock_bh(&q->lock);
> >       q->head = (q->head + i) % q->ndesc;
> --
> Pavel Begunkov