| Message ID | 20240609082726.32742-11-Julia.Lawall@inria.fr (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | replace call_rcu by kfree_rcu for simple kmem_cache_free callback | expand |
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; > } > >
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; }
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> --- net/can/gw.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)