| Message ID | 20240626100008.831849-1-sagi@grimberg.me (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d2d30a376d9cc94c6fb730c58b3e5b7426ecb6de |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v4] net: allow skb_datagram_iter to be called from any context | expand |
On Wed, 2024-06-26 at 13:00 +0300, Sagi Grimberg wrote: > We only use the mapping in a single context, so kmap_local is sufficient > and cheaper. Make sure to use skb_frag_foreach_page as skb frags may > contain highmem compound pages and we need to map page by page. My understanding of the discussion on previous revision is that Matthew prefers dropping the reference to 'highmem': https://lore.kernel.org/netdev/Zn1-2QVyOJe_y6l1@casper.infradead.org/#t To avoid a repost, I can drop it while applying the patch, if there is agreement. Thanks, Paolo
On 02/07/2024 12:51, Paolo Abeni wrote: > On Wed, 2024-06-26 at 13:00 +0300, Sagi Grimberg wrote: >> We only use the mapping in a single context, so kmap_local is sufficient >> and cheaper. Make sure to use skb_frag_foreach_page as skb frags may >> contain highmem compound pages and we need to map page by page. > My understanding of the discussion on previous revision is that Matthew > prefers dropping the reference to 'highmem': > > https://lore.kernel.org/netdev/Zn1-2QVyOJe_y6l1@casper.infradead.org/#t > > To avoid a repost, I can drop it while applying the patch, if there is > agreement. Agree, would be preferable avoiding another repost if possible.
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Wed, 26 Jun 2024 13:00:08 +0300 you wrote: > We only use the mapping in a single context, so kmap_local is sufficient > and cheaper. Make sure to use skb_frag_foreach_page as skb frags may > contain highmem compound pages and we need to map page by page. > > Reported-by: kernel test robot <oliver.sang@intel.com> > Closes: https://lore.kernel.org/oe-lkp/202406161539.b5ff7b20-oliver.sang@intel.com > Fixes: 950fcaecd5cc ("datagram: consolidate datagram copy to iter helpers") > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > > [...] Here is the summary with links: - [net,v4] net: allow skb_datagram_iter to be called from any context https://git.kernel.org/netdev/net/c/d2d30a376d9c You are awesome, thank you!
diff --git a/net/core/datagram.c b/net/core/datagram.c index e614cfd8e14a..e9ba4c7b449d 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -416,15 +416,22 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset, end = start + skb_frag_size(frag); if ((copy = end - offset) > 0) { - struct page *page = skb_frag_page(frag); - u8 *vaddr = kmap(page); + u32 p_off, p_len, copied; + struct page *p; + u8 *vaddr; if (copy > len) copy = len; - n = INDIRECT_CALL_1(cb, simple_copy_to_iter, - vaddr + skb_frag_off(frag) + offset - start, - copy, data, to); - kunmap(page); + + skb_frag_foreach_page(frag, + skb_frag_off(frag) + offset - start, + copy, p, p_off, p_len, copied) { + vaddr = kmap_local_page(p); + n = INDIRECT_CALL_1(cb, simple_copy_to_iter, + vaddr + p_off, p_len, data, to); + kunmap_local(vaddr); + } + offset += n; if (n != copy) goto short_copy;
We only use the mapping in a single context, so kmap_local is sufficient and cheaper. Make sure to use skb_frag_foreach_page as skb frags may contain highmem compound pages and we need to map page by page. Reported-by: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202406161539.b5ff7b20-oliver.sang@intel.com Fixes: 950fcaecd5cc ("datagram: consolidate datagram copy to iter helpers") Signed-off-by: Sagi Grimberg <sagi@grimberg.me> --- Changes from v3: - Add a fixes tag Changes from v2: - added a target tree in subject prefix - added reported credits and closes annotation Changes from v1: - Fix usercopy BUG() due to copy from highmem pages across page boundary by using skb_frag_foreach_page net/core/datagram.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)