[PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Uladzislau Rezki (Sony)]

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);
+		kfree_rcu_mightsleep(td);
 	}
 	kd->est_count--;
 	if (kd->est_count) {
-- 
2.30.2

[PATCH 11/13] rcuscale: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Uladzislau Rezki (Sony)]

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.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 kernel/rcu/rcuscale.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 4120f94030c3..e82ec9f9a5d8 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -715,7 +715,7 @@ kfree_scale_thread(void *arg)
 			// is tested.
 			if ((kfree_rcu_test_single && !kfree_rcu_test_double) ||
 					(kfree_rcu_test_both && torture_random(&tr) & 0x800))
-				kfree_rcu(alloc_ptr);
+				kfree_rcu_mightsleep(alloc_ptr);
 			else
 				kfree_rcu(alloc_ptr, rh);
 		}
-- 
2.30.2

[PATCH 12/13] doc: Update whatisRCU.rst

[Uladzislau Rezki (Sony)]

The kfree_rcu() macro is deprecated. Rename it to the
kfree_rcu_mightsleep() in this documentation.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 Documentation/RCU/whatisRCU.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/RCU/whatisRCU.rst b/Documentation/RCU/whatisRCU.rst
index 2c5563a91998..8eddef28d3a1 100644
--- a/Documentation/RCU/whatisRCU.rst
+++ b/Documentation/RCU/whatisRCU.rst
@@ -597,10 +597,10 @@ to avoid having to write your own callback::
 If the occasional sleep is permitted, the single-argument form may
 be used, omitting the rcu_head structure from struct foo.
 
-	kfree_rcu(old_fp);
+	kfree_rcu_mightsleep(old_fp);
 
-This variant of kfree_rcu() almost never blocks, but might do so by
-invoking synchronize_rcu() in response to memory-allocation failure.
+This variant almost never blocks, but might do so by invoking
+synchronize_rcu() in response to memory-allocation failure.
 
 Again, see checklist.rst for additional rules governing the use of RCU.
 
-- 
2.30.2

[PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Uladzislau Rezki (Sony)]

For a single argument invocations a new kfree_rcu_mightsleep()
and kvfree_rcu_mightsleep() macroses are used. This is done in
order to prevent users from calling a single argument from
atomic contexts as "_mightsleep" prefix signals that it can
schedule().

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 include/linux/rcupdate.h | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 094321c17e48..7571dbfecb18 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -957,9 +957,8 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 
 /**
  * kfree_rcu() - kfree an object after a grace period.
- * @ptr: pointer to kfree for both single- and double-argument invocations.
- * @rhf: the name of the struct rcu_head within the type of @ptr,
- *       but only for double-argument invocations.
+ * @ptr: pointer to kfree for double-argument invocations.
+ * @rhf: the name of the struct rcu_head within the type of @ptr.
  *
  * Many rcu callbacks functions just call kfree() on the base structure.
  * These functions are trivial, but their size adds up, and furthermore
@@ -982,26 +981,18 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
  * The BUILD_BUG_ON check must not involve any function calls, hence the
  * checks are done in macros here.
  */
-#define kfree_rcu(ptr, rhf...) kvfree_rcu(ptr, ## rhf)
+#define kfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
+#define kvfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
 
 /**
- * kvfree_rcu() - kvfree an object after a grace period.
- *
- * This macro consists of one or two arguments and it is
- * based on whether an object is head-less or not. If it
- * has a head then a semantic stays the same as it used
- * to be before:
- *
- *     kvfree_rcu(ptr, rhf);
- *
- * where @ptr is a pointer to kvfree(), @rhf is the name
- * of the rcu_head structure within the type of @ptr.
+ * kfree_rcu_mightsleep() - kfree an object after a grace period.
+ * @ptr: pointer to kfree for single-argument invocations.
  *
  * When it comes to head-less variant, only one argument
  * is passed and that is just a pointer which has to be
  * freed after a grace period. Therefore the semantic is
  *
- *     kvfree_rcu(ptr);
+ *     kfree_rcu_mightsleep(ptr);
  *
  * where @ptr is the pointer to be freed by kvfree().
  *
@@ -1010,13 +1001,9 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
  * annotation. Otherwise, please switch and embed the
  * rcu_head structure within the type of @ptr.
  */
-#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,		\
-	kvfree_rcu_arg_2, kvfree_rcu_arg_1)(__VA_ARGS__)
-
+#define kfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
 #define kvfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
-#define kfree_rcu_mightsleep(ptr) kvfree_rcu_mightsleep(ptr)
 
-#define KVFREE_GET_MACRO(_1, _2, NAME, ...) NAME
 #define kvfree_rcu_arg_2(ptr, rhf)					\
 do {									\
 	typeof (ptr) ___p = (ptr);					\
-- 
2.30.2

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Joel Fernandes]

