Re: [PATCH net-next 6/6] mvneta: recycle buffers

Lorenzo Bianconi
2021-03-24T09:29:44Z

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

[...]
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index a635cf84608a..8b3250394703 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -2332,7 +2332,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> > if (!skb)
> > return ERR_PTR(-ENOMEM);
> >
> > - page_pool_release_page(rxq->page_pool, virt_to_page(xdp->data));
> > + skb_mark_for_recycle(skb, virt_to_page(xdp->data), &xdp->rxq->mem);
> >
> > skb_reserve(skb, xdp->data - xdp->data_hard_start);
> > skb_put(skb, xdp->data_end - xdp->data);
> > @@ -2344,7 +2344,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> > skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> > skb_frag_page(frag), skb_frag_off(frag),
> > skb_frag_size(frag), PAGE_SIZE);
> > - page_pool_release_page(rxq->page_pool, skb_frag_page(frag));
> > + skb_mark_for_recycle(skb, skb_frag_page(frag), &xdp->rxq->mem);
> > }
> >
> > return skb;
>
> This cause skb_mark_for_recycle() to set 'skb->pp_recycle=1' multiple
> times, for the same SKB. (copy-pasted function below signature to help
> reviewers).
>
> This makes me question if we need an API for setting this per page
> fragment?
> Or if the API skb_mark_for_recycle() need to walk the page fragments in
> the SKB and set the info stored in the page for each?

Considering just performances, I guess it is better open-code here since the
driver already performs a loop over fragments to build the skb, but I guess
this approach is quite risky and I would prefer to have a single utility
routine to take care of linear area + fragments. What do you think?

Regards,
Lorenzo

>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]