| Message ID | 20220207190706.1499190-1-eric.dumazet@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] can: gw: use call_rcu() instead of costly synchronize_rcu() | expand |
On 07.02.22 20:07, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Commit fb8696ab14ad ("can: gw: synchronize rcu operations > before removing gw job entry") added three synchronize_rcu() calls > to make sure one rcu grace period was observed before freeing > a "struct cgw_job" (which are tiny objects). > > This should be converted to call_rcu() to avoid adding delays > in device / network dismantles. > > Use the rcu_head that was already in struct cgw_job, > not yet used. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Oliver Hartkopp <socketcan@hartkopp.net> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net> Thanks Eric! > --- > net/can/gw.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/net/can/gw.c b/net/can/gw.c > index d8861e862f157aec36c417b71eb7e8f59bd064b9..20e74fe7d0906dccc65732b8f9e7e14e2d1192c3 100644 > --- a/net/can/gw.c > +++ b/net/can/gw.c > @@ -577,6 +577,13 @@ 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) > { > @@ -596,8 +603,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); > - synchronize_rcu(); > - kmem_cache_free(cgw_cache, gwj); > + call_rcu(&gwj->rcu, cgw_job_free_rcu); > } > } > } > @@ -1155,8 +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); > - synchronize_rcu(); > - kmem_cache_free(cgw_cache, gwj); > + call_rcu(&gwj->rcu, cgw_job_free_rcu); > } > } > > @@ -1224,8 +1229,7 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh, > > hlist_del(&gwj->list); > cgw_unregister_filter(net, gwj); > - synchronize_rcu(); > - kmem_cache_free(cgw_cache, gwj); > + call_rcu(&gwj->rcu, cgw_job_free_rcu); > err = 0; > break; > }
On Mon, 7 Feb 2022 11:07:06 -0800 Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Commit fb8696ab14ad ("can: gw: synchronize rcu operations > before removing gw job entry") added three synchronize_rcu() calls > to make sure one rcu grace period was observed before freeing > a "struct cgw_job" (which are tiny objects). > > This should be converted to call_rcu() to avoid adding delays > in device / network dismantles. > > Use the rcu_head that was already in struct cgw_job, > not yet used. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Oliver Hartkopp <socketcan@hartkopp.net> Adding Marc and linux-can to CC.
On 08.02.2022 20:27:31, Oliver Hartkopp wrote: > > > On 07.02.22 20:07, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@google.com> > > > > Commit fb8696ab14ad ("can: gw: synchronize rcu operations > > before removing gw job entry") added three synchronize_rcu() calls > > to make sure one rcu grace period was observed before freeing > > a "struct cgw_job" (which are tiny objects). > > > > This should be converted to call_rcu() to avoid adding delays > > in device / network dismantles. > > > > Use the rcu_head that was already in struct cgw_job, > > not yet used. > > > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > Cc: Oliver Hartkopp <socketcan@hartkopp.net> > > Tested-by: Oliver Hartkopp <socketcan@hartkopp.net> Applied to linux-can-next/testing. There are several more synchronize_rcu() in net/can. Oliver, can you create similar patches for those? regards, Marc
On 07.02.2022 11:07:06, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Commit fb8696ab14ad ("can: gw: synchronize rcu operations > before removing gw job entry") added three synchronize_rcu() calls > to make sure one rcu grace period was observed before freeing > a "struct cgw_job" (which are tiny objects). > > This should be converted to call_rcu() to avoid adding delays > in device / network dismantles. > > Use the rcu_head that was already in struct cgw_job, > not yet used. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Oliver Hartkopp <socketcan@hartkopp.net> Added to linux-can-next/testing. Thanks, Marc
diff --git a/net/can/gw.c b/net/can/gw.c index d8861e862f157aec36c417b71eb7e8f59bd064b9..20e74fe7d0906dccc65732b8f9e7e14e2d1192c3 100644 --- a/net/can/gw.c +++ b/net/can/gw.c @@ -577,6 +577,13 @@ 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) { @@ -596,8 +603,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); - synchronize_rcu(); - kmem_cache_free(cgw_cache, gwj); + call_rcu(&gwj->rcu, cgw_job_free_rcu); } } } @@ -1155,8 +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); - synchronize_rcu(); - kmem_cache_free(cgw_cache, gwj); + call_rcu(&gwj->rcu, cgw_job_free_rcu); } } @@ -1224,8 +1229,7 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh, hlist_del(&gwj->list); cgw_unregister_filter(net, gwj); - synchronize_rcu(); - kmem_cache_free(cgw_cache, gwj); + call_rcu(&gwj->rcu, cgw_job_free_rcu); err = 0; break; }