Hi, All,

On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
<urezki@gmail.com> wrote:
>
> For a single argument invocations a new kfree_rcu_mightsleep()
> and kvfree_rcu_mightsleep() macroses are used. This is done in
> order to prevent users from calling a single argument from
> atomic contexts as "_mightsleep" prefix signals that it can
> schedule().
>

Since this commit in -dev branch [1] suggests more users still need
conversion, let us drop this single patch for 6.4 and move the rest of
the series forward? Let me know if you disagree.
https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b

All -- please supply Ack/Review tags for patches 1-12.

thanks,

 - Joel


> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  include/linux/rcupdate.h | 29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 094321c17e48..7571dbfecb18 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -957,9 +957,8 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>
>  /**
>   * kfree_rcu() - kfree an object after a grace period.
> - * @ptr: pointer to kfree for both single- and double-argument invocations.
> - * @rhf: the name of the struct rcu_head within the type of @ptr,
> - *       but only for double-argument invocations.
> + * @ptr: pointer to kfree for double-argument invocations.
> + * @rhf: the name of the struct rcu_head within the type of @ptr.
>   *
>   * Many rcu callbacks functions just call kfree() on the base structure.
>   * These functions are trivial, but their size adds up, and furthermore
> @@ -982,26 +981,18 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>   * The BUILD_BUG_ON check must not involve any function calls, hence the
>   * checks are done in macros here.
>   */
> -#define kfree_rcu(ptr, rhf...) kvfree_rcu(ptr, ## rhf)
> +#define kfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
> +#define kvfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
>
>  /**
> - * kvfree_rcu() - kvfree an object after a grace period.
> - *
> - * This macro consists of one or two arguments and it is
> - * based on whether an object is head-less or not. If it
> - * has a head then a semantic stays the same as it used
> - * to be before:
> - *
> - *     kvfree_rcu(ptr, rhf);
> - *
> - * where @ptr is a pointer to kvfree(), @rhf is the name
> - * of the rcu_head structure within the type of @ptr.
> + * kfree_rcu_mightsleep() - kfree an object after a grace period.
> + * @ptr: pointer to kfree for single-argument invocations.
>   *
>   * When it comes to head-less variant, only one argument
>   * is passed and that is just a pointer which has to be
>   * freed after a grace period. Therefore the semantic is
>   *
> - *     kvfree_rcu(ptr);
> + *     kfree_rcu_mightsleep(ptr);
>   *
>   * where @ptr is the pointer to be freed by kvfree().
>   *
> @@ -1010,13 +1001,9 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>   * annotation. Otherwise, please switch and embed the
>   * rcu_head structure within the type of @ptr.
>   */
> -#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,          \
> -       kvfree_rcu_arg_2, kvfree_rcu_arg_1)(__VA_ARGS__)
> -
> +#define kfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
>  #define kvfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
> -#define kfree_rcu_mightsleep(ptr) kvfree_rcu_mightsleep(ptr)
>
> -#define KVFREE_GET_MACRO(_1, _2, NAME, ...) NAME
>  #define kvfree_rcu_arg_2(ptr, rhf)                                     \
>  do {                                                                   \
>         typeof (ptr) ___p = (ptr);                                      \
> --
> 2.30.2
>

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Joel Fernandes]

> On Mar 5, 2023, at 5:29 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> 
> Hi, All,
> 
>> On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
>> <urezki@gmail.com> wrote:
>> 
>> For a single argument invocations a new kfree_rcu_mightsleep()
>> and kvfree_rcu_mightsleep() macroses are used. This is done in
>> order to prevent users from calling a single argument from
>> atomic contexts as "_mightsleep" prefix signals that it can
>> schedule().
>> 
> 
> Since this commit in -dev branch [1] suggests more users still need
> conversion, let us drop this single patch for 6.4 and move the rest of
> the series forward? Let me know if you disagree.
> https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b
> 
> All -- please supply Ack/Review tags for patches 1-12.

Or put another way, what is the transition plan for these remaining users?

I am getting on a plane right now but I can research which users are remaining later.

 - Joel


