Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Pavel Begunkov <asml.silence@gmail.com>]

On 7/10/25 19:11, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>>
>> To eliminate the use of the page pool fields in struct page, the page
>> pool code should use netmem descriptor and APIs instead.
>>
>> However, some code e.g. __netmem_to_page() is still used to access the
>> page pool fields e.g. ->pp via struct page, which should be changed so
>> as to access them via netmem descriptor, struct netmem_desc instead,
>> since the fields no longer will be available in struct page.
>>
>> Introduce utility APIs to make them easy to use struct netmem_desc as
>> descriptor.  The APIs are:
>>
>>     1. __netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
>>        but unsafely without checking if it's net_iov or system memory.
>>
>>     2. netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
>>        safely with checking if it's net_iov or system memory.
>>
>>     3. nmdesc_to_page(), to convert struct netmem_desc to struct page,
>>        assuming struct netmem_desc overlays on struct page.
>>
>>     4. page_to_nmdesc(), to convert struct page to struct netmem_desc,
>>        assuming struct netmem_desc overlays on struct page, allowing only
>>        head page to be converted.
>>
>>     5. nmdesc_adress(), to get its virtual address corresponding to the
>>        struct netmem_desc.
>>
>> Signed-off-by: Byungchul Park <byungchul@sk.com>
>> ---
>>   include/net/netmem.h | 41 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>>
>> diff --git a/include/net/netmem.h b/include/net/netmem.h
>> index 535cf17b9134..ad9444be229a 100644
>> --- a/include/net/netmem.h
>> +++ b/include/net/netmem.h
>> @@ -198,6 +198,32 @@ static inline struct page *netmem_to_page(netmem_ref netmem)
>>          return __netmem_to_page(netmem);
>>   }
>>
>> +/**
>> + * __netmem_to_nmdesc - unsafely get pointer to the &netmem_desc backing
>> + * @netmem
>> + * @netmem: netmem reference to convert
>> + *
>> + * Unsafe version of netmem_to_nmdesc(). When @netmem is always backed
>> + * by system memory, performs faster and generates smaller object code
>> + * (no check for the LSB, no WARN). When @netmem points to IOV, provokes
>> + * undefined behaviour.
>> + *
>> + * Return: pointer to the &netmem_desc (garbage if @netmem is not backed
>> + * by system memory).
>> + */
>> +static inline struct netmem_desc *__netmem_to_nmdesc(netmem_ref netmem)
>> +{
>> +       return (__force struct netmem_desc *)netmem;
>> +}
>> +
> 
> Does a netmem_desc represent the pp fields shared between struct page
> and struct net_iov, or does netmem_desc represent paged kernel memory?
> If the former, I don't think we need a safe and unsafe version of this
> helper, since netmem_ref always has netmem_desc fields underneath. If
> the latter, then this helper should not exist at all. We should not
> allow casting netmem_ref to a netmem_desc without first checking if
> it's a net_iov.

+1, and...

> 
> To be honest the cover letter should come up with a detailed
> explanation of (a) what are the current types (b) what are the new
> types (c) what are the relationships between the types, so these
> questions stop coming up.
> 
>> +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
>> +{
>> +       if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
>> +               return NULL;

... specifically this function should work with net_iov.

-- 
Pavel Begunkov