| Message ID | 20230201150954.409693-1-urezki@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep() | expand |
Hi, On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > The kfree_rcu()'s single argument name is deprecated therefore > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > underline that it is for sleepable contexts. > > Cc: Julian Anastasov <ja@ssi.bg> > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > Cc: Jiri Wiesner <jwiesner@suse.de> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > --- > net/netfilter/ipvs/ip_vs_est.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > index ce2a1549b304..a39baf6d1367 100644 > --- a/net/netfilter/ipvs/ip_vs_est.c > +++ b/net/netfilter/ipvs/ip_vs_est.c > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > __set_bit(row, kd->avail); > if (!kd->tick_len[row]) { > RCU_INIT_POINTER(kd->ticks[row], NULL); > - kfree_rcu(td); I also found this kfree_rcu() without rcu_head call a few weeks ago. @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ?
Hello, On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > Hi, > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > The kfree_rcu()'s single argument name is deprecated therefore > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > underline that it is for sleepable contexts. > > > > Cc: Julian Anastasov <ja@ssi.bg> > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > Cc: Jiri Wiesner <jwiesner@suse.de> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > --- > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > index ce2a1549b304..a39baf6d1367 100644 > > --- a/net/netfilter/ipvs/ip_vs_est.c > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > __set_bit(row, kd->avail); > > if (!kd->tick_len[row]) { > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > - kfree_rcu(td); > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? Yes, as simple as this: diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index c6c61100d244..6d71a5ff52df 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); /* Multiple chains processed in same tick */ struct ip_vs_est_tick_data { + struct rcu_head rcu_head; struct hlist_head chains[IPVS_EST_TICK_CHAINS]; DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c index df56073bb282..25c7118d9348 100644 --- a/net/netfilter/ipvs/ip_vs_est.c +++ b/net/netfilter/ipvs/ip_vs_est.c @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) __set_bit(row, kd->avail); if (!kd->tick_len[row]) { RCU_INIT_POINTER(kd->ticks[row], NULL); - kfree_rcu(td); + kfree_rcu(td, rcu_head); } kd->est_count--; if (kd->est_count) { I was about to reply to Uladzislau Rezki but his patchset looks more like a renaming, so I'm not sure how we are about to integrate this change, as separate patch or as part of his patchset. I don't have preference, just let me know how to handle it. Regards -- Julian Anastasov <ja@ssi.bg>
On Wed, Feb 01, 2023 at 06:12:04PM +0200, Julian Anastasov wrote: > > Hello, > > On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > > > Hi, > > > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > > The kfree_rcu()'s single argument name is deprecated therefore > > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > > underline that it is for sleepable contexts. > > > > > > Cc: Julian Anastasov <ja@ssi.bg> > > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > > Cc: Jiri Wiesner <jwiesner@suse.de> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > --- > > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > index ce2a1549b304..a39baf6d1367 100644 > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > __set_bit(row, kd->avail); > > > if (!kd->tick_len[row]) { > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > - kfree_rcu(td); > > > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? > > Yes, as simple as this: > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index c6c61100d244..6d71a5ff52df 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); > > /* Multiple chains processed in same tick */ > struct ip_vs_est_tick_data { > + struct rcu_head rcu_head; > struct hlist_head chains[IPVS_EST_TICK_CHAINS]; > DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); > DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > index df56073bb282..25c7118d9348 100644 > --- a/net/netfilter/ipvs/ip_vs_est.c > +++ b/net/netfilter/ipvs/ip_vs_est.c > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > __set_bit(row, kd->avail); > if (!kd->tick_len[row]) { > RCU_INIT_POINTER(kd->ticks[row], NULL); > - kfree_rcu(td); > + kfree_rcu(td, rcu_head); > } > kd->est_count--; > if (kd->est_count) { > > I was about to reply to Uladzislau Rezki but his patchset > looks more like a renaming, so I'm not sure how we are about > to integrate this change, as separate patch or as part of his > patchset. I don't have preference, just let me know how to > handle it. > If you give on this patch an Ack i will resend it once i collect all other ACKs. So it will be integrated via RCU-tree. So this is just renaming. If the ip_vs_stop_estimator() can be invoked from an atomic context then it is a bug and you need to integrate rcu_head in your structure. -- Uladzislau Rezki
On Wed, Feb 01, 2023 at 06:12:04PM +0200, Julian Anastasov wrote: > > Hello, > > On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > > > Hi, > > > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > > The kfree_rcu()'s single argument name is deprecated therefore > > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > > underline that it is for sleepable contexts. > > > > > > Cc: Julian Anastasov <ja@ssi.bg> > > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > > Cc: Jiri Wiesner <jwiesner@suse.de> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > --- > > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > index ce2a1549b304..a39baf6d1367 100644 > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > __set_bit(row, kd->avail); > > > if (!kd->tick_len[row]) { > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > - kfree_rcu(td); > > > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? > > Yes, as simple as this: > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index c6c61100d244..6d71a5ff52df 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); > > /* Multiple chains processed in same tick */ > struct ip_vs_est_tick_data { > + struct rcu_head rcu_head; > struct hlist_head chains[IPVS_EST_TICK_CHAINS]; > DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); > DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > index df56073bb282..25c7118d9348 100644 > --- a/net/netfilter/ipvs/ip_vs_est.c > +++ b/net/netfilter/ipvs/ip_vs_est.c > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > __set_bit(row, kd->avail); > if (!kd->tick_len[row]) { > RCU_INIT_POINTER(kd->ticks[row], NULL); > - kfree_rcu(td); > + kfree_rcu(td, rcu_head); > } > kd->est_count--; > if (kd->est_count) { > > I was about to reply to Uladzislau Rezki but his patchset > looks more like a renaming, so I'm not sure how we are about > to integrate this change, as separate patch or as part of his > patchset. I don't have preference, just let me know how to > handle it. @Uladzislau Rezki: Are you fine with dropping this patch from your series and Julian will send us a patch for inclusion into net-next to use the kfree_rcu(x, rcu_head) variant?
On Wed, Feb 01, 2023 at 06:10:18PM +0100, Pablo Neira Ayuso wrote: > On Wed, Feb 01, 2023 at 06:12:04PM +0200, Julian Anastasov wrote: > > > > Hello, > > > > On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > > > > > Hi, > > > > > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > > > The kfree_rcu()'s single argument name is deprecated therefore > > > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > > > underline that it is for sleepable contexts. > > > > > > > > Cc: Julian Anastasov <ja@ssi.bg> > > > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > > > Cc: Jiri Wiesner <jwiesner@suse.de> > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > > --- > > > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > > index ce2a1549b304..a39baf6d1367 100644 > > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > > __set_bit(row, kd->avail); > > > > if (!kd->tick_len[row]) { > > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > > - kfree_rcu(td); > > > > > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > > > > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? > > > > Yes, as simple as this: > > > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > > index c6c61100d244..6d71a5ff52df 100644 > > --- a/include/net/ip_vs.h > > +++ b/include/net/ip_vs.h > > @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); > > > > /* Multiple chains processed in same tick */ > > struct ip_vs_est_tick_data { > > + struct rcu_head rcu_head; > > struct hlist_head chains[IPVS_EST_TICK_CHAINS]; > > DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); > > DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > index df56073bb282..25c7118d9348 100644 > > --- a/net/netfilter/ipvs/ip_vs_est.c > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > __set_bit(row, kd->avail); > > if (!kd->tick_len[row]) { > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > - kfree_rcu(td); > > + kfree_rcu(td, rcu_head); > > } > > kd->est_count--; > > if (kd->est_count) { > > > > I was about to reply to Uladzislau Rezki but his patchset > > looks more like a renaming, so I'm not sure how we are about > > to integrate this change, as separate patch or as part of his > > patchset. I don't have preference, just let me know how to > > handle it. > > @Uladzislau Rezki: Are you fine with dropping this patch from your > series and Julian will send us a patch for inclusion into net-next to > use the kfree_rcu(x, rcu_head) variant? Absolutely. So i will drop it from my series. Thanks! -- Uladzislau Rezki
Hello, On Wed, Feb 01, 2023 at 06:19:49PM +0100, Uladzislau Rezki wrote: > On Wed, Feb 01, 2023 at 06:10:18PM +0100, Pablo Neira Ayuso wrote: > > On Wed, Feb 01, 2023 at 06:12:04PM +0200, Julian Anastasov wrote: > > > > > > Hello, > > > > > > On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > > > > > > > Hi, > > > > > > > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > > > > The kfree_rcu()'s single argument name is deprecated therefore > > > > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > > > > underline that it is for sleepable contexts. > > > > > > > > > > Cc: Julian Anastasov <ja@ssi.bg> > > > > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > > > > Cc: Jiri Wiesner <jwiesner@suse.de> > > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > > > --- > > > > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > > > index ce2a1549b304..a39baf6d1367 100644 > > > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > > > __set_bit(row, kd->avail); > > > > > if (!kd->tick_len[row]) { > > > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > > > - kfree_rcu(td); > > > > > > > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > > > > > > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? > > > > > > Yes, as simple as this: > > > > > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > > > index c6c61100d244..6d71a5ff52df 100644 > > > --- a/include/net/ip_vs.h > > > +++ b/include/net/ip_vs.h > > > @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); > > > > > > /* Multiple chains processed in same tick */ > > > struct ip_vs_est_tick_data { > > > + struct rcu_head rcu_head; > > > struct hlist_head chains[IPVS_EST_TICK_CHAINS]; > > > DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); > > > DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > index df56073bb282..25c7118d9348 100644 > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > __set_bit(row, kd->avail); > > > if (!kd->tick_len[row]) { > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > - kfree_rcu(td); > > > + kfree_rcu(td, rcu_head); > > > } > > > kd->est_count--; > > > if (kd->est_count) { > > > > > > I was about to reply to Uladzislau Rezki but his patchset > > > looks more like a renaming, so I'm not sure how we are about > > > to integrate this change, as separate patch or as part of his > > > patchset. I don't have preference, just let me know how to > > > handle it. > > > > @Uladzislau Rezki: Are you fine with dropping this patch from your > > series and Julian will send us a patch for inclusion into net-next to > > use the kfree_rcu(x, rcu_head) variant? > Absolutely. So i will drop it from my series. > Since this patch was dropped, it is the only case blocking the proper integration of this series into linux-next. We want to drop the old API and currently we are not able to, thus this revert [1] has to be unfortunately carried in linux-next. For that reason, there are 2 options: 1. Can we get the new rcu_head approach for ipvs posted and reviewed with suitable Acks? 2. Can we carry Vlad's patch to use kfree_rcu_mightsleep() in ipvs and drop it later if/when #1 is completed? Option 2 has the unfortunate effect that it will conflict with your new approach of using rcu_head so I'd rather you fix it that way and get it Acked. And once acked, we can also take it via the RCU tree if the net maintainers are Ok with that. Please advise. thanks, - Joel [1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=3898e7642316732e23716ca902f9d122736f9805
On Thu, Mar 09, 2023 at 12:11:01AM +0000, Joel Fernandes wrote: > Hello, > > On Wed, Feb 01, 2023 at 06:19:49PM +0100, Uladzislau Rezki wrote: > > On Wed, Feb 01, 2023 at 06:10:18PM +0100, Pablo Neira Ayuso wrote: > > > On Wed, Feb 01, 2023 at 06:12:04PM +0200, Julian Anastasov wrote: > > > > > > > > Hello, > > > > > > > > On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > > > > > > > > > Hi, > > > > > > > > > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > > > > > The kfree_rcu()'s single argument name is deprecated therefore > > > > > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > > > > > underline that it is for sleepable contexts. > > > > > > > > > > > > Cc: Julian Anastasov <ja@ssi.bg> > > > > > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > > > > > Cc: Jiri Wiesner <jwiesner@suse.de> > > > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > > > > --- > > > > > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > > > > index ce2a1549b304..a39baf6d1367 100644 > > > > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > > > > __set_bit(row, kd->avail); > > > > > > if (!kd->tick_len[row]) { > > > > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > > > > - kfree_rcu(td); > > > > > > > > > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > > > > > > > > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? > > > > > > > > Yes, as simple as this: > > > > > > > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > > > > index c6c61100d244..6d71a5ff52df 100644 > > > > --- a/include/net/ip_vs.h > > > > +++ b/include/net/ip_vs.h > > > > @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); > > > > > > > > /* Multiple chains processed in same tick */ > > > > struct ip_vs_est_tick_data { > > > > + struct rcu_head rcu_head; > > > > struct hlist_head chains[IPVS_EST_TICK_CHAINS]; > > > > DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); > > > > DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > > index df56073bb282..25c7118d9348 100644 > > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > > __set_bit(row, kd->avail); > > > > if (!kd->tick_len[row]) { > > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > > - kfree_rcu(td); > > > > + kfree_rcu(td, rcu_head); > > > > } > > > > kd->est_count--; > > > > if (kd->est_count) { > > > > > > > > I was about to reply to Uladzislau Rezki but his patchset > > > > looks more like a renaming, so I'm not sure how we are about > > > > to integrate this change, as separate patch or as part of his > > > > patchset. I don't have preference, just let me know how to > > > > handle it. > > > > > > @Uladzislau Rezki: Are you fine with dropping this patch from your > > > series and Julian will send us a patch for inclusion into net-next to > > > use the kfree_rcu(x, rcu_head) variant? > > Absolutely. So i will drop it from my series. > > Since this patch was dropped, it is the only case blocking the proper > integration of this series into linux-next. We want to drop the old API and > currently we are not able to, thus this revert [1] has to be unfortunately > carried in linux-next. > > For that reason, there are 2 options: > > 1. Can we get the new rcu_head approach for ipvs posted and reviewed with > suitable Acks? > > 2. Can we carry Vlad's patch to use kfree_rcu_mightsleep() in ipvs and drop > it later if/when #1 is completed? > > Option 2 has the unfortunate effect that it will conflict with your new > approach of using rcu_head so I'd rather you fix it that way and get it > Acked. And once acked, we can also take it via the RCU tree if the net > maintainers are Ok with that. > > Please advise. JFYI, this patch is already in linux.git https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e4d0fe71f59dc5137a2793ff7560730d80d1e1f4
On Thu, Mar 09, 2023 at 10:19:43AM +0100, Pablo Neira Ayuso wrote: > On Thu, Mar 09, 2023 at 12:11:01AM +0000, Joel Fernandes wrote: > > Hello, > > > > On Wed, Feb 01, 2023 at 06:19:49PM +0100, Uladzislau Rezki wrote: > > > On Wed, Feb 01, 2023 at 06:10:18PM +0100, Pablo Neira Ayuso wrote: > > > > On Wed, Feb 01, 2023 at 06:12:04PM +0200, Julian Anastasov wrote: > > > > > > > > > > Hello, > > > > > > > > > > On Wed, 1 Feb 2023, Pablo Neira Ayuso wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > On Wed, Feb 01, 2023 at 04:09:51PM +0100, Uladzislau Rezki (Sony) wrote: > > > > > > > The kfree_rcu()'s single argument name is deprecated therefore > > > > > > > rename it to kfree_rcu_mightsleep() variant. The goal is explicitly > > > > > > > underline that it is for sleepable contexts. > > > > > > > > > > > > > > Cc: Julian Anastasov <ja@ssi.bg> > > > > > > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > > > > > > Cc: Jiri Wiesner <jwiesner@suse.de> > > > > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > > > > > > --- > > > > > > > net/netfilter/ipvs/ip_vs_est.c | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > > > > > index ce2a1549b304..a39baf6d1367 100644 > > > > > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > > > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > > > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > > > > > __set_bit(row, kd->avail); > > > > > > > if (!kd->tick_len[row]) { > > > > > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > > > > > - kfree_rcu(td); > > > > > > > > > > > > I also found this kfree_rcu() without rcu_head call a few weeks ago. > > > > > > > > > > > > @Wiesner, @Julian: Any chance this can be turned into kfree_rcu(td, rcu_head); ? > > > > > > > > > > Yes, as simple as this: > > > > > > > > > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > > > > > index c6c61100d244..6d71a5ff52df 100644 > > > > > --- a/include/net/ip_vs.h > > > > > +++ b/include/net/ip_vs.h > > > > > @@ -461,6 +461,7 @@ void ip_vs_stats_free(struct ip_vs_stats *stats); > > > > > > > > > > /* Multiple chains processed in same tick */ > > > > > struct ip_vs_est_tick_data { > > > > > + struct rcu_head rcu_head; > > > > > struct hlist_head chains[IPVS_EST_TICK_CHAINS]; > > > > > DECLARE_BITMAP(present, IPVS_EST_TICK_CHAINS); > > > > > DECLARE_BITMAP(full, IPVS_EST_TICK_CHAINS); > > > > > diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c > > > > > index df56073bb282..25c7118d9348 100644 > > > > > --- a/net/netfilter/ipvs/ip_vs_est.c > > > > > +++ b/net/netfilter/ipvs/ip_vs_est.c > > > > > @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) > > > > > __set_bit(row, kd->avail); > > > > > if (!kd->tick_len[row]) { > > > > > RCU_INIT_POINTER(kd->ticks[row], NULL); > > > > > - kfree_rcu(td); > > > > > + kfree_rcu(td, rcu_head); > > > > > } > > > > > kd->est_count--; > > > > > if (kd->est_count) { > > > > > > > > > > I was about to reply to Uladzislau Rezki but his patchset > > > > > looks more like a renaming, so I'm not sure how we are about > > > > > to integrate this change, as separate patch or as part of his > > > > > patchset. I don't have preference, just let me know how to > > > > > handle it. > > > > > > > > @Uladzislau Rezki: Are you fine with dropping this patch from your > > > > series and Julian will send us a patch for inclusion into net-next to > > > > use the kfree_rcu(x, rcu_head) variant? > > > Absolutely. So i will drop it from my series. > > > > Since this patch was dropped, it is the only case blocking the proper > > integration of this series into linux-next. We want to drop the old API and > > currently we are not able to, thus this revert [1] has to be unfortunately > > carried in linux-next. > > > > For that reason, there are 2 options: > > > > 1. Can we get the new rcu_head approach for ipvs posted and reviewed with > > suitable Acks? > > > > 2. Can we carry Vlad's patch to use kfree_rcu_mightsleep() in ipvs and drop > > it later if/when #1 is completed? > > > > Option 2 has the unfortunate effect that it will conflict with your new > > approach of using rcu_head so I'd rather you fix it that way and get it > > Acked. And once acked, we can also take it via the RCU tree if the net > > maintainers are Ok with that. > > > > Please advise. > > JFYI, this patch is already in linux.git > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e4d0fe71f59dc5137a2793ff7560730d80d1e1f4 Indeed it is in, thank you! - Joel
diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c index ce2a1549b304..a39baf6d1367 100644 --- a/net/netfilter/ipvs/ip_vs_est.c +++ b/net/netfilter/ipvs/ip_vs_est.c @@ -549,7 +549,7 @@ void ip_vs_stop_estimator(struct netns_ipvs *ipvs, struct ip_vs_stats *stats) __set_bit(row, kd->avail); if (!kd->tick_len[row]) { RCU_INIT_POINTER(kd->ticks[row], NULL); - kfree_rcu(td); + kfree_rcu_mightsleep(td); } kd->est_count--; if (kd->est_count) {
The kfree_rcu()'s single argument name is deprecated therefore rename it to kfree_rcu_mightsleep() variant. The goal is explicitly underline that it is for sleepable contexts. Cc: Julian Anastasov <ja@ssi.bg> Cc: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Jiri Wiesner <jwiesner@suse.de> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> --- net/netfilter/ipvs/ip_vs_est.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)