v5:
- Applied feedback from Eric to inline napi_pp_get_page().
- Applied Reviewed-By's.
v4:
- Rebased to net-next.
- Clarified skb_shift() code change in commit message.
- Use skb->pp_recycle in a couple of places where I previously hardcoded
'false'.
v3:
- Fixed patchwork build errors/warnings from patch-by-patch modallconfig
build
v2:
- Removed RFC tag.
- Rebased on net-next after the merge window opening.
- Added 1 patch at the beginning, "net: make napi_frag_unref reuse
skb_page_unref" because a recent patch introduced some code
duplication that can also be improved.
- Addressed feedback from Dragos & Yunsheng.
- Added Dragos's Reviewed-by.
This series is largely motivated by a recent discussion where there was
some confusion on how to properly ref/unref pp pages vs non pp pages:
https://lore.kernel.org/netdev/CAHS8izOoO-EovwMwAm9tLYetwikNPxC0FKyVGu1TPJWSz4bGoA@mail.gmail.com/T/#t
There is some subtely there because pp uses page->pp_ref_count for
refcounting, while non-pp uses get_page()/put_page() for ref counting.
Getting the refcounting pairs wrong can lead to kernel crash.
Additionally currently it may not be obvious to skb users unaware of
page pool internals how to properly acquire a ref on a pp frag. It
requires checking of skb->pp_recycle & is_pp_page() to make the correct
calls and may require some handling at the call site aware of arguable pp
internals.
This series is a minor refactor with a couple of goals:
1. skb users should be able to ref/unref a frag using
[__]skb_frag_[un]ref() functions without needing to understand pp
concepts and pp_ref_count vs get/put_page() differences.
2. reference counting functions should have a mirror opposite. I.e. there
should be a foo_unref() to every foo_ref() with a mirror opposite
implementation (as much as possible).
This is RFC to collect feedback if this change is desirable, but also so
that I don't race with the fix for the issue Dragos is seeing for his
crash.
https://lore.kernel.org/lkml/CAHS8izN436pn3SndrzsCyhmqvJHLyxgCeDpWXA4r1ANt3RCDLQ@mail.gmail.com/T/
Cc: Dragos Tatulea <[email protected]>
Mina Almasry (3):
net: make napi_frag_unref reuse skb_page_unref
net: mirror skb frag ref/unref helpers
net: remove napi_frag_unref
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
drivers/net/veth.c | 2 +-
include/linux/skbuff.h | 60 +++++++++++++------
include/net/page_pool/helpers.h | 5 --
net/core/skbuff.c | 48 ++-------------
net/ipv4/esp4.c | 2 +-
net/ipv6/esp6.c | 2 +-
net/tls/tls_device_fallback.c | 2 +-
9 files changed, 54 insertions(+), 73 deletions(-)
--
2.44.0.478.gd926399ef9-goog
The implementations of these 2 functions are almost identical. Remove
the implementation of napi_frag_unref, and make it a call into
skb_page_unref so we don't duplicate the implementation.
Signed-off-by: Mina Almasry <[email protected]>
Reviewed-by: Eric Dumazet <[email protected]>
---
include/linux/skbuff.h | 12 +++---------
net/ipv4/esp4.c | 2 +-
net/ipv6/esp6.c | 2 +-
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7dfb906d92f7..c0ff85bb087a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3522,10 +3522,10 @@ int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb,
bool napi_pp_put_page(struct page *page);
static inline void
-skb_page_unref(const struct sk_buff *skb, struct page *page)
+skb_page_unref(struct page *page, bool recycle)
{
#ifdef CONFIG_PAGE_POOL
- if (skb->pp_recycle && napi_pp_put_page(page))
+ if (recycle && napi_pp_put_page(page))
return;
#endif
put_page(page);
@@ -3534,13 +3534,7 @@ skb_page_unref(const struct sk_buff *skb, struct page *page)
static inline void
napi_frag_unref(skb_frag_t *frag, bool recycle)
{
- struct page *page = skb_frag_page(frag);
-
-#ifdef CONFIG_PAGE_POOL
- if (recycle && napi_pp_put_page(page))
- return;
-#endif
- put_page(page);
+ skb_page_unref(skb_frag_page(frag), recycle);
}
/**
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 3d647c9a7a21..40330253f076 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -114,7 +114,7 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb)
*/
if (req->src != req->dst)
for (sg = sg_next(req->src); sg; sg = sg_next(sg))
- skb_page_unref(skb, sg_page(sg));
+ skb_page_unref(sg_page(sg), skb->pp_recycle);
}
#ifdef CONFIG_INET_ESPINTCP
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index fe8d53f5a5ee..fb431d0a3475 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -131,7 +131,7 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb)
*/
if (req->src != req->dst)
for (sg = sg_next(req->src); sg; sg = sg_next(sg))
- skb_page_unref(skb, sg_page(sg));
+ skb_page_unref(sg_page(sg), skb->pp_recycle);
}
#ifdef CONFIG_INET6_ESPINTCP
--
2.44.0.478.gd926399ef9-goog
Refactor some of the skb frag ref/unref helpers for improved clarity.
Implement napi_pp_get_page() to be the mirror counterpart of
napi_pp_put_page().
Implement skb_page_ref() to be the mirror of skb_page_unref().
Improve __skb_frag_ref() to become a mirror counterpart of
__skb_frag_unref(). Previously unref could handle pp & non-pp pages,
while the ref could only handle non-pp pages. Now both the ref & unref
helpers can correctly handle both pp & non-pp pages.
Now that __skb_frag_ref() can handle both pp & non-pp pages, remove
skb_pp_frag_ref(), and use __skb_frag_ref() instead. This lets us
remove pp specific handling from skb_try_coalesce.
Additionally, since __skb_frag_ref() can now handle both pp & non-pp
pages, a latent issue in skb_shift() should now be fixed. Previously
this function would do a non-pp ref & pp unref on potential pp frags
(fragfrom). After this patch, skb_shift() should correctly do a pp
ref/unref on pp frags.
Signed-off-by: Mina Almasry <[email protected]>
Reviewed-by: Dragos Tatulea <[email protected]>
---
v5:
- Made changes to inline napi_pp_get_page() (Eric). I had to move
page_pool_ref_page() from include/net/page_pool/helpers.h to
include/linux/skbuff.h, so I don't add more includes to skbuff.h,
which slows down the incremental builds.
v4:
- pass skb->pp_recycle instead of 'false' in __skb_frag_ref in
chcr_ktls.c & cassini.c.
- Add some details on the changes to skb_shift() in this commit in the
commit message.
v3:
- Fix build errors reported by patchwork.
- Fix drivers/net/veth.c & tls_device_fallback.c callsite I missed to update.
- Fix page_pool_ref_page(head_page) -> page_pool_ref_page(page)
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
drivers/net/veth.c | 2 +-
include/linux/skbuff.h | 44 ++++++++++++++++--
include/net/page_pool/helpers.h | 5 --
net/core/skbuff.c | 46 ++-----------------
net/tls/tls_device_fallback.c | 2 +-
7 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index 6482728794dd..d7e8deafddf1 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1658,7 +1658,7 @@ static void chcr_ktls_copy_record_in_skb(struct sk_buff *nskb,
for (i = 0; i < record->num_frags; i++) {
skb_shinfo(nskb)->frags[i] = record->frags[i];
/* increase the frag ref count */
- __skb_frag_ref(&skb_shinfo(nskb)->frags[i]);
+ __skb_frag_ref(&skb_shinfo(nskb)->frags[i], nskb->pp_recycle);
}
skb_shinfo(nskb)->nr_frags = record->num_frags;
diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index bfb903506367..31878256feee 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1999,7 +1999,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
skb->len += hlen - swivel;
skb_frag_fill_page_desc(frag, page->buffer, off, hlen - swivel);
- __skb_frag_ref(frag);
+ __skb_frag_ref(frag, skb->pp_recycle);
/* any more data? */
if ((words[0] & RX_COMP1_SPLIT_PKT) && ((dlen -= hlen) > 0)) {
@@ -2023,7 +2023,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
frag++;
skb_frag_fill_page_desc(frag, page->buffer, 0, hlen);
- __skb_frag_ref(frag);
+ __skb_frag_ref(frag, skb->pp_recycle);
RX_USED_ADD(page, hlen + cp->crc_size);
}
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index bcdfbf61eb66..6160a3e8d341 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -716,7 +716,7 @@ static void veth_xdp_get(struct xdp_buff *xdp)
return;
for (i = 0; i < sinfo->nr_frags; i++)
- __skb_frag_ref(&sinfo->frags[i]);
+ __skb_frag_ref(&sinfo->frags[i], false);
}
static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c0ff85bb087a..2e6a77c398e6 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3492,15 +3492,51 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
return netmem_to_page(frag->netmem);
}
+#ifdef CONFIG_PAGE_POOL
+static inline bool is_pp_page(struct page *page)
+{
+ return (page->pp_magic & ~0x3UL) == PP_SIGNATURE;
+}
+
+/* page_pool_unref_page() lives in net/page_pool/helpers.h */
+static inline void page_pool_ref_page(struct page *page)
+{
+ atomic_long_inc(&page->pp_ref_count);
+}
+
+static inline bool napi_pp_get_page(struct page *page)
+{
+ page = compound_head(page);
+
+ if (!is_pp_page(page))
+ return false;
+
+ page_pool_ref_page(page);
+ return true;
+}
+#endif
+
+static inline void skb_page_ref(struct page *page, bool recycle)
+{
+#ifdef CONFIG_PAGE_POOL
+ if (recycle && napi_pp_get_page(page))
+ return;
+#endif
+ get_page(page);
+}
+
/**
* __skb_frag_ref - take an addition reference on a paged fragment.
* @frag: the paged fragment
+ * @recycle: skb->pp_recycle param of the parent skb. False if no parent skb.
*
- * Takes an additional reference on the paged fragment @frag.
+ * Takes an additional reference on the paged fragment @frag. Obtains the
+ * correct reference count depending on whether skb->pp_recycle is set and
+ * whether the frag is a page pool frag.
*/
-static inline void __skb_frag_ref(skb_frag_t *frag)
+static inline void __skb_frag_ref(skb_frag_t *frag, bool recycle)
{
- get_page(skb_frag_page(frag));
+ skb_page_ref(skb_frag_page(frag), recycle);
}
/**
@@ -3512,7 +3548,7 @@ static inline void __skb_frag_ref(skb_frag_t *frag)
*/
static inline void skb_frag_ref(struct sk_buff *skb, int f)
{
- __skb_frag_ref(&skb_shinfo(skb)->frags[f]);
+ __skb_frag_ref(&skb_shinfo(skb)->frags[f], skb->pp_recycle);
}
int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 1d397c1a0043..ead2c0d24b2c 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -277,11 +277,6 @@ static inline long page_pool_unref_page(struct page *page, long nr)
return ret;
}
-static inline void page_pool_ref_page(struct page *page)
-{
- atomic_long_inc(&page->pp_ref_count);
-}
-
static inline bool page_pool_is_last_ref(struct page *page)
{
/* If page_pool_unref_page() returns 0, we were the last user */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 21cd01641f4c..928c615b0fd8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -906,11 +906,6 @@ static void skb_clone_fraglist(struct sk_buff *skb)
skb_get(list);
}
-static bool is_pp_page(struct page *page)
-{
- return (page->pp_magic & ~0x3UL) == PP_SIGNATURE;
-}
-
int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
unsigned int headroom)
{
@@ -1032,37 +1027,6 @@ static bool skb_pp_recycle(struct sk_buff *skb, void *data)
return napi_pp_put_page(virt_to_page(data));
}
-/**
- * skb_pp_frag_ref() - Increase fragment references of a page pool aware skb
- * @skb: page pool aware skb
- *
- * Increase the fragment reference count (pp_ref_count) of a skb. This is
- * intended to gain fragment references only for page pool aware skbs,
- * i.e. when skb->pp_recycle is true, and not for fragments in a
- * non-pp-recycling skb. It has a fallback to increase references on normal
- * pages, as page pool aware skbs may also have normal page fragments.
- */
-static int skb_pp_frag_ref(struct sk_buff *skb)
-{
- struct skb_shared_info *shinfo;
- struct page *head_page;
- int i;
-
- if (!skb->pp_recycle)
- return -EINVAL;
-
- shinfo = skb_shinfo(skb);
-
- for (i = 0; i < shinfo->nr_frags; i++) {
- head_page = compound_head(skb_frag_page(&shinfo->frags[i]));
- if (likely(is_pp_page(head_page)))
- page_pool_ref_page(head_page);
- else
- page_ref_inc(head_page);
- }
- return 0;
-}
-
static void skb_kfree_head(void *head, unsigned int end_offset)
{
if (end_offset == SKB_SMALL_HEAD_HEADROOM)
@@ -4175,7 +4139,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
to++;
} else {
- __skb_frag_ref(fragfrom);
+ __skb_frag_ref(fragfrom, skb->pp_recycle);
skb_frag_page_copy(fragto, fragfrom);
skb_frag_off_copy(fragto, fragfrom);
skb_frag_size_set(fragto, todo);
@@ -4825,7 +4789,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
}
*nskb_frag = (i < 0) ? skb_head_frag_to_page_desc(frag_skb) : *frag;
- __skb_frag_ref(nskb_frag);
+ __skb_frag_ref(nskb_frag, nskb->pp_recycle);
size = skb_frag_size(nskb_frag);
if (pos < offset) {
@@ -5956,10 +5920,8 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
/* if the skb is not cloned this does nothing
* since we set nr_frags to 0.
*/
- if (skb_pp_frag_ref(from)) {
- for (i = 0; i < from_shinfo->nr_frags; i++)
- __skb_frag_ref(&from_shinfo->frags[i]);
- }
+ for (i = 0; i < from_shinfo->nr_frags; i++)
+ __skb_frag_ref(&from_shinfo->frags[i], from->pp_recycle);
to->truesize += delta;
to->len += len;
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 4e7228f275fa..d4000b4a1f7d 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -277,7 +277,7 @@ static int fill_sg_in(struct scatterlist *sg_in,
for (i = 0; remaining > 0; i++) {
skb_frag_t *frag = &record->frags[i];
- __skb_frag_ref(frag);
+ __skb_frag_ref(frag, false);
sg_set_page(sg_in + i, skb_frag_page(frag),
skb_frag_size(frag), skb_frag_off(frag));
--
2.44.0.478.gd926399ef9-goog
With the changes in the last patches, napi_frag_unref() is now
reduandant. Remove it and use skb_page_unref directly.
Signed-off-by: Mina Almasry <[email protected]>
Reviewed-by: Dragos Tatulea <[email protected]>
Reviewed-by: Eric Dumazet <[email protected]>
---
include/linux/skbuff.h | 8 +-------
net/core/skbuff.c | 2 +-
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e6a77c398e6..182371f4834d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3567,12 +3567,6 @@ skb_page_unref(struct page *page, bool recycle)
put_page(page);
}
-static inline void
-napi_frag_unref(skb_frag_t *frag, bool recycle)
-{
- skb_page_unref(skb_frag_page(frag), recycle);
-}
-
/**
* __skb_frag_unref - release a reference on a paged fragment.
* @frag: the paged fragment
@@ -3583,7 +3577,7 @@ napi_frag_unref(skb_frag_t *frag, bool recycle)
*/
static inline void __skb_frag_unref(skb_frag_t *frag, bool recycle)
{
- napi_frag_unref(frag, recycle);
+ skb_page_unref(skb_frag_page(frag), recycle);
}
/**
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 928c615b0fd8..38c20b44cb14 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1065,7 +1065,7 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
}
for (i = 0; i < shinfo->nr_frags; i++)
- napi_frag_unref(&shinfo->frags[i], skb->pp_recycle);
+ __skb_frag_unref(&shinfo->frags[i], skb->pp_recycle);
free_head:
if (shinfo->frag_list)
--
2.44.0.478.gd926399ef9-goog
On 4/8/2024 8:29 AM, Mina Almasry wrote:
> The implementations of these 2 functions are almost identical. Remove
> the implementation of napi_frag_unref, and make it a call into
> skb_page_unref so we don't duplicate the implementation.
>
> Signed-off-by: Mina Almasry <[email protected]>
> Reviewed-by: Eric Dumazet <[email protected]>
>
Reviewed-by: Jacob Keller <[email protected]>
On 4/8/2024 8:29 AM, Mina Almasry wrote:
> Refactor some of the skb frag ref/unref helpers for improved clarity.
>
> Implement napi_pp_get_page() to be the mirror counterpart of
> napi_pp_put_page().
>
> Implement skb_page_ref() to be the mirror of skb_page_unref().
>
> Improve __skb_frag_ref() to become a mirror counterpart of
> __skb_frag_unref(). Previously unref could handle pp & non-pp pages,
> while the ref could only handle non-pp pages. Now both the ref & unref
> helpers can correctly handle both pp & non-pp pages.
>
> Now that __skb_frag_ref() can handle both pp & non-pp pages, remove
> skb_pp_frag_ref(), and use __skb_frag_ref() instead. This lets us
> remove pp specific handling from skb_try_coalesce.
>
> Additionally, since __skb_frag_ref() can now handle both pp & non-pp
> pages, a latent issue in skb_shift() should now be fixed. Previously
> this function would do a non-pp ref & pp unref on potential pp frags
> (fragfrom). After this patch, skb_shift() should correctly do a pp
> ref/unref on pp frags.
>
The description sounds like a lot is going on here, and at first I was
thinking this should be split further. However I think that would just
end up with a lot of needless thrash.
Reviewed-by: Jacob Keller <[email protected]>
On 4/8/2024 8:29 AM, Mina Almasry wrote:
> With the changes in the last patches, napi_frag_unref() is now
> reduandant. Remove it and use skb_page_unref directly.
>
> Signed-off-by: Mina Almasry <[email protected]>
> Reviewed-by: Dragos Tatulea <[email protected]>
> Reviewed-by: Eric Dumazet <[email protected]>
>
Reviewed-by: Jacob Keller <[email protected]>
On Mon, 8 Apr 2024 08:29:57 -0700 Mina Almasry wrote:
> +#ifdef CONFIG_PAGE_POOL
> +static inline bool is_pp_page(struct page *page)
> +{
> + return (page->pp_magic & ~0x3UL) == PP_SIGNATURE;
> +}
> +
> +/* page_pool_unref_page() lives in net/page_pool/helpers.h */
> +static inline void page_pool_ref_page(struct page *page)
> +{
> + atomic_long_inc(&page->pp_ref_count);
> +}
> +
> +static inline bool napi_pp_get_page(struct page *page)
> +{
> + page = compound_head(page);
> +
> + if (!is_pp_page(page))
> + return false;
> +
> + page_pool_ref_page(page);
> + return true;
> +}
> +#endif
> +
> +static inline void skb_page_ref(struct page *page, bool recycle)
> +{
> +#ifdef CONFIG_PAGE_POOL
> + if (recycle && napi_pp_get_page(page))
> + return;
> +#endif
> + get_page(page);
> +}
Shifting of all this code from pp to skbuff catches the eye.
There aren't that many callers to these, can we start a new header?
We can then include page pool headers in the without worry.
I'll apply the other patches, they look independent.
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:
On Mon, 8 Apr 2024 08:29:55 -0700 you wrote:
> v5:
> - Applied feedback from Eric to inline napi_pp_get_page().
> - Applied Reviewed-By's.
>
> v4:
> - Rebased to net-next.
> - Clarified skb_shift() code change in commit message.
> - Use skb->pp_recycle in a couple of places where I previously hardcoded
> 'false'.
>
> [...]
Here is the summary with links:
- [net-next,v5,1/3] net: make napi_frag_unref reuse skb_page_unref
https://git.kernel.org/netdev/net-next/c/959fa5c188bf
- [net-next,v5,2/3] net: mirror skb frag ref/unref helpers
(no matching commit)
- [net-next,v5,3/3] net: remove napi_frag_unref
https://git.kernel.org/netdev/net-next/c/f58f3c956340
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On Tue, Apr 9, 2024 at 6:23 PM Jakub Kicinski <[email protected]> wrote:
>
> On Mon, 8 Apr 2024 08:29:57 -0700 Mina Almasry wrote:
> > +#ifdef CONFIG_PAGE_POOL
> > +static inline bool is_pp_page(struct page *page)
> > +{
> > + return (page->pp_magic & ~0x3UL) == PP_SIGNATURE;
> > +}
> > +
> > +/* page_pool_unref_page() lives in net/page_pool/helpers.h */
> > +static inline void page_pool_ref_page(struct page *page)
> > +{
> > + atomic_long_inc(&page->pp_ref_count);
> > +}
> > +
> > +static inline bool napi_pp_get_page(struct page *page)
> > +{
> > + page = compound_head(page);
> > +
> > + if (!is_pp_page(page))
> > + return false;
> > +
> > + page_pool_ref_page(page);
> > + return true;
> > +}
> > +#endif
> > +
> > +static inline void skb_page_ref(struct page *page, bool recycle)
> > +{
> > +#ifdef CONFIG_PAGE_POOL
> > + if (recycle && napi_pp_get_page(page))
> > + return;
> > +#endif
> > + get_page(page);
> > +}
>
> Shifting of all this code from pp to skbuff catches the eye.
> There aren't that many callers to these, can we start a new header?
I am not 100% sure what the new header should be named and what
exactly you wanted moved to the new header, but made a guess and
uploaded v6 with this change. Let me know if it's not what you had in
mind. I added a new linux/skbuff_ref.h header file, moved all the ref
functions there, and included page pool headers in that without worry.
v6: https://patchwork.kernel.org/project/netdevbpf/list/?series=843380
> We can then include page pool headers in the without worry.
>
> I'll apply the other patches, they look independent.
--
Thanks,
Mina