On Mon, Jul 27, 2020 at 03:44:34PM -0700, Jonathan Lemon wrote:
> From: Jonathan Lemon <b...@fb.com>
>
> netgpu pages will always have a refcount of at least one (held by
> the netgpu module). If the skb is marked as containing netgpu ZC
> pages, recycle them back to netgpu.
What???
>
> Signed-off-by: Jonathan Lemon <jonathan.le...@gmail.com>
> ---
> net/core/skbuff.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 1422b99b7090..50dbb7ce1965 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -591,6 +591,27 @@ static void skb_free_head(struct sk_buff *skb)
> kfree(head);
> }
>
> +#if IS_ENABLED(CONFIG_NETGPU)
> +static void skb_netgpu_unref(struct skb_shared_info *shinfo)
> +{
> + struct netgpu_ifq *ifq = shinfo->destructor_arg;
> + struct page *page;
> + int i;
> +
> + /* pages attached for skbs for TX shouldn't come here, since
> + * the skb is not marked as "zc_netgpu". (only RX skbs have this).
> + * dummy page does come here, but always has elevated refc.
> + *
> + * Undelivered zc skb's will arrive at this point.
> + */
> + for (i = 0; i < shinfo->nr_frags; i++) {
> + page = skb_frag_page(&shinfo->frags[i]);
> + if (page && page_ref_dec_return(page) <= 2)
> + netgpu_put_page(ifq, page, false);
> + }
> +}
> +#endif
Becides the basic "no #if in C files" issue here, why is this correct?
> +
> static void skb_release_data(struct sk_buff *skb)
> {
> struct skb_shared_info *shinfo = skb_shinfo(skb);
> @@ -601,8 +622,15 @@ static void skb_release_data(struct sk_buff *skb)
> &shinfo->dataref))
> return;
>
> - for (i = 0; i < shinfo->nr_frags; i++)
> - __skb_frag_unref(&shinfo->frags[i]);
> +#if IS_ENABLED(CONFIG_NETGPU)
> + if (skb->zc_netgpu && shinfo->nr_frags) {
> + skb_netgpu_unref(shinfo);
> + } else
> +#endif
> + {
> + for (i = 0; i < shinfo->nr_frags; i++)
> + __skb_frag_unref(&shinfo->frags[i]);
> + }
Again, no #if in C code. But even then, this feels really really wrong.
greg k-h
