Re: [PATCH net-next v2 1/6] netmem: rename __get/__put_netmem() to get/put_net_iov()

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new kernel-doc for get_netmem() and put_netmem() in
  include/net/netmem.h is wrong for one of the two net_iov types.

--

netmem: rename __get/__put_netmem() to get/put_net_iov()

This commit renames __get_netmem()/__put_netmem() to get_net_iov() and
put_net_iov(), changing them to take a struct net_iov pointer. It adds
kernel-doc to get_netmem()/put_netmem() and the new helpers, and adds a
DEBUG_NET_WARN_ON_ONCE() for net_iov types without non-pp reference
support.

> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 0cc8572f62caf..697d50c51a5ee 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -399,21 +399,37 @@ static inline bool net_is_devmem_iov(const struct net_iov *niov)
>  }
>  #endif
>  
> -void __get_netmem(netmem_ref netmem);
> -void __put_netmem(netmem_ref netmem);
> +void get_net_iov(struct net_iov *niov);
> +void put_net_iov(struct net_iov *niov);
>  
> +/**
> + * get_netmem - acquire a non-page_pool reference on a netmem
> + * @netmem: netmem to reference
> + *
> + * Acquires a non-pp backing reference (page._refcount via get_page() or
> + * binding->ref via get_net_iov()) regardless of page_pool membership.
> + * Counterpart to put_netmem().
> + */

[Severity: Low]
Is this kernel-doc accurate for every net_iov type?

get_net_iov() only takes a reference when net_is_devmem_iov() is true,
which means the NET_IOV_DMABUF type:

net/core/skbuff.c:get_net_iov() {
	if (net_is_devmem_iov(niov))
		net_devmem_get_net_iov(niov);
	else
		DEBUG_NET_WARN_ON_ONCE(true);
}

For NET_IOV_IOURING (io_uring zcrx) niovs, and for all niovs when
CONFIG_NET_DEVMEM=n, neither get_netmem() nor put_netmem() takes or drops
any reference.

Without CONFIG_DEBUG_NET, DEBUG_NET_WARN_ON_ONCE() expands to
BUILD_BUG_ON_INVALID(). On production kernels that case is skipped
silently, so the "warning if @niov does not support non-pp references"
in the get_net_iov()/put_net_iov() kernel-doc never fires there.

The commit message has a similar statement:

    Make the non-pp layer explicit and symmetric across both
    netmem types: get/put_netmem() dispatches to get/put_page() for pages
    and get/put_net_iov() for net_iovs, always acquiring or releasing a
    non-pp reference regardless of page_pool membership.

This doesn't change runtime behavior, since __get_netmem()/__put_netmem()
also did nothing for io_uring niovs. A future caller that relies on this
kernel-doc could still pair get_netmem()/put_netmem() on a zcrx niov and
get no lifetime protection.

Could the get_netmem()/put_netmem() kernel-doc say that NET_IOV_IOURING
has no non-pp refcount, so these helpers do nothing for it? It could also
say that the warning only exists under CONFIG_DEBUG_NET.

>  static __always_inline void get_netmem(netmem_ref netmem)
>  {
>  	if (netmem_is_net_iov(netmem))
> -		__get_netmem(netmem);
> +		get_net_iov(netmem_to_net_iov(netmem));
>  	else
>  		get_page(netmem_to_page(netmem));
>  }
>  
> +/**
> + * put_netmem - release a non-page_pool reference on a netmem
> + * @netmem: netmem to unreference
> + *
> + * Drops a non-pp backing reference (page._refcount via put_page() or
> + * binding->ref via put_net_iov()) regardless of page_pool membership.
> + * Counterpart to get_netmem().
> + */

The same question applies to put_netmem(). put_net_iov() only calls
net_devmem_put_net_iov() for NET_IOV_DMABUF niovs.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012082148.1698596-1-almasrymina%40google.com