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

[Lorenzo Bianconi]

[...]
> > 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: signature.asc
Description: PGP signature