Re: [PATCH net-next] page_pool: check if nmdesc->pp is !NULL to confirm its usage as pp for net_iov

[Byungchul Park <byungchul@sk.com>]

On Fri, Oct 17, 2025 at 01:33:43PM +0100, Pavel Begunkov wrote:
> On 10/16/25 08:21, Byungchul Park wrote:
> > On Thu, Oct 16, 2025 at 03:36:57PM +0900, Byungchul Park wrote:
> > > ->pp_magic field in struct page is current used to identify if a page
> > > belongs to a page pool.  However, ->pp_magic will be removed and page
> > > type bit in struct page e.g. PGTY_netpp should be used for that purpose.
> > > 
> > > As a preparation, the check for net_iov, that is not page-backed, should
> > > avoid using ->pp_magic since net_iov doens't have to do with page type.
> > > Instead, nmdesc->pp can be used if a net_iov or its nmdesc belongs to a
> > > page pool, by making sure nmdesc->pp is NULL otherwise.
> > > 
> > > For page-backed netmem, just leave unchanged as is, while for net_iov,
> > > make sure nmdesc->pp is initialized to NULL and use nmdesc->pp for the
> > > check.
> > 
> > IIRC,
> > 
> > Suggested-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> Pointing out a problem in a patch with a fix doesn't qualify to
> me as "suggested-by", you don't need to worry about that.
> 
> Did you get the PGTY bits merged? There is some uneasiness about
> this patch as it does nothing good by itself, it'd be much better
> to have it in a series finalising the page_pool conversion. And

I also considered that.  However, I think the finalizing e.i. removing
pp fields in struct page, would better be done once every thing else has
been ready, so that I can focus on examining more thoroughly if there
aren't accesses through struct page and it can be fianlized safely :-)

	Byungchul

> I don't think it simplify merging anyhow, hmm?
> 
> ...>> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
> > > index 723e4266b91f..cf78227c0ca6 100644
> > > --- a/io_uring/zcrx.c
> > > +++ b/io_uring/zcrx.c
> > > @@ -450,6 +450,10 @@ static int io_zcrx_create_area(struct io_zcrx_ifq *ifq,
> > >              area->freelist[i] = i;
> > >              atomic_set(&area->user_refs[i], 0);
> > >              niov->type = NET_IOV_IOURING;
> > > +
> > > +            /* niov->desc.pp is already initialized to NULL by
> > > +             * kvmalloc_array(__GFP_ZERO).
> > > +             */
> 
> Please drop this hunk if you'll be resubmitting, it's not
> needed.
> 
> --
> Pavel Begunkov