> 
> thanks,
> 
> - Joel
> 
> 
>> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
>> ---
>> include/linux/rcupdate.h | 29 ++++++++---------------------
>> 1 file changed, 8 insertions(+), 21 deletions(-)
>> 
>> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
>> index 094321c17e48..7571dbfecb18 100644
>> --- a/include/linux/rcupdate.h
>> +++ b/include/linux/rcupdate.h
>> @@ -957,9 +957,8 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>> 
>> /**
>>  * kfree_rcu() - kfree an object after a grace period.
>> - * @ptr: pointer to kfree for both single- and double-argument invocations.
>> - * @rhf: the name of the struct rcu_head within the type of @ptr,
>> - *       but only for double-argument invocations.
>> + * @ptr: pointer to kfree for double-argument invocations.
>> + * @rhf: the name of the struct rcu_head within the type of @ptr.
>>  *
>>  * Many rcu callbacks functions just call kfree() on the base structure.
>>  * These functions are trivial, but their size adds up, and furthermore
>> @@ -982,26 +981,18 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>>  * The BUILD_BUG_ON check must not involve any function calls, hence the
>>  * checks are done in macros here.
>>  */
>> -#define kfree_rcu(ptr, rhf...) kvfree_rcu(ptr, ## rhf)
>> +#define kfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
>> +#define kvfree_rcu(ptr, rhf) kvfree_rcu_arg_2(ptr, rhf)
>> 
>> /**
>> - * kvfree_rcu() - kvfree an object after a grace period.
>> - *
>> - * This macro consists of one or two arguments and it is
>> - * based on whether an object is head-less or not. If it
>> - * has a head then a semantic stays the same as it used
>> - * to be before:
>> - *
>> - *     kvfree_rcu(ptr, rhf);
>> - *
>> - * where @ptr is a pointer to kvfree(), @rhf is the name
>> - * of the rcu_head structure within the type of @ptr.
>> + * kfree_rcu_mightsleep() - kfree an object after a grace period.
>> + * @ptr: pointer to kfree for single-argument invocations.
>>  *
>>  * When it comes to head-less variant, only one argument
>>  * is passed and that is just a pointer which has to be
>>  * freed after a grace period. Therefore the semantic is
>>  *
>> - *     kvfree_rcu(ptr);
>> + *     kfree_rcu_mightsleep(ptr);
>>  *
>>  * where @ptr is the pointer to be freed by kvfree().
>>  *
>> @@ -1010,13 +1001,9 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>>  * annotation. Otherwise, please switch and embed the
>>  * rcu_head structure within the type of @ptr.
>>  */
>> -#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,          \
>> -       kvfree_rcu_arg_2, kvfree_rcu_arg_1)(__VA_ARGS__)
>> -
>> +#define kfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
>> #define kvfree_rcu_mightsleep(ptr) kvfree_rcu_arg_1(ptr)
>> -#define kfree_rcu_mightsleep(ptr) kvfree_rcu_mightsleep(ptr)
>> 
>> -#define KVFREE_GET_MACRO(_1, _2, NAME, ...) NAME
>> #define kvfree_rcu_arg_2(ptr, rhf)                                     \
>> do {                                                                   \
>>        typeof (ptr) ___p = (ptr);                                      \
>> --
>> 2.30.2
>> 

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Uladzislau Rezki]

> > On Mar 5, 2023, at 5:29 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> > 
> > Hi, All,
> > 
> >> On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
> >> <urezki@gmail.com> wrote:
> >> 
> >> For a single argument invocations a new kfree_rcu_mightsleep()
> >> and kvfree_rcu_mightsleep() macroses are used. This is done in
> >> order to prevent users from calling a single argument from
> >> atomic contexts as "_mightsleep" prefix signals that it can
> >> schedule().
> >> 
> > 
> > Since this commit in -dev branch [1] suggests more users still need
> > conversion, let us drop this single patch for 6.4 and move the rest of
> > the series forward? Let me know if you disagree.
> > https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b
> > 
> > All -- please supply Ack/Review tags for patches 1-12.
> 
> Or put another way, what is the transition plan for these remaining users?
> 
> I am getting on a plane right now but I can research which users are remaining later.
> 
I am not sure. I think we can cover it on the meeting. My feeling is
that, we introduced "_mightsleep" macros first and after that try to
convert users.

@Paul what is your view?

Thanks!

--
Uladzislau Rezki

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Joel Fernandes]

> On Mar 5, 2023, at 6:41 AM, Uladzislau Rezki <urezki@gmail.com> wrote:
> 
> 
>> 
>>>> On Mar 5, 2023, at 5:29 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
>>> 
>>> Hi, All,
>>> 
>>>> On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
>>>> <urezki@gmail.com> wrote:
>>>> 
>>>> For a single argument invocations a new kfree_rcu_mightsleep()
>>>> and kvfree_rcu_mightsleep() macroses are used. This is done in
>>>> order to prevent users from calling a single argument from
>>>> atomic contexts as "_mightsleep" prefix signals that it can
>>>> schedule().
>>>> 
>>> 
>>> Since this commit in -dev branch [1] suggests more users still need
>>> conversion, let us drop this single patch for 6.4 and move the rest of
>>> the series forward? Let me know if you disagree.
>>> https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b
>>> 
>>> All -- please supply Ack/Review tags for patches 1-12.
>> 
>> Or put another way, what is the transition plan for these remaining users?
>> 
>> I am getting on a plane right now but I can research which users are remaining later.
>> 
> I am not sure. I think we can cover it on the meeting.

