Re: [EXTERNAL] Re: [PATCH] mm: page_frag: Fix refill handling in __page_frag_alloc_align()

[Yunsheng Lin <yunshenglin0825@gmail.com>]

On 3/2/2025 7:16 AM, Haiyang Zhang wrote:

...

>> We are already aware of this, and have error checking in place for the
>> failover
>> case to "base page".
>>
>>  From the discussion thread above, there are other drivers using
>> page_frag_alloc_align() for over PAGE_SIZE too. If making the page_frag
>> API
>> support only fragsz <= PAGE_SIZE is desired, can we create another API?
>> One
>> keeps the existing API semantics (allowing > PAGE_SIZE), the other uses
>> your new code. By the way, it should add an explicit check and fail ALL
>> requests
>> for fragsz > PAGE_SIZE. Currently your code successfully allocates big
>> frags
>> for a few times, then fail. This is not a desired behavior. It's also a
>> breaking change for our MANA driver, which can no longer run Jumbo frames.

It seems there was some memory corruption problem that may caused by
reuse of the previously allocated frag cache memory by following a
LARGER allocations before 'using initial zero offset'.

https://lore.kernel.org/netdev/b711ca5f-4180-d658-a330-89bd5dcb0acb@gmail.com/T/#m12ebdefb8ee653b281d477d2310218b4ac138cde

'using initial zero offset' seems to just make the API misuse problem
more obvious.

>>
>> @Andrew Morton <akpm@linux-foundation.org>
>> And other maintainers, could you please also evaluate the idea above?
>>
> 
> And, quote from current doc 6.14.0-rc4:
> "A page fragment is an arbitrary-length arbitrary-offset area of memory
> which resides within a 0 or higher order compound page."
> https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/mm/page_frags.rst
> 
> So, it is designed to be *arbitrary-length* within a 0 or higher order
> compound page.
> 
> If the commit 8218f62c9c9b ("mm: page_frag: use initial zero offset for
> page_frag_alloc_align()") intended to change the existing API semantics
> to be Page Frag Length <= PAGE_SIZE, the document and all breaking drivers
> need to be updated.

Yes, updating the Documemntation to make it more obvious seems like the
right thing to do.

> 
> Thanks,
> - Haiyang
>