Re: [net-next,v3] wireguard: allowedips: Use kfree_rcu() instead of call_rcu()

[Jakub Kicinski]

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
---
wireguard: allowedips: Use kfree_rcu() instead of call_rcu()

This commit replaces call_rcu() with a custom kmem_cache_free() callback
with kfree_rcu() to simplify the code. The change affects how nodes are
freed in the remove_node() function.

> diff --git a/drivers/net/wireguard/allowedips.c b/drivers/net/wireguard/allowedips.c
> index 09f7fcd7da78..5ece9acad64d 100644
> --- a/drivers/net/wireguard/allowedips.c
> +++ b/drivers/net/wireguard/allowedips.c
> @@ -48,11 +48,6 @@ static void push_rcu(struct allowedips_node **stack,
>  	}
>  }
>
> -static void node_free_rcu(struct rcu_head *rcu)
> -{
> -	kmem_cache_free(node_cache, container_of(rcu, struct allowedips_node, rcu));
> -}
> -

[ ... ]

> @@ -271,13 +266,13 @@ static void remove_node(struct allowedips_node *node, struct mutex *lock)
>  	if (free_parent)
>  		child = rcu_dereference_protected(parent->bit[!(node->parent_bit_packed & 1)],
>  						  lockdep_is_held(lock));
> -	call_rcu(&node->rcu, node_free_rcu);
> +	kfree_rcu(node, rcu);

Does wg_allowedips_slab_uninit() need to be updated to use
kvfree_rcu_barrier() instead of rcu_barrier()?

When CONFIG_KVFREE_RCU_BATCHED is enabled (the default), kfree_rcu()
uses a batched mechanism that queues work via queue_rcu_work(). The
rcu_barrier() call waits for RCU callbacks to complete, but these
callbacks only queue the actual free to a workqueue via rcu_work_rcufn().
The workqueue work that calls kvfree() may still be pending after
rcu_barrier() returns.

The existing cleanup path is:
  wg_allowedips_slab_uninit() -> rcu_barrier() -> kmem_cache_destroy()

With kfree_rcu(), this sequence could destroy the slab cache while
kfree_rcu_work() still has pending frees queued. The proper barrier for
kfree_rcu() is kvfree_rcu_barrier() which also calls flush_rcu_work()
on all pending batches.

>  	if (!free_parent)
>  		return;
>  	if (child)
>  		child->parent_bit_packed = parent->parent_bit_packed;
>  	*(struct allowedips_node **)(parent->parent_bit_packed & ~3UL) = child;
> -	call_rcu(&parent->rcu, node_free_rcu);
> +	kfree_rcu(parent, rcu);
>  }
-- 
pw-bot: cr