From: Jakub Kicinski <kuba@kernel•org>
To: bobbyeshleman@gmail•com
Cc: Jakub Kicinski <kuba@kernel•org>,
horms@kernel•org, linux-arch@vger•kernel.org, pabeni@redhat•com,
willemb@google•com, davem@davemloft•net,
linux-doc@vger•kernel.org, donald.hunter@gmail•com,
matttbe@kernel•org, linux-kernel@vger•kernel.org,
asml.silence@gmail•com, arnd@arndb•de, netdev@vger•kernel.org,
almasrymina@google•com, linux-kselftest@vger•kernel.org,
skhawaja@google•com, bobbyeshleman@meta•com,
ncardwell@google•com, andrew+netdev@lunn•ch, kuniyu@google•com,
edumazet@google•com, sdf@fomichev•me, corbet@lwn•net,
dsahern@kernel•org, shuah@kernel•org
Subject: Re: [net-next,v9,3/5] net: devmem: implement autorelease token management
Date: Mon, 12 Jan 2026 20:00:27 -0800 [thread overview]
Message-ID: <20260113040027.409308-1-kuba@kernel.org> (raw)
In-Reply-To: <20260109-scratch-bobbyeshleman-devmem-tcp-token-upstream-v9-3-8042930d00d7@meta.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least pausible.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: devmem: implement autorelease token management
This commit adds autorelease mode toggling for devmem tokens using a
static branch. The system is locked into the mode set by the first
binding and can only change when all bindings are released.
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 05a9a9e7abb9..05c16df657c7 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
[ ... ]
> @@ -292,25 +333,62 @@ net_devmem_bind_dmabuf(struct net_device *dev,
[ ... ]
> + mutex_lock(&devmem_ar_lock);
> +
> + if (direction == DMA_FROM_DEVICE) {
> + if (net_devmem_dmabuf_rx_bindings_count > 0) {
> + bool mode;
> +
> + mode = static_key_enabled(&tcp_devmem_ar_key);
> +
> + /* When bindings exist, enforce that the mode does not
> + * change.
> + */
> + if (mode != autorelease) {
> + NL_SET_ERR_MSG_FMT(extack,
> + "System already configured with autorelease=%d",
> + mode);
> + err = -EINVAL;
> + goto err_unlock_mutex;
> + }
> + } else if (autorelease) {
> + /* First binding with autorelease enabled sets the
> + * mode. If autorelease is false, the key is already
> + * disabled by default so no action is needed.
> + */
> + static_branch_enable(&tcp_devmem_ar_key);
> + }
> +
> + net_devmem_dmabuf_rx_bindings_count++;
> + }
> +
> + err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id,
> + binding, xa_limit_32b, &id_alloc_next,
> + GFP_KERNEL);
> + if (err < 0)
> + goto err_dec_binding_count;
> +
> + mutex_unlock(&devmem_ar_lock);
> +
> + list_add(&binding->list, &priv->bindings);
> +
> + return binding;
> +
> +err_dec_binding_count:
> + if (direction == DMA_FROM_DEVICE)
> + net_devmem_dmabuf_rx_bindings_count--;
^^^
If xa_alloc_cyclic() fails after the first binding with autorelease=true,
the static branch tcp_devmem_ar_key remains enabled even though
net_devmem_dmabuf_rx_bindings_count is decremented back to zero. Should
the error path also call static_branch_disable() when the count returns
to zero, similar to what __net_devmem_dmabuf_binding_free() does?
This appears to leave the system in an inconsistent state where a
subsequent binding with autorelease=false would succeed (since count is
zero) but the static branch is still enabled from the failed attempt.
The SO_DEVMEM_DONTNEED handler would then call the wrong function
(autorelease path instead of manual release path).
> +
> +err_unlock_mutex:
> + mutex_unlock(&devmem_ar_lock);
[ ... ]
--
pw-bot: cr
next prev parent reply other threads:[~2026-01-13 4:01 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-10 2:18 [PATCH net-next v9 0/5] net: devmem: improve cpu cost of RX token management Bobby Eshleman
2026-01-10 2:18 ` [PATCH net-next v9 1/5] net: devmem: rename tx_vec to vec in dmabuf binding Bobby Eshleman
2026-01-10 2:18 ` [PATCH net-next v9 2/5] net: devmem: refactor sock_devmem_dontneed for autorelease split Bobby Eshleman
2026-01-10 2:18 ` [PATCH net-next v9 3/5] net: devmem: implement autorelease token management Bobby Eshleman
2026-01-11 19:12 ` Mina Almasry
2026-01-11 19:27 ` Mina Almasry
2026-01-12 17:42 ` Bobby Eshleman
2026-01-12 16:24 ` Bobby Eshleman
2026-01-13 19:27 ` Mina Almasry
2026-01-13 20:32 ` Bobby Eshleman
2026-01-13 20:42 ` Mina Almasry
2026-01-14 3:18 ` Bobby Eshleman
2026-01-14 17:04 ` Bobby Eshleman
2026-01-14 20:54 ` Stanislav Fomichev
2026-01-14 21:25 ` Bobby Eshleman
2026-01-13 4:00 ` Jakub Kicinski [this message]
2026-01-10 2:18 ` [PATCH net-next v9 4/5] net: devmem: document NETDEV_A_DMABUF_AUTORELEASE netlink attribute Bobby Eshleman
2026-01-11 19:14 ` Mina Almasry
2026-01-10 2:18 ` [PATCH net-next v9 5/5] selftests: drv-net: devmem: add autorelease test Bobby Eshleman
2026-01-11 19:16 ` Mina Almasry
2026-01-12 19:56 ` Bobby Eshleman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260113040027.409308-1-kuba@kernel.org \
--to=kuba@kernel$(echo .)org \
--cc=almasrymina@google$(echo .)com \
--cc=andrew+netdev@lunn$(echo .)ch \
--cc=arnd@arndb$(echo .)de \
--cc=asml.silence@gmail$(echo .)com \
--cc=bobbyeshleman@gmail$(echo .)com \
--cc=bobbyeshleman@meta$(echo .)com \
--cc=corbet@lwn$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=donald.hunter@gmail$(echo .)com \
--cc=dsahern@kernel$(echo .)org \
--cc=edumazet@google$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=kuniyu@google$(echo .)com \
--cc=linux-arch@vger$(echo .)kernel.org \
--cc=linux-doc@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-kselftest@vger$(echo .)kernel.org \
--cc=matttbe@kernel$(echo .)org \
--cc=ncardwell@google$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=sdf@fomichev$(echo .)me \
--cc=shuah@kernel$(echo .)org \
--cc=skhawaja@google$(echo .)com \
--cc=willemb@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox