On Wed, 14 Apr 2021 13:09:47 -0700
Shakeel Butt [off-list ref] wrote:

On Wed, Apr 14, 2021 at 12:42 PM Jesper Dangaard Brouer
[off-list ref] wrote:

quoted

[...]

quoted

quoted

quoted

Can this page_pool be used for TCP RX zerocopy? If yes then PageType
can not be used.  

Yes it can, since it's going to be used as your default allocator for
payloads, which might end up on an SKB.  

I'm not sure we want or should "allow" page_pool be used for TCP RX
zerocopy.
For several reasons.

(1) This implies mapping these pages page to userspace, which AFAIK
means using page->mapping and page->index members (right?).
 

No, only page->_mapcount is used.

Good to know.
I will admit that I don't fully understand the usage of page->mapping
and page->index members.

quoted

(2) It feels wrong (security wise) to keep the DMA-mapping (for the
device) and also map this page into userspace.
 

I think this is already the case i.e pages still DMA-mapped and also
mapped into userspace.

True, other drivers are doing the same.

quoted

(3) The page_pool is optimized for refcnt==1 case, and AFAIK TCP-RX
zerocopy will bump the refcnt, which means the page_pool will not
recycle the page when it see the elevated refcnt (it will instead
release its DMA-mapping).  

Yes this is right but the userspace might have already consumed and
unmapped the page before the driver considers to recycle the page.

That is a good point.  So, there is a race window where it is possible
to gain recycling.

It seems my page_pool co-maintainer Ilias is interested in taking up the
challenge to get this working with TCP RX zerocopy.  So, lets see how
this is doable.

 

quoted

(4) I remember vaguely that this code path for (TCP RX zerocopy) uses
page->private for tricks.  And our patch [3/5] use page->private for
storing xdp_mem_info.

IMHO when the SKB travel into this TCP RX zerocopy code path, we should
call page_pool_release_page() to release its DMA-mapping.
 

I will let TCP RX zerocopy experts respond to this but from my high
level code inspection, I didn't see page->private usage.

I trust when Eric says page->private isn't used in this code path.
So, it might actually be possible :-)

I will challenge Ilias and Matteo to pull this off. (But I know that
both of them are busy for personal reasons, so be patient with them).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer