[net-next,v15,11/14] net: add SO_DEVMEM_DONTNEED setsockopt to release RX frags
Commit Message
Add an interface for the user to notify the kernel that it is done
reading the devmem dmabuf frags returned as cmsg. The kernel will
drop the reference on the frags to make them available for reuse.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
v10:
- Fix leak of tokens (Nikolay).
v7:
- Updated SO_DEVMEM_* uapi to use the next available entry (Arnd).
v6:
- Squash in locking optimizations from edumazet@google.com. With his
changes we lock the xarray once per sock_devmem_dontneed operation
rather than once per frag.
Changes in v1:
- devmemtoken -> dmabuf_token (David).
- Use napi_pp_put_page() for refcounting (Yunsheng).
- Fix build error with missing socket options on other asms.
---
arch/alpha/include/uapi/asm/socket.h | 1 +
arch/mips/include/uapi/asm/socket.h | 1 +
arch/parisc/include/uapi/asm/socket.h | 1 +
arch/sparc/include/uapi/asm/socket.h | 1 +
include/uapi/asm-generic/socket.h | 1 +
include/uapi/linux/uio.h | 4 ++
net/core/sock.c | 61 +++++++++++++++++++++++++++
7 files changed, 70 insertions(+)
Comments
On Fri, Jun 28, 2024 at 2:33 AM Mina Almasry <almasrymina@google.com> wrote:
>
> Add an interface for the user to notify the kernel that it is done
> reading the devmem dmabuf frags returned as cmsg. The kernel will
> drop the reference on the frags to make them available for reuse.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>
>
> ---
>
> v10:
> - Fix leak of tokens (Nikolay).
>
> v7:
> - Updated SO_DEVMEM_* uapi to use the next available entry (Arnd).
>
> v6:
> - Squash in locking optimizations from edumazet@google.com. With his
> changes we lock the xarray once per sock_devmem_dontneed operation
> rather than once per frag.
>
> Changes in v1:
> - devmemtoken -> dmabuf_token (David).
> - Use napi_pp_put_page() for refcounting (Yunsheng).
> - Fix build error with missing socket options on other asms.
>
> ---
> arch/alpha/include/uapi/asm/socket.h | 1 +
> arch/mips/include/uapi/asm/socket.h | 1 +
> arch/parisc/include/uapi/asm/socket.h | 1 +
> arch/sparc/include/uapi/asm/socket.h | 1 +
> include/uapi/asm-generic/socket.h | 1 +
> include/uapi/linux/uio.h | 4 ++
> net/core/sock.c | 61 +++++++++++++++++++++++++++
> 7 files changed, 70 insertions(+)
>
>
> +struct dmabuf_token {
> + __u32 token_start;
> + __u32 token_count;
> +};
> /*
> * UIO_MAXIOV shall be at least 16 1003.1g (5.4.1.1)
> */
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 9abc4fe259535..040c66ac26244 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -124,6 +124,7 @@
> #include <linux/netdevice.h>
> #include <net/protocol.h>
> #include <linux/skbuff.h>
> +#include <linux/skbuff_ref.h>
> #include <net/net_namespace.h>
> #include <net/request_sock.h>
> #include <net/sock.h>
> @@ -1049,6 +1050,62 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
> return 0;
> }
>
> +#ifdef CONFIG_PAGE_POOL
> +static noinline_for_stack int
> +sock_devmem_dontneed(struct sock *sk, sockptr_t optval, unsigned int optlen)
> +{
> + unsigned int num_tokens, i, j, k, netmem_num = 0;
> + struct dmabuf_token *tokens;
> + netmem_ref netmems[16];
> + int ret = 0;
> +
> + if (sk->sk_type != SOCK_STREAM || sk->sk_protocol != IPPROTO_TCP)
> + return -EBADF;
This might use sk_is_tcp() helper.
> +
> + if (optlen % sizeof(struct dmabuf_token) ||
> + optlen > sizeof(*tokens) * 128)
> + return -EINVAL;
> +
> + tokens = kvmalloc_array(128, sizeof(*tokens), GFP_KERNEL);
This allocates 8192 bytes even for small optlen ?
Probably no big deal, no need to send a new version.
Reviewed-by: Eric Dumazet <edumazet@google.com>
On 6/28/24 01:32, Mina Almasry wrote:
> Add an interface for the user to notify the kernel that it is done
> reading the devmem dmabuf frags returned as cmsg. The kernel will
> drop the reference on the frags to make them available for reuse.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>
Same, lost tag from v13
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
And, as a follow up, would be great to clean up the loop.
Helper functions and "continue" should help to bring the
indention down.
On 6/27/24 6:32 PM, Mina Almasry wrote:
> @@ -1049,6 +1050,62 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
> return 0;
> }
>
> +#ifdef CONFIG_PAGE_POOL
> +static noinline_for_stack int
> +sock_devmem_dontneed(struct sock *sk, sockptr_t optval, unsigned int optlen)
> +{
> + unsigned int num_tokens, i, j, k, netmem_num = 0;
> + struct dmabuf_token *tokens;
> + netmem_ref netmems[16];
> + int ret = 0;
> +
> + if (sk->sk_type != SOCK_STREAM || sk->sk_protocol != IPPROTO_TCP)
> + return -EBADF;
> +
> + if (optlen % sizeof(struct dmabuf_token) ||
> + optlen > sizeof(*tokens) * 128)
> + return -EINVAL;
> +
> + tokens = kvmalloc_array(128, sizeof(*tokens), GFP_KERNEL);
The '128' should be a named macro with a comment on why that value.
@@ -144,6 +144,7 @@
#define SCM_DEVMEM_LINEAR SO_DEVMEM_LINEAR
#define SO_DEVMEM_DMABUF 79
#define SCM_DEVMEM_DMABUF SO_DEVMEM_DMABUF
+#define SO_DEVMEM_DONTNEED 80
#if !defined(__KERNEL__)
@@ -155,6 +155,7 @@
#define SCM_DEVMEM_LINEAR SO_DEVMEM_LINEAR
#define SO_DEVMEM_DMABUF 79
#define SCM_DEVMEM_DMABUF SO_DEVMEM_DMABUF
+#define SO_DEVMEM_DONTNEED 80
#if !defined(__KERNEL__)
@@ -136,6 +136,7 @@
#define SCM_DEVMEM_LINEAR SO_DEVMEM_LINEAR
#define SO_DEVMEM_DMABUF 79
#define SCM_DEVMEM_DMABUF SO_DEVMEM_DMABUF
+#define SO_DEVMEM_DONTNEED 80
#if !defined(__KERNEL__)
@@ -137,6 +137,7 @@
#define SCM_DEVMEM_LINEAR SO_DEVMEM_LINEAR
#define SO_DEVMEM_DMABUF 0x0058
#define SCM_DEVMEM_DMABUF SO_DEVMEM_DMABUF
+#define SO_DEVMEM_DONTNEED 0x0059
#if !defined(__KERNEL__)
@@ -135,6 +135,7 @@
#define SO_PASSPIDFD 76
#define SO_PEERPIDFD 77
+#define SO_DEVMEM_DONTNEED 97
#define SO_DEVMEM_LINEAR 98
#define SCM_DEVMEM_LINEAR SO_DEVMEM_LINEAR
#define SO_DEVMEM_DMABUF 99
@@ -33,6 +33,10 @@ struct dmabuf_cmsg {
*/
};
+struct dmabuf_token {
+ __u32 token_start;
+ __u32 token_count;
+};
/*
* UIO_MAXIOV shall be at least 16 1003.1g (5.4.1.1)
*/
@@ -124,6 +124,7 @@
#include <linux/netdevice.h>
#include <net/protocol.h>
#include <linux/skbuff.h>
+#include <linux/skbuff_ref.h>
#include <net/net_namespace.h>
#include <net/request_sock.h>
#include <net/sock.h>
@@ -1049,6 +1050,62 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
return 0;
}
+#ifdef CONFIG_PAGE_POOL
+static noinline_for_stack int
+sock_devmem_dontneed(struct sock *sk, sockptr_t optval, unsigned int optlen)
+{
+ unsigned int num_tokens, i, j, k, netmem_num = 0;
+ struct dmabuf_token *tokens;
+ netmem_ref netmems[16];
+ int ret = 0;
+
+ if (sk->sk_type != SOCK_STREAM || sk->sk_protocol != IPPROTO_TCP)
+ return -EBADF;
+
+ if (optlen % sizeof(struct dmabuf_token) ||
+ optlen > sizeof(*tokens) * 128)
+ return -EINVAL;
+
+ tokens = kvmalloc_array(128, sizeof(*tokens), GFP_KERNEL);
+ if (!tokens)
+ return -ENOMEM;
+
+ num_tokens = optlen / sizeof(struct dmabuf_token);
+ if (copy_from_sockptr(tokens, optval, optlen)) {
+ kvfree(tokens);
+ return -EFAULT;
+ }
+
+ xa_lock_bh(&sk->sk_user_frags);
+ for (i = 0; i < num_tokens; i++) {
+ for (j = 0; j < tokens[i].token_count; j++) {
+ netmem_ref netmem = (__force netmem_ref)__xa_erase(
+ &sk->sk_user_frags, tokens[i].token_start + j);
+
+ if (netmem &&
+ !WARN_ON_ONCE(!netmem_is_net_iov(netmem))) {
+ netmems[netmem_num++] = netmem;
+ if (netmem_num == ARRAY_SIZE(netmems)) {
+ xa_unlock_bh(&sk->sk_user_frags);
+ for (k = 0; k < netmem_num; k++)
+ WARN_ON_ONCE(!napi_pp_put_page(netmems[k]));
+ netmem_num = 0;
+ xa_lock_bh(&sk->sk_user_frags);
+ }
+ ret++;
+ }
+ }
+ }
+
+ xa_unlock_bh(&sk->sk_user_frags);
+ for (k = 0; k < netmem_num; k++)
+ WARN_ON_ONCE(!napi_pp_put_page(netmems[k]));
+
+ kvfree(tokens);
+ return ret;
+}
+#endif
+
void sockopt_lock_sock(struct sock *sk)
{
/* When current->bpf_ctx is set, the setsockopt is called from
@@ -1211,6 +1268,10 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
ret = -EOPNOTSUPP;
return ret;
}
+#ifdef CONFIG_PAGE_POOL
+ case SO_DEVMEM_DONTNEED:
+ return sock_devmem_dontneed(sk, optval, optlen);
+#endif
}
sockopt_lock_sock(sk);