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 <[email protected]>
Cc: Pablo Neira Ayuso <[email protected]>
Cc: Jiri Wiesner <[email protected]>
Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
---
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);
+ kfree_rcu_mightsleep(td);
}
kd->est_count--;
if (kd->est_count) {
--
2.30.2
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 <[email protected]>
> Cc: Pablo Neira Ayuso <[email protected]>
> Cc: Jiri Wiesner <[email protected]>
> Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> ---
> 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 <[email protected]>
> > Cc: Pablo Neira Ayuso <[email protected]>
> > Cc: Jiri Wiesner <[email protected]>
> > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > ---
> > 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 <[email protected]>
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 <[email protected]>
> > > Cc: Pablo Neira Ayuso <[email protected]>
> > > Cc: Jiri Wiesner <[email protected]>
> > > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > > ---
> > > 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 <[email protected]>
> > > Cc: Pablo Neira Ayuso <[email protected]>
> > > Cc: Jiri Wiesner <[email protected]>
> > > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > > ---
> > > 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 <[email protected]>
> > > > Cc: Pablo Neira Ayuso <[email protected]>
> > > > Cc: Jiri Wiesner <[email protected]>
> > > > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > > > ---
> > > > 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 <[email protected]>
> > > > > Cc: Pablo Neira Ayuso <[email protected]>
> > > > > Cc: Jiri Wiesner <[email protected]>
> > > > > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > > > > ---
> > > > > 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 <[email protected]>
> > > > > > Cc: Pablo Neira Ayuso <[email protected]>
> > > > > > Cc: Jiri Wiesner <[email protected]>
> > > > > > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > > > > > ---
> > > > > > 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 <[email protected]>
> > > > > > > Cc: Pablo Neira Ayuso <[email protected]>
> > > > > > > Cc: Jiri Wiesner <[email protected]>
> > > > > > > Signed-off-by: Uladzislau Rezki (Sony) <[email protected]>
> > > > > > > ---
> > > > > > > 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