On 7/11/25 02:02, Byungchul Park wrote:
On Thu, Jul 10, 2025 at 11:11:51AM -0700, Mina Almasry wrote:
...>>> +#define nmdesc_to_page(nmdesc) (_Generic((nmdesc), \
+ const struct netmem_desc * : (const struct page *)(nmdesc), \
+ struct netmem_desc * : (struct page *)(nmdesc)))
+
+static inline struct netmem_desc *page_to_nmdesc(struct page *page)
+{
+ VM_BUG_ON_PAGE(PageTail(page), page);
+ return (struct netmem_desc *)page;
+}
+
It's not safe to cast a page to netmem_desc, without first checking if
it's a pp page or not, otherwise you may be casting random non-pp
pages to netmem_desc...
I'd suggest to rename it to sth like pp_page_to_nmdesc() and add:
DEBUG_NET_WARN_ON_ONCE(!page_pool_page_is_pp(page));
...
Agree, but page_to_nmdesc() will be used in page_pool_page_is_pp() to
check if it's a pp page or not:
static inline bool page_pool_page_is_pp(struct page *page)
{
struct netmem_desc *desc = page_to_nmdesc(page);
return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
}
Hm.. maybe, it'd be better to resore the original code and remove this
page_to_nmdesc() helper. FYI, the original code was:
static inline bool page_pool_page_is_pp(struct page *page)
{
struct netmem_desc *desc = (struct netmem_desc *)page;
return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
}
... And use this version. It's supposed to be temporary anyway.
It'd be great to add a build check that the page still has space
to alias with for now.
--
Pavel Begunkov