Re: [PATCH net-next] page_pool: check if nmdesc->pp is !NULL to confirm its usage as pp for net_iov
From: Mina Almasry <hidden>
Date: 2025-10-18 15:06:05
Also in:
io-uring, lkml
On Fri, Oct 17, 2025 at 11:47 PM Byungchul Park [off-list ref] wrote:
On Fri, Oct 17, 2025 at 08:13:14AM -0700, Mina Almasry wrote:quoted
On Fri, Oct 17, 2025 at 5:32 AM Pavel Begunkov [off-list ref] wrote:quoted
On 10/16/25 08:21, Byungchul Park wrote:quoted
On Thu, Oct 16, 2025 at 03:36:57PM +0900, Byungchul Park wrote:quoted
->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 don't think it simplify merging anyhow, hmm?+1 honestly. If you want to 'extract the networking bits' into its own patch, let it be a patch series where this is a patch doing pre-work, and the next patches in the series are adding the page_flag.Okay. Then is it possible that one for mm tree and the other for net-next in the same patch series? I've never tried patches that way.quoted
I don't want added netmem_is_net_iov checks unnecessarily tbh. These checks are bad and only used when absolutely necessary, so let the patch series that adds them also do something useful (i.e. add the page flag), if possible. But I honestly think this patch was almost good as-is:Hm.. but the following patch includes both networking changes and mm changes. Jakub thinks it should go to mm and I don't know how Andrew thinks it should be. It's not clear even to me. That's why I splitted it into two, and this is the networking part, and I will post the mm part to mm folks later. Any suggestions?
I think 1 series with all the mm and networking changes targeting mm and Cc: netdev@ would work (and lets see if Andrew prefers something different). The networking changes are very small. -- Thanks, Mina