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

[Oliver Hartkopp <socketcan@xxxxxxxxxxxx>]

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@xxxxxxxx>
> Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>

For net/can/gw.c

Acked-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>

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;
>   	}