Cool, thanks.

> My feeling is
> that, we introduced "_mightsleep" macros first and after that try to
> convert users.

One stopgap could be to add a checkpatch error if anyone tries to use old API,
and then in the meanwhile convert all users.
Though, that requires people listening to checkpatch complaints.

Thanks,

 - Joel


> 
> @Paul what is your view?
> 
> Thanks!
> 
> --
> Uladzislau Rezki

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Paul E. McKenney]

On Sun, Mar 05, 2023 at 07:56:33AM -0500, Joel Fernandes wrote:
> 
> 
> > On Mar 5, 2023, at 6:41 AM, Uladzislau Rezki <urezki@gmail.com> wrote:
> > 
> > 
> >> 
> >>>> On Mar 5, 2023, at 5:29 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> >>> 
> >>> Hi, All,
> >>> 
> >>>> On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
> >>>> <urezki@gmail.com> wrote:
> >>>> 
> >>>> For a single argument invocations a new kfree_rcu_mightsleep()
> >>>> and kvfree_rcu_mightsleep() macroses are used. This is done in
> >>>> order to prevent users from calling a single argument from
> >>>> atomic contexts as "_mightsleep" prefix signals that it can
> >>>> schedule().
> >>>> 
> >>> 
> >>> Since this commit in -dev branch [1] suggests more users still need
> >>> conversion, let us drop this single patch for 6.4 and move the rest of
> >>> the series forward? Let me know if you disagree.
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b
> >>> 
> >>> All -- please supply Ack/Review tags for patches 1-12.
> >> 
> >> Or put another way, what is the transition plan for these remaining users?
> >> 
> >> I am getting on a plane right now but I can research which users are remaining later.
> >> 
> > I am not sure. I think we can cover it on the meeting.
> 
> Cool, thanks.

My current plan is as follows:

1.	Addition of kvfree_rcu_mightsleep() went into v6.3.

2.	After creating branches, I send out the series, including 12/12.
	The -rcu tree's "dev" branch continues to have a revert to avoid
	breaking -next until we achieve clean -next and clean "dev"
	branch.

3.	Any conversion patches that get maintainer acks go into v6.4.
	Along with a checkpatch error, as Joel notes below.

4.	There are periodic checks for new code using the single-argument
	forms of kfree_rcu() and kvfree_rcu().	Patches are produced
	for them, or responses to the patches introducing them, as
	appropriate.  A coccinelle script might be helpful, perhaps
	even as part of kernel test robot or similar.

5.	The -rcu tree's "dev" branch will revert to unclean from time
	to time as maintainers choose to take conversion patches into
	their own trees.

6.	Once mainline is clean, we push 12/12 into the next merge
	window.

7.	We then evaluate whether further cleanups are needed.

> > My feeling is
> > that, we introduced "_mightsleep" macros first and after that try to
> > convert users.

> One stopgap could be to add a checkpatch error if anyone tries to use old API,
> and then in the meanwhile convert all users.
> Though, that requires people listening to checkpatch complaints.

Every person who listens is that much less hassle.  It doesn't have to
be perfect.  ;-)

							Thanx, Paul

> Thanks,
> 
>  - Joel
> 
> 
> > 
> > @Paul what is your view?
> > 
> > Thanks!
> > 
> > --
> > Uladzislau Rezki

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Joel Fernandes]

On Sun, Mar 05, 2023 at 10:05:24AM -0800, Paul E. McKenney wrote:
> On Sun, Mar 05, 2023 at 07:56:33AM -0500, Joel Fernandes wrote:
> > 
> > 
> > > On Mar 5, 2023, at 6:41 AM, Uladzislau Rezki <urezki@gmail.com> wrote:
> > > 
> > > 
> > >> 
> > >>>> On Mar 5, 2023, at 5:29 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> > >>> 
> > >>> Hi, All,
> > >>> 
> > >>>> On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
> > >>>> <urezki@gmail.com> wrote:
> > >>>> 
> > >>>> For a single argument invocations a new kfree_rcu_mightsleep()
> > >>>> and kvfree_rcu_mightsleep() macroses are used. This is done in
> > >>>> order to prevent users from calling a single argument from
> > >>>> atomic contexts as "_mightsleep" prefix signals that it can
> > >>>> schedule().
> > >>>> 
> > >>> 
> > >>> Since this commit in -dev branch [1] suggests more users still need
> > >>> conversion, let us drop this single patch for 6.4 and move the rest of
> > >>> the series forward? Let me know if you disagree.
> > >>> https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b
> > >>> 
> > >>> All -- please supply Ack/Review tags for patches 1-12.
> > >> 
> > >> Or put another way, what is the transition plan for these remaining users?
> > >> 
> > >> I am getting on a plane right now but I can research which users are remaining later.
> > >> 
> > > I am not sure. I think we can cover it on the meeting.
> > 
> > Cool, thanks.
> 
> My current plan is as follows:
> 
> 1.	Addition of kvfree_rcu_mightsleep() went into v6.3.
> 
> 2.	After creating branches, I send out the series, including 12/12.
> 	The -rcu tree's "dev" branch continues to have a revert to avoid
> 	breaking -next until we achieve clean -next and clean "dev"
> 	branch.
> 
> 3.	Any conversion patches that get maintainer acks go into v6.4.
> 	Along with a checkpatch error, as Joel notes below.
> 
> 4.	There are periodic checks for new code using the single-argument
> 	forms of kfree_rcu() and kvfree_rcu().	Patches are produced
> 	for them, or responses to the patches introducing them, as
> 	appropriate.  A coccinelle script might be helpful, perhaps
> 	even as part of kernel test robot or similar.
> 
> 5.	The -rcu tree's "dev" branch will revert to unclean from time
> 	to time as maintainers choose to take conversion patches into
> 	their own trees.
> 
> 6.	Once mainline is clean, we push 12/12 into the next merge
> 	window.

