Subject: Re: [PATCH net-next v3 4/6] enic: Use the Page Pool API for RX when MTU is less than page size

From: Jakub Kicinski <[email-masked]>
Date: Mon, 30 Dec 2024 08:44:49 -0800

On Mon, 30 Dec 2024 17:18:39 +0800 Yunsheng Lin wrote:
> On 2024/12/28 8:10, John Daley wrote:
> > +void enic_rq_free_page(struct vnic_rq *vrq, struct vnic_rq_buf *buf)
> > +{
> > +	struct enic *enic = vnic_dev_priv(vrq->vdev);
> > +	struct enic_rq *rq = &enic->rq[vrq->index];
> > +
> > +	if (!buf->os_buf)
> > +		return;
> > +
> > +	page_pool_put_page(rq->pool, (struct page *)buf->os_buf,
> > +			   get_max_pkt_len(enic), true);  
> 
> It seems the above has a similar problem of not using
> page_pool_put_full_page() when page_pool_dev_alloc() API is used and
> page_pool is created with PP_FLAG_DMA_SYNC_DEV flags.
> 
> It seems like a common mistake that a WARN_ON might be needed to catch
> this kind of problem.

Agreed. Maybe also add an alias to page_pool_put_full_page() called
something like page_pool_dev_put_page() to correspond to the alloc
call? I suspect people don't understand the internals and "releasing
full page" feels wrong when they only allocated a portion..