Re: [RFC PATCH net-next v1 4/4] net: page_pool: use netmem_t instead of struct page in API

On Wed, Jan 3, 2024 at 1:47 AM Yunsheng Lin [off-list ref] wrote:

On 2024/1/3 0:14, Mina Almasry wrote:

quoted

The idea being that skb_frag_page() can return NULL if the frag is not
paged, and the relevant callers are modified to handle that.

There are many existing drivers which are not expecting NULL returning for
skb_frag_page() as those drivers are not supporting devmem, adding additionl
checking overhead in skb_frag_page() for those drivers does not make much
sense, IMHO, it may make more sense to introduce a new helper for the driver
supporting devmem or networking core that needing dealing with both normal
page and devmem.

And we are also able to keep the old non-NULL returning semantic for
skb_frag_page().

I think I'm seeing agreement that the direction we're heading into
here is that most net stack & drivers should use the abstract netmem
type, and only specific code that needs a page or devmem (like
tcp_receive_zerocopy or tcp_recvmsg_dmabuf) will be the ones that
unpack the netmem and get the underlying page or devmem, using
skb_frag_page() or something like skb_frag_dmabuf(), etc.

As Jason says repeatedly, I'm not allowed to blindly cast a netmem to
a page and assume netmem==page. Netmem can only be cast to a page
after checking the low bits and verifying the netmem is actually a
page. I think any suggestions that blindly cast a netmem to page
without the checks will get nacked by Jason & Christian, so the
checking in the specific cases where the code needs to know the
underlying memory type seems necessary.

IMO I'm not sure the checking is expensive. With likely/unlikely &
static branches the checks should be very minimal or a straight no-op.
For example in RFC v2 where we were doing a lot of checks for devmem
(we don't do that anymore for RFCv5), I had run the page_pool perf
tests and proved there is little to no perf regression:

https://lore.kernel.org/netdev/CAHS8izM4w2UETAwfnV7w+ZzTMxLkz+FKO+xTgRdtYKzV8RzqXw@mail.gmail.com/ (local)

-- 
Thanks,
Mina