Since in theory, mainline could also be after 6.4-rc1, I am assuming next merge
window could also mean 6.5 right? But yes, agreed.

> 7.	We then evaluate whether further cleanups are needed.
> 
> > > My feeling is
> > > that, we introduced "_mightsleep" macros first and after that try to
> > > convert users.
> 
> > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > and then in the meanwhile convert all users.
> > Though, that requires people listening to checkpatch complaints.
> 
> Every person who listens is that much less hassle.  It doesn't have to
> be perfect.  ;-)

The below checkpatch change can catch at least simple single-arg uses (i.e.
not having compound expressions inside of k[v]free_rcu() args). I will submit
a proper patch to it which we can include in this set.

Thoughts?
---
 scripts/checkpatch.pl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 78cc595b98ce..fc73786064b3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6362,6 +6362,15 @@ sub process {
 			}
 		}
 
+# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
+		if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
+			if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
+				ERROR("DEPRECATED_API",
+				      "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);
+			}
+		}
+
+
 # check for unnecessary "Out of Memory" messages
 		if ($line =~ /^\+.*\b$logFunctions\s*\(/ &&
 		    $prevline =~ /^[ \+]\s*if\s*\(\s*(\!\s*|NULL\s*==\s*)?($Lval)(\s*==\s*NULL\s*)?\s*\)/ &&
-- 
2.40.0.rc0.216.gc4246ad0f0-goog

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Paul E. McKenney]

On Mon, Mar 06, 2023 at 02:49:48PM +0000, Joel Fernandes wrote:
> On Sun, Mar 05, 2023 at 10:05:24AM -0800, Paul E. McKenney wrote:
> > On Sun, Mar 05, 2023 at 07:56:33AM -0500, Joel Fernandes wrote:
> > > 
> > > 
> > > > On Mar 5, 2023, at 6:41 AM, Uladzislau Rezki <urezki@gmail.com> wrote:
> > > > 
> > > > 
> > > >> 
> > > >>>> On Mar 5, 2023, at 5:29 AM, Joel Fernandes <joel@joelfernandes.org> wrote:
> > > >>> 
> > > >>> Hi, All,
> > > >>> 
> > > >>>> On Wed, Feb 1, 2023 at 10:11 AM Uladzislau Rezki (Sony)
> > > >>>> <urezki@gmail.com> wrote:
> > > >>>> 
> > > >>>> For a single argument invocations a new kfree_rcu_mightsleep()
> > > >>>> and kvfree_rcu_mightsleep() macroses are used. This is done in
> > > >>>> order to prevent users from calling a single argument from
> > > >>>> atomic contexts as "_mightsleep" prefix signals that it can
> > > >>>> schedule().
> > > >>>> 
> > > >>> 
> > > >>> Since this commit in -dev branch [1] suggests more users still need
> > > >>> conversion, let us drop this single patch for 6.4 and move the rest of
> > > >>> the series forward? Let me know if you disagree.
> > > >>> https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=dev&id=9bf5e3a2626ed474d080f695007541b6ecd6e60b
> > > >>> 
> > > >>> All -- please supply Ack/Review tags for patches 1-12.
> > > >> 
> > > >> Or put another way, what is the transition plan for these remaining users?
> > > >> 
> > > >> I am getting on a plane right now but I can research which users are remaining later.
> > > >> 
> > > > I am not sure. I think we can cover it on the meeting.
> > > 
> > > Cool, thanks.
> > 
> > My current plan is as follows:
> > 
> > 1.	Addition of kvfree_rcu_mightsleep() went into v6.3.
> > 
> > 2.	After creating branches, I send out the series, including 12/12.
> > 	The -rcu tree's "dev" branch continues to have a revert to avoid
> > 	breaking -next until we achieve clean -next and clean "dev"
> > 	branch.
> > 
> > 3.	Any conversion patches that get maintainer acks go into v6.4.
> > 	Along with a checkpatch error, as Joel notes below.
> > 
> > 4.	There are periodic checks for new code using the single-argument
> > 	forms of kfree_rcu() and kvfree_rcu().	Patches are produced
> > 	for them, or responses to the patches introducing them, as
> > 	appropriate.  A coccinelle script might be helpful, perhaps
> > 	even as part of kernel test robot or similar.
> > 
> > 5.	The -rcu tree's "dev" branch will revert to unclean from time
> > 	to time as maintainers choose to take conversion patches into
> > 	their own trees.
> > 
> > 6.	Once mainline is clean, we push 12/12 into the next merge
> > 	window.
> 
> Since in theory, mainline could also be after 6.4-rc1, I am assuming next merge
> window could also mean 6.5 right? But yes, agreed.

I would rather not waste Linus's time with a separate pull request for
this.  It is after all not a regression.  ;-)

