On 5/28/25 11:44, Byungchul Park wrote:
On Wed, May 28, 2025 at 10:51:29AM +0100, Pavel Begunkov wrote:
On 5/28/25 10:33, Byungchul Park wrote:
On Wed, May 28, 2025 at 10:20:29AM +0100, Pavel Begunkov wrote:
On 5/28/25 10:14, Byungchul Park wrote:
On Wed, May 28, 2025 at 10:07:52AM +0100, Pavel Begunkov wrote:
On 5/28/25 09:14, Byungchul Park wrote:
On Wed, May 28, 2025 at 08:51:47AM +0100, Pavel Begunkov wrote:
On 5/26/25 03:23, Byungchul Park wrote:
On Fri, May 23, 2025 at 10:21:17AM -0700, Mina Almasry wrote:
On Thu, May 22, 2025 at 8:26 PM Byungchul Park <byungchul@xxxxxx> wrote:
To simplify struct page, the effort to seperate its own descriptor from
struct page is required and the work for page pool is on going.
To achieve that, all the code should avoid accessing page pool members
of struct page directly, but use safe APIs for the purpose.
Use netmem_is_pp() instead of directly accessing page->pp_magic in
page_pool_page_is_pp().
Signed-off-by: Byungchul Park <byungchul@xxxxxx>
---
include/linux/mm.h | 5 +----
net/core/page_pool.c | 5 +++++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8dc012e84033..3f7c80fb73ce 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4312,10 +4312,7 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
#define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL)
#ifdef CONFIG_PAGE_POOL
-static inline bool page_pool_page_is_pp(struct page *page)
-{
- return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
-}
I vote for keeping this function as-is (do not convert it to netmem),
and instead modify it to access page->netmem_desc->pp_magic.
Once the page pool fields are removed from struct page, struct page will
have neither struct netmem_desc nor the fields..
So it's unevitable to cast it to netmem_desc in order to refer to
pp_magic. Again, pp_magic is no longer associated to struct page.
Thoughts?
Once the indirection / page shrinking is realized, the page is
supposed to have a type field, isn't it? And all pp_magic trickery
will be replaced with something like
page_pool_page_is_pp() { return page->type == PAGE_TYPE_PP; }
Agree, but we need a temporary solution until then. I will use the
following way for now:
The question is what is the problem that you need another temporary
solution? If, for example, we go the placeholder way, page_pool_page_is_pp()
I prefer using the place-holder, but Matthew does not. I explained it:
https://lore.kernel.org/all/20250528013145.GB2986@xxxxxxxxxxxxxxxxxxx/
Now, I'm going with the same way as the other approaches e.g. ptdesc.
Sure, but that doesn't change my point
What's your point? The other appoaches do not use place-holders. I
don't get your point.
As I told you, I will introduce a new struct, netmem_desc, instead of
struct_group_tagged() on struct net_iov, and modify the static assert on
the offsets to keep the important fields between struct page and
netmem_desc.
Then, is that following your point? Or could you explain your point in
more detail? Did you say other points than these?
Then please read the message again first. I was replying to th
aliasing with "lru", and even at the place you cut the message it
says "for example", which was followed by "You should be able to
do the same with the overlay option.".
With struct_group_tagged() on struct net_iov, no idea about how to.
However, it's doable with a new separate struct, struct netmem_desc.
static inline bool page_pool_page_is_pp(struct page *page)
{
pp_magic = page_to_netdesc(page)->pp_magic;
return pp_magic == ...;
}
page_to_netdesc() is either casting directly in case of full page
overlays, or "&page->netdesc" for the placeholder option.
--
Pavel Begunkov
