Re: [PATCH net-next v11] virtio_net: add page_pool support for buffer allocation

[Jason Wang <jasowang@redhat.com>]

On Sat, Mar 14, 2026 at 12:51 AM Vishwanath Seshagiri <vishs@meta.com> wrote:
>
>
>
> On 3/13/26 1:21 PM, Jason Wang wrote:
> >> -static void put_xdp_frags(struct xdp_buff *xdp)
> >> +static void put_xdp_frags(struct receive_queue *rq, struct xdp_buff *xdp)
> >>   {
> >>          struct skb_shared_info *shinfo;
> >>          struct page *xdp_page;
> >> @@ -1781,7 +1660,7 @@ static void put_xdp_frags(struct xdp_buff *xdp)
> >>                  shinfo = xdp_get_shared_info_from_buff(xdp);
> >>                  for (i = 0; i < shinfo->nr_frags; i++) {
> >>                          xdp_page = skb_frag_page(&shinfo->frags[i]);
> >> -                       put_page(xdp_page);
> >> +                       page_pool_put_page(rq->page_pool, xdp_page, -1, true);
> >>                  }
> >>          }
> >>   }
> >> @@ -1873,7 +1752,7 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >>          if (page_off + *len + tailroom > PAGE_SIZE)
> >>                  return NULL;
> >>
> >> -       page = alloc_page(GFP_ATOMIC);
> >> +       page = page_pool_alloc_pages(rq->page_pool, GFP_ATOMIC);
> >>          if (!page)
> >>                  return NULL;
> >>
> >> @@ -1896,8 +1775,12 @@ static struct page *xdp_linearize_page(struct net_device *dev,
> >>                  p = virt_to_head_page(buf);
> >>                  off = buf - page_address(p);
> >>
> >> +               if (rq->use_page_pool_dma)
> >> +                       page_pool_dma_sync_for_cpu(rq->page_pool, p,
> >> +                                                  off, buflen);
> >
> > Intresting, I think we need a patch for -stable to sync for cpu as
> > well (and probably the XDP_TX path).
>
> In the old code, I believed the sync was handled implicitly inside
> virtnet_rq_get_buf() -> virtnet_rq_unmap() before data was accessed.
> Could you point me at the specific path you're concerned about for
> -stable? I want to make sure I understand the issue correctly before
> sending a fix.

You're right; the code seems fine.

(Note that, when big_packets is true but small is used, we will go
virtqueue_get_buf(), so the unamp is done via deatch_buf().

>
> >
> >
> >> +
> >>                  if (check_mergeable_len(dev, ctx, buflen)) {
> >> -                       put_page(p);
> >> +                       page_pool_put_page(rq->page_pool, p, -1, true);
> >>                          goto err_buf;
> >>                  }
>
> Sorry for missing that comment in the earlier email

No worries.

Thanks

>