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