> > 7.	We then evaluate whether further cleanups are needed.
> > 
> > > > My feeling is
> > > > that, we introduced "_mightsleep" macros first and after that try to
> > > > convert users.
> > 
> > > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > > and then in the meanwhile convert all users.
> > > Though, that requires people listening to checkpatch complaints.
> > 
> > Every person who listens is that much less hassle.  It doesn't have to
> > be perfect.  ;-)
> 
> The below checkpatch change can catch at least simple single-arg uses (i.e.
> not having compound expressions inside of k[v]free_rcu() args). I will submit
> a proper patch to it which we can include in this set.
> 
> Thoughts?
> ---
>  scripts/checkpatch.pl | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 78cc595b98ce..fc73786064b3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6362,6 +6362,15 @@ sub process {
>  			}
>  		}
>  
> +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> +		if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> +			if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> +				ERROR("DEPRECATED_API",
> +				      "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);

Nice!

But could you please also tell them what to use instead?  Sure, they
could look it up, but if it tells them directly, they are less likely
to ignore it.

							Thanx, Paul

> +			}
> +		}
> +
> +
>  # check for unnecessary "Out of Memory" messages
>  		if ($line =~ /^\+.*\b$logFunctions\s*\(/ &&
>  		    $prevline =~ /^[ \+]\s*if\s*\(\s*(\!\s*|NULL\s*==\s*)?($Lval)(\s*==\s*NULL\s*)?\s*\)/ &&
> -- 
> 2.40.0.rc0.216.gc4246ad0f0-goog
> 

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Joel Fernandes]

On Mon, Mar 06, 2023 at 07:01:08AM -0800, Paul E. McKenney wrote:
[..] 
> > > 7.	We then evaluate whether further cleanups are needed.
> > > 
> > > > > My feeling is
> > > > > that, we introduced "_mightsleep" macros first and after that try to
> > > > > convert users.
> > > 
> > > > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > > > and then in the meanwhile convert all users.
> > > > Though, that requires people listening to checkpatch complaints.
> > > 
> > > Every person who listens is that much less hassle.  It doesn't have to
> > > be perfect.  ;-)
> > 
> > The below checkpatch change can catch at least simple single-arg uses (i.e.
> > not having compound expressions inside of k[v]free_rcu() args). I will submit
> > a proper patch to it which we can include in this set.
> > 
> > Thoughts?
> > ---
> >  scripts/checkpatch.pl | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 78cc595b98ce..fc73786064b3 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -6362,6 +6362,15 @@ sub process {
> >  			}
> >  		}
> >  
> > +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> > +		if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> > +			if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> > +				ERROR("DEPRECATED_API",
> > +				      "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);
> 
> Nice!
> 
> But could you please also tell them what to use instead?  Sure, they
> could look it up, but if it tells them directly, they are less likely
> to ignore it.

Sounds good, I will modify the warning to include the API to call and send
out a patch soon.

thanks,

 - Joel

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Uladzislau Rezki]

