Re: [PATCH 10/14] can: gw: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

Oliver Hartkopp
2024-06-10T13:10:37Z

On 09.06.24 10:27, Julia Lawall wrote:
> Since SLOB was removed, it is not necessary to use call_rcu
> when the callback only performs kmem_cache_free. Use
> kfree_rcu() directly.
>
> The changes were done using the following Coccinelle semantic patch.
> This semantic patch is designed to ignore cases where the callback
> function is used in another way.
>
> // <smpl>
> @r@
> expression e;
> local idexpression e2;
> identifier cb,f;
> position p;
> @@
>
> (
> call_rcu(...,e2)
> |
> call_rcu(&e->f,cb@p)
> )
>
> @r1@
> type T;
> identifier x,r.cb;
> @@
>
> cb(...) {
> (
> kmem_cache_free(...);
> |
> T x = ...;
> kmem_cache_free(...,x);
> |
> T x;
> x = ...;
> kmem_cache_free(...,x);
> )
> }
>
> @s depends on r1@
> position p != r.p;
> identifier r.cb;
> @@
>
> cb@p
>
> @script:ocaml@
> cb << r.cb;
> p << s.p;
> @@
>
> Printf.eprintf "Other use of %s at %s:%d\n"
> cb (List.hd p).file (List.hd p).line
>
> @depends on r1 && !s@
> expression e;
> identifier r.cb,f;
> position r.p;
> @@
>
> - call_rcu(&e->f,cb@p)
> + kfree_rcu(e,f)
>
> @r1a depends on !s@
> type T;
> identifier x,r.cb;
> @@
>
> - cb(...) {
> (
> - kmem_cache_free(...);
> |
> - T x = ...;
> - kmem_cache_free(...,x);
> |
> - T x;
> - x = ...;
> - kmem_cache_free(...,x);
> )
> - }
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

For net/can/gw.c

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Thanks Julia!

>
> ---
> net/can/gw.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/net/can/gw.c b/net/can/gw.c
> index 37528826935e..ffb9870e2d01 100644
> --- a/net/can/gw.c
> +++ b/net/can/gw.c
> @@ -577,13 +577,6 @@ static inline void cgw_unregister_filter(struct net *net, struct cgw_job *gwj)
> gwj->ccgw.filter.can_mask, can_can_gw_rcv, gwj);
> }
>
> -static void cgw_job_free_rcu(struct rcu_head *rcu_head)
> -{
> - struct cgw_job *gwj = container_of(rcu_head, struct cgw_job, rcu);
> -
> - kmem_cache_free(cgw_cache, gwj);
> -}
> -
> static int cgw_notifier(struct notifier_block *nb,
> unsigned long msg, void *ptr)
> {
> @@ -603,7 +596,7 @@ static int cgw_notifier(struct notifier_block *nb,
> if (gwj->src.dev == dev || gwj->dst.dev == dev) {
> hlist_del(&gwj->list);
> cgw_unregister_filter(net, gwj);
> - call_rcu(&gwj->rcu, cgw_job_free_rcu);
> + kfree_rcu(gwj, rcu);
> }
> }
> }
> @@ -1168,7 +1161,7 @@ static void cgw_remove_all_jobs(struct net *net)
> hlist_for_each_entry_safe(gwj, nx, &net->can.cgw_list, list) {
> hlist_del(&gwj->list);
> cgw_unregister_filter(net, gwj);
> - call_rcu(&gwj->rcu, cgw_job_free_rcu);
> + kfree_rcu(gwj, rcu);
> }
> }
>
> @@ -1236,7 +1229,7 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh,
>
> hlist_del(&gwj->list);
> cgw_unregister_filter(net, gwj);
> - call_rcu(&gwj->rcu, cgw_job_free_rcu);
> + kfree_rcu(gwj, rcu);
> err = 0;
> break;
> }
>
>