| Message ID | 20220220035739.577181-1-eric.dumazet@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] gro_cells: avoid using synchronize_rcu() in gro_cells_destroy() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | fail | Errors and warnings before: 2 this patch: 4 |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 18 this patch: 20 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 7 this patch: 9 |
| netdev/checkpatch | warning | WARNING: Block comments should align the * on each line |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Sat, Feb 19, 2022 at 7:57 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > From: Eric Dumazet <edumazet@google.com> > > Another thing making netns dismantles potentially very slow is located > in gro_cells_destroy(), > whenever cleanup_net() has to remove a device using gro_cells framework. > > RTNL is not held at this stage, so synchronize_net() > is calling synchronize_rcu(): > > netdev_run_todo() > ip_tunnel_dev_free() > gro_cells_destroy() > synchronize_net() > synchronize_rcu() // Ouch. > > This patch uses call_rcu(), and gave me a 25x performance improvement > in my tests. > > cleanup_net() is no longer blocked ~10 ms per synchronize_rcu() > call. > > In the case we could not allocate the memory needed to queue the > deferred free, use synchronize_rcu_expedited() > > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > net/core/gro_cells.c | 36 +++++++++++++++++++++++++++++++----- > 1 file changed, 31 insertions(+), 5 deletions(-) > > diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c > index 6eb2e5ec2c5068e1d798557e55d084b785187a9b..46fa7d93fd9696755efd56b72731f08e821042e1 100644 > --- a/net/core/gro_cells.c > +++ b/net/core/gro_cells.c > @@ -89,8 +89,23 @@ int gro_cells_init(struct gro_cells *gcells, struct net_device *dev) > } > EXPORT_SYMBOL(gro_cells_init); > > +struct percpu_free_defer { > + struct rcu_head rcu; > + void __percpu *ptr; > +}; > + > +void percpu_free_defer_callback(struct rcu_head *head) This will get a static in v2 > +{ > + struct percpu_free_defer *defer; > + > + defer = container_of(head, struct percpu_free_defer, rcu); > + free_percpu(defer->ptr); > + kfree(defer); > +} > + > void gro_cells_destroy(struct gro_cells *gcells) > { > + struct percpu_free_defer *defer; > int i; > > if (!gcells->cells) > @@ -102,12 +117,23 @@ void gro_cells_destroy(struct gro_cells *gcells) > __netif_napi_del(&cell->napi); > __skb_queue_purge(&cell->napi_skbs); > } > - /* This barrier is needed because netpoll could access dev->napi_list > - * under rcu protection. > + /* We need to observe an rcu grace period before freeing ->cells, > + * because netpoll could access dev->napi_list under rcu protection. > + * Try hard using call_rcu() instead of synchronize_rcu(), > + * because we might be called from cleanup_net(), and we > + * definitely do not want to block this critical task. > */ > - synchronize_net(); > - > - free_percpu(gcells->cells); > + defer = kmalloc(sizeof(*defer), GFP_KERNEL | __GFP_NOWARN); > + if (likely(defer)) { > + defer->ptr = gcells->cells; > + call_rcu(&defer->rcu, percpu_free_defer_callback); > + } else { > + /* We do not hold RTNL at this point, synchronize_net() > + * would not be able to expedite this sync. > + */ I also will fix the comment alignment. > + synchronize_rcu_expedited(); > + free_percpu(gcells->cells); > + } > gcells->cells = NULL; > } > EXPORT_SYMBOL(gro_cells_destroy); > -- > 2.35.1.473.g83b2b277ed-goog >
diff --git a/net/core/gro_cells.c b/net/core/gro_cells.c index 6eb2e5ec2c5068e1d798557e55d084b785187a9b..46fa7d93fd9696755efd56b72731f08e821042e1 100644 --- a/net/core/gro_cells.c +++ b/net/core/gro_cells.c @@ -89,8 +89,23 @@ int gro_cells_init(struct gro_cells *gcells, struct net_device *dev) } EXPORT_SYMBOL(gro_cells_init); +struct percpu_free_defer { + struct rcu_head rcu; + void __percpu *ptr; +}; + +void percpu_free_defer_callback(struct rcu_head *head) +{ + struct percpu_free_defer *defer; + + defer = container_of(head, struct percpu_free_defer, rcu); + free_percpu(defer->ptr); + kfree(defer); +} + void gro_cells_destroy(struct gro_cells *gcells) { + struct percpu_free_defer *defer; int i; if (!gcells->cells) @@ -102,12 +117,23 @@ void gro_cells_destroy(struct gro_cells *gcells) __netif_napi_del(&cell->napi); __skb_queue_purge(&cell->napi_skbs); } - /* This barrier is needed because netpoll could access dev->napi_list - * under rcu protection. + /* We need to observe an rcu grace period before freeing ->cells, + * because netpoll could access dev->napi_list under rcu protection. + * Try hard using call_rcu() instead of synchronize_rcu(), + * because we might be called from cleanup_net(), and we + * definitely do not want to block this critical task. */ - synchronize_net(); - - free_percpu(gcells->cells); + defer = kmalloc(sizeof(*defer), GFP_KERNEL | __GFP_NOWARN); + if (likely(defer)) { + defer->ptr = gcells->cells; + call_rcu(&defer->rcu, percpu_free_defer_callback); + } else { + /* We do not hold RTNL at this point, synchronize_net() + * would not be able to expedite this sync. + */ + synchronize_rcu_expedited(); + free_percpu(gcells->cells); + } gcells->cells = NULL; } EXPORT_SYMBOL(gro_cells_destroy);