On Mon, Mar 06, 2023 at 03:12:03PM +0000, Joel Fernandes wrote:
> On Mon, Mar 06, 2023 at 07:01:08AM -0800, Paul E. McKenney wrote:
> [..] 
> > > > 7.	We then evaluate whether further cleanups are needed.
> > > > 
> > > > > > My feeling is
> > > > > > that, we introduced "_mightsleep" macros first and after that try to
> > > > > > convert users.
> > > > 
> > > > > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > > > > and then in the meanwhile convert all users.
> > > > > Though, that requires people listening to checkpatch complaints.
> > > > 
> > > > Every person who listens is that much less hassle.  It doesn't have to
> > > > be perfect.  ;-)
> > > 
> > > The below checkpatch change can catch at least simple single-arg uses (i.e.
> > > not having compound expressions inside of k[v]free_rcu() args). I will submit
> > > a proper patch to it which we can include in this set.
> > > 
> > > Thoughts?
> > > ---
> > >  scripts/checkpatch.pl | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 78cc595b98ce..fc73786064b3 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -6362,6 +6362,15 @@ sub process {
> > >  			}
> > >  		}
> > >  
> > > +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> > > +		if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> > > +			if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> > > +				ERROR("DEPRECATED_API",
> > > +				      "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);
> > 
> > Nice!
> > 
> > But could you please also tell them what to use instead?  Sure, they
> > could look it up, but if it tells them directly, they are less likely
> > to ignore it.
> 
> Sounds good, I will modify the warning to include the API to call and send
> out a patch soon.
> 
Maybe compile warnings? Or is it too aggressive?

Thanks!

--
Uladzislau Rezki

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Paul E. McKenney]

On Mon, Mar 06, 2023 at 05:42:44PM +0100, Uladzislau Rezki wrote:
> On Mon, Mar 06, 2023 at 03:12:03PM +0000, Joel Fernandes wrote:
> > On Mon, Mar 06, 2023 at 07:01:08AM -0800, Paul E. McKenney wrote:
> > [..] 
> > > > > 7.	We then evaluate whether further cleanups are needed.
> > > > > 
> > > > > > > My feeling is
> > > > > > > that, we introduced "_mightsleep" macros first and after that try to
> > > > > > > convert users.
> > > > > 
> > > > > > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > > > > > and then in the meanwhile convert all users.
> > > > > > Though, that requires people listening to checkpatch complaints.
> > > > > 
> > > > > Every person who listens is that much less hassle.  It doesn't have to
> > > > > be perfect.  ;-)
> > > > 
> > > > The below checkpatch change can catch at least simple single-arg uses (i.e.
> > > > not having compound expressions inside of k[v]free_rcu() args). I will submit
> > > > a proper patch to it which we can include in this set.
> > > > 
> > > > Thoughts?
> > > > ---
> > > >  scripts/checkpatch.pl | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > > 
> > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > index 78cc595b98ce..fc73786064b3 100755
> > > > --- a/scripts/checkpatch.pl
> > > > +++ b/scripts/checkpatch.pl
> > > > @@ -6362,6 +6362,15 @@ sub process {
> > > >  			}
> > > >  		}
> > > >  
> > > > +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> > > > +		if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> > > > +			if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> > > > +				ERROR("DEPRECATED_API",
> > > > +				      "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);
> > > 
> > > Nice!
> > > 
> > > But could you please also tell them what to use instead?  Sure, they
> > > could look it up, but if it tells them directly, they are less likely
> > > to ignore it.
> > 
> > Sounds good, I will modify the warning to include the API to call and send
> > out a patch soon.
> > 
> Maybe compile warnings? Or is it too aggressive?

That is an excellent option if people ignore the checkpatch.pl warnings,
thus forcing us to delay past v6.5.  So Murphy would argue that we will
in fact take your good advice at some point.  ;-)

							Thanx, Paul

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Uladzislau Rezki]

On Mon, Mar 06, 2023 at 08:55:01AM -0800, Paul E. McKenney wrote:
> On Mon, Mar 06, 2023 at 05:42:44PM +0100, Uladzislau Rezki wrote:
> > On Mon, Mar 06, 2023 at 03:12:03PM +0000, Joel Fernandes wrote:
> > > On Mon, Mar 06, 2023 at 07:01:08AM -0800, Paul E. McKenney wrote:
> > > [..] 
> > > > > > 7.	We then evaluate whether further cleanups are needed.
> > > > > > 
> > > > > > > > My feeling is
> > > > > > > > that, we introduced "_mightsleep" macros first and after that try to
> > > > > > > > convert users.
> > > > > > 
> > > > > > > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > > > > > > and then in the meanwhile convert all users.
> > > > > > > Though, that requires people listening to checkpatch complaints.
> > > > > > 
> > > > > > Every person who listens is that much less hassle.  It doesn't have to
> > > > > > be perfect.  ;-)
> > > > > 
> > > > > The below checkpatch change can catch at least simple single-arg uses (i.e.
> > > > > not having compound expressions inside of k[v]free_rcu() args). I will submit
> > > > > a proper patch to it which we can include in this set.
> > > > > 
> > > > > Thoughts?
> > > > > ---
> > > > >  scripts/checkpatch.pl | 9 +++++++++
> > > > >  1 file changed, 9 insertions(+)
> > > > > 
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > index 78cc595b98ce..fc73786064b3 100755
> > > > > --- a/scripts/checkpatch.pl
> > > > > +++ b/scripts/checkpatch.pl
> > > > > @@ -6362,6 +6362,15 @@ sub process {
> > > > >  			}
> > > > >  		}
> > > > >  
> > > > > +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> > > > > +		if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> > > > > +			if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> > > > > +				ERROR("DEPRECATED_API",
> > > > > +				      "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);
> > > > 
> > > > Nice!
> > > > 
> > > > But could you please also tell them what to use instead?  Sure, they
> > > > could look it up, but if it tells them directly, they are less likely
> > > > to ignore it.
> > > 
> > > Sounds good, I will modify the warning to include the API to call and send
> > > out a patch soon.
> > > 
> > Maybe compile warnings? Or is it too aggressive?
> 
> That is an excellent option if people ignore the checkpatch.pl warnings,
> thus forcing us to delay past v6.5.  So Murphy would argue that we will
> in fact take your good advice at some point.  ;-)
> 
OK. On this step it sounds like a bit aggressive. checkpatch.pl should
be fine as a light reminder :)

--
Uladzislau Rezki

Re: [PATCH 13/13] rcu/kvfree: Eliminate k[v]free_rcu() single argument macro

[Joel Fernandes]

On Mon, Mar 6, 2023 at 12:10 PM Uladzislau Rezki <urezki@gmail.com> wrote:
>
> On Mon, Mar 06, 2023 at 08:55:01AM -0800, Paul E. McKenney wrote:
> > On Mon, Mar 06, 2023 at 05:42:44PM +0100, Uladzislau Rezki wrote:
> > > On Mon, Mar 06, 2023 at 03:12:03PM +0000, Joel Fernandes wrote:
> > > > On Mon, Mar 06, 2023 at 07:01:08AM -0800, Paul E. McKenney wrote:
> > > > [..]
> > > > > > > 7.  We then evaluate whether further cleanups are needed.
> > > > > > >
> > > > > > > > > My feeling is
> > > > > > > > > that, we introduced "_mightsleep" macros first and after that try to
> > > > > > > > > convert users.
> > > > > > >
> > > > > > > > One stopgap could be to add a checkpatch error if anyone tries to use old API,
> > > > > > > > and then in the meanwhile convert all users.
> > > > > > > > Though, that requires people listening to checkpatch complaints.
> > > > > > >
> > > > > > > Every person who listens is that much less hassle.  It doesn't have to
> > > > > > > be perfect.  ;-)
> > > > > >
> > > > > > The below checkpatch change can catch at least simple single-arg uses (i.e.
> > > > > > not having compound expressions inside of k[v]free_rcu() args). I will submit
> > > > > > a proper patch to it which we can include in this set.
> > > > > >
> > > > > > Thoughts?
> > > > > > ---
> > > > > >  scripts/checkpatch.pl | 9 +++++++++
> > > > > >  1 file changed, 9 insertions(+)
> > > > > >
> > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > > index 78cc595b98ce..fc73786064b3 100755
> > > > > > --- a/scripts/checkpatch.pl
> > > > > > +++ b/scripts/checkpatch.pl
> > > > > > @@ -6362,6 +6362,15 @@ sub process {
> > > > > >                       }
> > > > > >               }
> > > > > >
> > > > > > +# check for soon-to-be-deprecated single-argument k[v]free_rcu() API
> > > > > > +             if ($line =~ /\bk[v]?free_rcu\s*\([^(]+\)/) {
> > > > > > +                     if ($line =~ /\bk[v]?free_rcu\s*\([^,]+\)/) {
> > > > > > +                             ERROR("DEPRECATED_API",
> > > > > > +                                   "Single-argument k[v]free_rcu() API is deprecated, please pass an rcu_head object." . $herecurr);
> > > > >
> > > > > Nice!
> > > > >
> > > > > But could you please also tell them what to use instead?  Sure, they
> > > > > could look it up, but if it tells them directly, they are less likely
> > > > > to ignore it.
> > > >
> > > > Sounds good, I will modify the warning to include the API to call and send
> > > > out a patch soon.
> > > >
> > > Maybe compile warnings? Or is it too aggressive?
> >
> > That is an excellent option if people ignore the checkpatch.pl warnings,
> > thus forcing us to delay past v6.5.  So Murphy would argue that we will
> > in fact take your good advice at some point.  ;-)
> >
> OK. On this step it sounds like a bit aggressive. checkpatch.pl should
> be fine as a light reminder :)

Agreed :). Also on some configs AFAIK, I believe the build will break
with _any_ build warning so checkpatch is a good first step instead.

I will post the checkpatch change today (also to get any early feedback).
thanks,

 - Joel

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Pablo Neira Ayuso]

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); ?

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Julian Anastasov]

	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>

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[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.
> 
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

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Pablo Neira Ayuso]

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?

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Uladzislau Rezki]

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

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Joel Fernandes]

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

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Pablo Neira Ayuso]

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

Re: [PATCH 10/13] ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()

[Joel Fernandes]

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