All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] checkpatch: Error out if deprecated RCU API used
@ 2023-03-07  3:04 Joel Fernandes (Google)
  2023-03-07  3:08 ` Joe Perches
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Fernandes (Google) @ 2023-03-07  3:04 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Andy Whitcroft, Dwaipayan Ray,
	Joe Perches, Lukas Bulwahn, Paul E. McKenney, RCU,
	Oleksiy Avramchenko, Jens Axboe, Philipp Reisner, Bryan Tan,
	Steven Rostedt, Eric Dumazet, Bob Pearson, Ariel Levkovich,
	Theodore Ts'o, Julian Anastasov, Uladzislau Rezki

Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
error-prone. However, till all users are converted, we would like to introduce
checkpatch errors for new patches submitted.

This patch adds support for the same. Tested with a trial patch.

For now, we are only considering usages that don't have compound
nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
This is sufficient as such usages are unlikely.

Once all users are converted and we remove the old API, we can also revert this
checkpatch patch then.

[1] https://lore.kernel.org/rcu/CAEXW_YRhHaVuq+5f+VgCZM=SF+9xO+QXaxe0yE7oA9iCXK-XPg@mail.gmail.com/
[2] https://lore.kernel.org/rcu/CAEXW_YSY=q2_uaE2qo4XSGjzs4+C102YMVJ7kWwuT5LGmJGGew@mail.gmail.com/

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 scripts/checkpatch.pl |  9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c9..9da0a3cb1615 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6388,6 +6388,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 call the API with an rcu_head object passed, like: k[v]free_rcu(object_ptr, rcu_head_name);  " . $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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  3:04 [PATCH] checkpatch: Error out if deprecated RCU API used Joel Fernandes (Google)
@ 2023-03-07  3:08 ` Joe Perches
  2023-03-07  3:10   ` Joel Fernandes
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2023-03-07  3:08 UTC (permalink / raw)
  To: Joel Fernandes (Google), linux-kernel
  Cc: Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn, Paul E. McKenney,
	RCU, Oleksiy Avramchenko, Jens Axboe, Philipp Reisner, Bryan Tan,
	Steven Rostedt, Eric Dumazet, Bob Pearson, Ariel Levkovich,
	Theodore Ts'o, Julian Anastasov, Uladzislau Rezki

On Tue, 2023-03-07 at 03:04 +0000, Joel Fernandes (Google) wrote:
> Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
> error-prone. However, till all users are converted, we would like to introduce
> checkpatch errors for new patches submitted.
> 
> This patch adds support for the same. Tested with a trial patch.
> 
> For now, we are only considering usages that don't have compound
> nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> This is sufficient as such usages are unlikely.
> 
> Once all users are converted and we remove the old API, we can also revert this
> checkpatch patch then.

I think this should be added to the deprecated_apis hash instead

our %deprecated_apis = (
	"synchronize_rcu_bh"			=> "synchronize_rcu",
	"synchronize_rcu_bh_expedited"		=> "synchronize_rcu_expedited",
	"call_rcu_bh"				=> "call_rcu",
	"rcu_barrier_bh"			=> "rcu_barrier",
	"synchronize_sched"			=> "synchronize_rcu",
	"synchronize_sched_expedited"		=> "synchronize_rcu_expedited",
	"call_rcu_sched"			=> "call_rcu",
	"rcu_barrier_sched"			=> "rcu_barrier",
	"get_state_synchronize_sched"		=> "get_state_synchronize_rcu",
	"cond_synchronize_sched"		=> "cond_synchronize_rcu",
	"kmap"					=> "kmap_local_page",
	"kunmap"				=> "kunmap_local",
	"kmap_atomic"				=> "kmap_local_page",
	"kunmap_atomic"				=> "kunmap_local",
);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  3:08 ` Joe Perches
@ 2023-03-07  3:10   ` Joel Fernandes
  2023-03-07  3:23     ` Joe Perches
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Fernandes @ 2023-03-07  3:10 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Mon, Mar 6, 2023 at 10:08 PM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2023-03-07 at 03:04 +0000, Joel Fernandes (Google) wrote:
> > Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
> > error-prone. However, till all users are converted, we would like to introduce
> > checkpatch errors for new patches submitted.
> >
> > This patch adds support for the same. Tested with a trial patch.
> >
> > For now, we are only considering usages that don't have compound
> > nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> > This is sufficient as such usages are unlikely.
> >
> > Once all users are converted and we remove the old API, we can also revert this
> > checkpatch patch then.
>
> I think this should be added to the deprecated_apis hash instead
>
> our %deprecated_apis = (
>         "synchronize_rcu_bh"                    => "synchronize_rcu",
>         "synchronize_rcu_bh_expedited"          => "synchronize_rcu_expedited",
>         "call_rcu_bh"                           => "call_rcu",
>         "rcu_barrier_bh"                        => "rcu_barrier",
>         "synchronize_sched"                     => "synchronize_rcu",
>         "synchronize_sched_expedited"           => "synchronize_rcu_expedited",
>         "call_rcu_sched"                        => "call_rcu",
>         "rcu_barrier_sched"                     => "rcu_barrier",
>         "get_state_synchronize_sched"           => "get_state_synchronize_rcu",
>         "cond_synchronize_sched"                => "cond_synchronize_rcu",
>         "kmap"                                  => "kmap_local_page",
>         "kunmap"                                => "kunmap_local",
>         "kmap_atomic"                           => "kmap_local_page",
>         "kunmap_atomic"                         => "kunmap_local",
> );

This is not an API name change though, it is a "number of arguments"
or argument list change. Is there a different way to do it?

thanks,

 - Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  3:10   ` Joel Fernandes
@ 2023-03-07  3:23     ` Joe Perches
  2023-03-07  4:41       ` Joel Fernandes
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2023-03-07  3:23 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Mon, 2023-03-06 at 22:10 -0500, Joel Fernandes wrote:
> On Mon, Mar 6, 2023 at 10:08 PM Joe Perches <joe@perches.com> wrote:
> > 
> > On Tue, 2023-03-07 at 03:04 +0000, Joel Fernandes (Google) wrote:
> > > Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
> > > error-prone. However, till all users are converted, we would like to introduce
> > > checkpatch errors for new patches submitted.
> > > 
> > > This patch adds support for the same. Tested with a trial patch.
> > > 
> > > For now, we are only considering usages that don't have compound
> > > nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> > > This is sufficient as such usages are unlikely.
> > > 
> > > Once all users are converted and we remove the old API, we can also revert this
> > > checkpatch patch then.
> > 
> > I think this should be added to the deprecated_apis hash instead
> > 
> > our %deprecated_apis = (
> >         "synchronize_rcu_bh"                    => "synchronize_rcu",
> >         "synchronize_rcu_bh_expedited"          => "synchronize_rcu_expedited",
> >         "call_rcu_bh"                           => "call_rcu",
> >         "rcu_barrier_bh"                        => "rcu_barrier",
> >         "synchronize_sched"                     => "synchronize_rcu",
> >         "synchronize_sched_expedited"           => "synchronize_rcu_expedited",
> >         "call_rcu_sched"                        => "call_rcu",
> >         "rcu_barrier_sched"                     => "rcu_barrier",
> >         "get_state_synchronize_sched"           => "get_state_synchronize_rcu",
> >         "cond_synchronize_sched"                => "cond_synchronize_rcu",
> >         "kmap"                                  => "kmap_local_page",
> >         "kunmap"                                => "kunmap_local",
> >         "kmap_atomic"                           => "kmap_local_page",
> >         "kunmap_atomic"                         => "kunmap_local",
> > );
> 
> This is not an API name change though, it is a "number of arguments"
> or argument list change. Is there a different way to do it?

Ah, no, not really.

btw: I don't see a single use of this call without a comma in the tree.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  3:23     ` Joe Perches
@ 2023-03-07  4:41       ` Joel Fernandes
  2023-03-07  4:53         ` Joe Perches
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Fernandes @ 2023-03-07  4:41 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Mon, Mar 06, 2023 at 07:23:23PM -0800, Joe Perches wrote:
> On Mon, 2023-03-06 at 22:10 -0500, Joel Fernandes wrote:
> > On Mon, Mar 6, 2023 at 10:08 PM Joe Perches <joe@perches.com> wrote:
> > > 
> > > On Tue, 2023-03-07 at 03:04 +0000, Joel Fernandes (Google) wrote:
> > > > Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
> > > > error-prone. However, till all users are converted, we would like to introduce
> > > > checkpatch errors for new patches submitted.
> > > > 
> > > > This patch adds support for the same. Tested with a trial patch.
> > > > 
> > > > For now, we are only considering usages that don't have compound
> > > > nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> > > > This is sufficient as such usages are unlikely.
> > > > 
> > > > Once all users are converted and we remove the old API, we can also revert this
> > > > checkpatch patch then.
> > > 
> > > I think this should be added to the deprecated_apis hash instead
> > > 
> > > our %deprecated_apis = (
> > >         "synchronize_rcu_bh"                    => "synchronize_rcu",
> > >         "synchronize_rcu_bh_expedited"          => "synchronize_rcu_expedited",
> > >         "call_rcu_bh"                           => "call_rcu",
> > >         "rcu_barrier_bh"                        => "rcu_barrier",
> > >         "synchronize_sched"                     => "synchronize_rcu",
> > >         "synchronize_sched_expedited"           => "synchronize_rcu_expedited",
> > >         "call_rcu_sched"                        => "call_rcu",
> > >         "rcu_barrier_sched"                     => "rcu_barrier",
> > >         "get_state_synchronize_sched"           => "get_state_synchronize_rcu",
> > >         "cond_synchronize_sched"                => "cond_synchronize_rcu",
> > >         "kmap"                                  => "kmap_local_page",
> > >         "kunmap"                                => "kunmap_local",
> > >         "kmap_atomic"                           => "kmap_local_page",
> > >         "kunmap_atomic"                         => "kunmap_local",
> > > );
> > 
> > This is not an API name change though, it is a "number of arguments"
> > or argument list change. Is there a different way to do it?
> 
> Ah, no, not really.
> 
> btw: I don't see a single use of this call without a comma in the tree.
> 

Did you look for kvfree_rcu? It is either kvfree_rcu() or kfree_rcu().

thanks,

 - Joel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  4:41       ` Joel Fernandes
@ 2023-03-07  4:53         ` Joe Perches
  2023-03-07  5:11           ` Joel Fernandes
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2023-03-07  4:53 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Tue, 2023-03-07 at 04:41 +0000, Joel Fernandes wrote:
> On Mon, Mar 06, 2023 at 07:23:23PM -0800, Joe Perches wrote:
> > On Mon, 2023-03-06 at 22:10 -0500, Joel Fernandes wrote:
> > > On Mon, Mar 6, 2023 at 10:08 PM Joe Perches <joe@perches.com> wrote:
> > > > 
> > > > On Tue, 2023-03-07 at 03:04 +0000, Joel Fernandes (Google) wrote:
> > > > > Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
> > > > > error-prone. However, till all users are converted, we would like to introduce
> > > > > checkpatch errors for new patches submitted.
> > > > > 
> > > > > This patch adds support for the same. Tested with a trial patch.
> > > > > 
> > > > > For now, we are only considering usages that don't have compound
> > > > > nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> > > > > This is sufficient as such usages are unlikely.
> > > > > 
> > > > > Once all users are converted and we remove the old API, we can also revert this
> > > > > checkpatch patch then.
> > > > 
> > > > I think this should be added to the deprecated_apis hash instead
> > > > 
> > > > our %deprecated_apis = (
> > > >         "synchronize_rcu_bh"                    => "synchronize_rcu",
> > > >         "synchronize_rcu_bh_expedited"          => "synchronize_rcu_expedited",
> > > >         "call_rcu_bh"                           => "call_rcu",
> > > >         "rcu_barrier_bh"                        => "rcu_barrier",
> > > >         "synchronize_sched"                     => "synchronize_rcu",
> > > >         "synchronize_sched_expedited"           => "synchronize_rcu_expedited",
> > > >         "call_rcu_sched"                        => "call_rcu",
> > > >         "rcu_barrier_sched"                     => "rcu_barrier",
> > > >         "get_state_synchronize_sched"           => "get_state_synchronize_rcu",
> > > >         "cond_synchronize_sched"                => "cond_synchronize_rcu",
> > > >         "kmap"                                  => "kmap_local_page",
> > > >         "kunmap"                                => "kunmap_local",
> > > >         "kmap_atomic"                           => "kmap_local_page",
> > > >         "kunmap_atomic"                         => "kunmap_local",
> > > > );
> > > 
> > > This is not an API name change though, it is a "number of arguments"
> > > or argument list change. Is there a different way to do it?
> > 
> > Ah, no, not really.
> > 
> > btw: I don't see a single use of this call without a comma in the tree.
> 
> Did you look for kvfree_rcu? It is either kvfree_rcu() or kfree_rcu().

$ git grep -P '\bkv?free_rcu\s*\(' -- '*.[ch]' | grep -v -P 'kv?free_rcu\s*\([^,]+,.*\)'
drivers/infiniband/core/device.c:		kfree_rcu(container_of(dev->port_data, struct ib_port_data_rcu,
drivers/infiniband/core/rdma_core.c:	 * kfree_rcu(). However the object may still have been released and
drivers/target/target_core_configfs.c:			 * callbacks to complete post kfree_rcu(), before allowing
include/linux/rcupdate.h: *     kvfree_rcu(ptr);
include/linux/rcupdate.h:#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,		\
include/linux/rcutiny.h:	// kvfree_rcu(one_arg) call.
include/rdma/ib_verbs.h:	struct rcu_head		rcu;		/* kfree_rcu() overhead */
include/scsi/scsi_device.h: * @rcu: For kfree_rcu().
kernel/rcu/rcuscale.c:		pr_alert("CONFIG_RCU_LAZY is disabled, falling back to kfree_rcu() for delayed RCU kfree'ing\n");
kernel/rcu/tree.c: * struct kvfree_rcu_bulk_data - single block to store kvfree_rcu() pointers
kernel/rcu/tree.c: * @records: Array of the kvfree_rcu() pointers
kernel/rcu/tree.c: * struct kfree_rcu_cpu_work - single batch of kfree_rcu() requests
kernel/rcu/tree.c: * @head_free: List of kfree_rcu() objects waiting for a grace period
kernel/rcu/tree.c: * @bulk_head_free: Bulk-List of kvfree_rcu() objects waiting for a grace period
kernel/rcu/tree.c: * struct kfree_rcu_cpu - batch up kfree_rcu() requests for RCU grace period
kernel/rcu/tree.c: * @head: List of kfree_rcu() objects not yet waiting for a grace period
kernel/rcu/tree.c: * @bulk_head: Bulk-List of kvfree_rcu() objects not yet waiting for a grace period
kernel/rcu/tree.c: * @krw_arr: Array of batches of kfree_rcu() objects waiting for a grace period
kernel/rcu/tree.c:	 * double-argument of kvfree_rcu().  This happens when the
kernel/rcu/tree.c: * reduce the number of grace periods during heavy kfree_rcu()/kvfree_rcu() load.
kernel/rcu/tree.c:		// Probable double kfree_rcu(), just leak.
net/core/pktgen.c:	/* Don't need rcu_barrier() due to use of kfree_rcu() */


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  4:53         ` Joe Perches
@ 2023-03-07  5:11           ` Joel Fernandes
  2023-03-07  5:22             ` Joe Perches
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Fernandes @ 2023-03-07  5:11 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Mon, Mar 6, 2023 at 11:53 PM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2023-03-07 at 04:41 +0000, Joel Fernandes wrote:
> > On Mon, Mar 06, 2023 at 07:23:23PM -0800, Joe Perches wrote:
> > > On Mon, 2023-03-06 at 22:10 -0500, Joel Fernandes wrote:
> > > > On Mon, Mar 6, 2023 at 10:08 PM Joe Perches <joe@perches.com> wrote:
> > > > >
> > > > > On Tue, 2023-03-07 at 03:04 +0000, Joel Fernandes (Google) wrote:
> > > > > > Single-argument kvfree_rcu() usage is being deprecated [1] [2] as it is
> > > > > > error-prone. However, till all users are converted, we would like to introduce
> > > > > > checkpatch errors for new patches submitted.
> > > > > >
> > > > > > This patch adds support for the same. Tested with a trial patch.
> > > > > >
> > > > > > For now, we are only considering usages that don't have compound
> > > > > > nesting, for example ignore: kvfree_rcu( (rcu_head_obj), rcu_head_name).
> > > > > > This is sufficient as such usages are unlikely.
> > > > > >
> > > > > > Once all users are converted and we remove the old API, we can also revert this
> > > > > > checkpatch patch then.
> > > > >
> > > > > I think this should be added to the deprecated_apis hash instead
> > > > >
> > > > > our %deprecated_apis = (
> > > > >         "synchronize_rcu_bh"                    => "synchronize_rcu",
> > > > >         "synchronize_rcu_bh_expedited"          => "synchronize_rcu_expedited",
> > > > >         "call_rcu_bh"                           => "call_rcu",
> > > > >         "rcu_barrier_bh"                        => "rcu_barrier",
> > > > >         "synchronize_sched"                     => "synchronize_rcu",
> > > > >         "synchronize_sched_expedited"           => "synchronize_rcu_expedited",
> > > > >         "call_rcu_sched"                        => "call_rcu",
> > > > >         "rcu_barrier_sched"                     => "rcu_barrier",
> > > > >         "get_state_synchronize_sched"           => "get_state_synchronize_rcu",
> > > > >         "cond_synchronize_sched"                => "cond_synchronize_rcu",
> > > > >         "kmap"                                  => "kmap_local_page",
> > > > >         "kunmap"                                => "kunmap_local",
> > > > >         "kmap_atomic"                           => "kmap_local_page",
> > > > >         "kunmap_atomic"                         => "kunmap_local",
> > > > > );
> > > >
> > > > This is not an API name change though, it is a "number of arguments"
> > > > or argument list change. Is there a different way to do it?
> > >
> > > Ah, no, not really.
> > >
> > > btw: I don't see a single use of this call without a comma in the tree.
> >
> > Did you look for kvfree_rcu? It is either kvfree_rcu() or kfree_rcu().
>
> $ git grep -P '\bkv?free_rcu\s*\(' -- '*.[ch]' | grep -v -P 'kv?free_rcu\s*\([^,]+,.*\)'
> drivers/infiniband/core/device.c:               kfree_rcu(container_of(dev->port_data, struct ib_port_data_rcu,
> drivers/infiniband/core/rdma_core.c:     * kfree_rcu(). However the object may still have been released and
> drivers/target/target_core_configfs.c:                   * callbacks to complete post kfree_rcu(), before allowing
> include/linux/rcupdate.h: *     kvfree_rcu(ptr);
> include/linux/rcupdate.h:#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,          \
> include/linux/rcutiny.h:        // kvfree_rcu(one_arg) call.
> include/rdma/ib_verbs.h:        struct rcu_head         rcu;            /* kfree_rcu() overhead */
> include/scsi/scsi_device.h: * @rcu: For kfree_rcu().
> kernel/rcu/rcuscale.c:          pr_alert("CONFIG_RCU_LAZY is disabled, falling back to kfree_rcu() for delayed RCU kfree'ing\n");
> kernel/rcu/tree.c: * struct kvfree_rcu_bulk_data - single block to store kvfree_rcu() pointers
> kernel/rcu/tree.c: * @records: Array of the kvfree_rcu() pointers
> kernel/rcu/tree.c: * struct kfree_rcu_cpu_work - single batch of kfree_rcu() requests
> kernel/rcu/tree.c: * @head_free: List of kfree_rcu() objects waiting for a grace period
> kernel/rcu/tree.c: * @bulk_head_free: Bulk-List of kvfree_rcu() objects waiting for a grace period
> kernel/rcu/tree.c: * struct kfree_rcu_cpu - batch up kfree_rcu() requests for RCU grace period
> kernel/rcu/tree.c: * @head: List of kfree_rcu() objects not yet waiting for a grace period
> kernel/rcu/tree.c: * @bulk_head: Bulk-List of kvfree_rcu() objects not yet waiting for a grace period
> kernel/rcu/tree.c: * @krw_arr: Array of batches of kfree_rcu() objects waiting for a grace period
> kernel/rcu/tree.c:       * double-argument of kvfree_rcu().  This happens when the
> kernel/rcu/tree.c: * reduce the number of grace periods during heavy kfree_rcu()/kvfree_rcu() load.
> kernel/rcu/tree.c:              // Probable double kfree_rcu(), just leak.
> net/core/pktgen.c:      /* Don't need rcu_barrier() due to use of kfree_rcu() */
>

Do you mind sharing which tree you are looking at? I checked both
6.3-rc1 and linux-next.

Your grep returned:

kernel/trace/trace_osnoise.c:   kvfree_rcu(inst);
kernel/trace/trace_probe.c:     kvfree_rcu(link);
lib/test_vmalloc.c:             kvfree_rcu(p);
mm/list_lru.c:   * We need kvfree_rcu() here. And the walking of the list
net/core/pktgen.c:      /* Don't need rcu_barrier() due to use of kfree_rcu() */
net/core/sysctl_net_core.c:
kvfree_rcu(orig_sock_table);
net/core/sysctl_net_core.c:                             kfree_rcu(cur);
net/mac802154/scan.c:   kfree_rcu(request);
net/mac802154/scan.c:   kfree_rcu(request);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  5:11           ` Joel Fernandes
@ 2023-03-07  5:22             ` Joe Perches
  2023-03-07  5:32               ` Joel Fernandes
  0 siblings, 1 reply; 9+ messages in thread
From: Joe Perches @ 2023-03-07  5:22 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Tue, 2023-03-07 at 00:11 -0500, Joel Fernandes wrote:

> Do you mind sharing which tree you are looking at? I checked both
> 6.3-rc1 and linux-next.
> 
> Your grep returned:
> 
> kernel/trace/trace_osnoise.c:   kvfree_rcu(inst);
> kernel/trace/trace_probe.c:     kvfree_rcu(link);
> lib/test_vmalloc.c:             kvfree_rcu(p);
> mm/list_lru.c:   * We need kvfree_rcu() here. And the walking of the list
> net/core/pktgen.c:      /* Don't need rcu_barrier() due to use of kfree_rcu() */
> net/core/sysctl_net_core.c:
> kvfree_rcu(orig_sock_table);
> net/core/sysctl_net_core.c:                             kfree_rcu(cur);
> net/mac802154/scan.c:   kfree_rcu(request);
> net/mac802154/scan.c:   kfree_rcu(request);

rather old.  I'm not subscribed and haven't been following much.

Add linux-next specific files for 20230217

Updating to today's next:

Add linux-next specific files for 20230307

I get several instances:

$ git grep -P '\bkv?free_rcu\s*\(' -- '*.[ch]' | grep -v -P 'kv?free_rcu\s*\([^,]+,.*\)'
drivers/block/drbd/drbd_nl.c:	kvfree_rcu(old_disk_conf);
drivers/block/drbd/drbd_nl.c:	kvfree_rcu(old_net_conf);
drivers/block/drbd/drbd_nl.c:		kvfree_rcu(old_disk_conf);
drivers/block/drbd/drbd_receiver.c:	kvfree_rcu(old_net_conf);
drivers/block/drbd/drbd_receiver.c:			kvfree_rcu(old_disk_conf);
drivers/block/drbd/drbd_state.c:		kvfree_rcu(old_conf);
drivers/infiniband/core/device.c:		kfree_rcu(container_of(dev->port_data, struct ib_port_data_rcu,
drivers/infiniband/core/rdma_core.c:	 * kfree_rcu(). However the object may still have been released and
drivers/infiniband/sw/rxe/rxe_mr.c:	kfree_rcu(mr);
drivers/misc/vmw_vmci/vmci_context.c:		kvfree_rcu(notifier);
drivers/misc/vmw_vmci/vmci_event.c:	kvfree_rcu(s);
drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c:	kfree_rcu(int_port);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c:	kfree_rcu(tx_sa);
drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c:	kfree_rcu(rx_sc);
drivers/target/target_core_configfs.c:			 * callbacks to complete post kfree_rcu(), before allowing
fs/ext4/super.c:				kfree_rcu(qname);
include/linux/rcupdate.h: *     kvfree_rcu(ptr);
include/linux/rcupdate.h:#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,		\
include/linux/rcutiny.h:	// kvfree_rcu(one_arg) call.
include/rdma/ib_verbs.h:	struct rcu_head		rcu;		/* kfree_rcu() overhead */
include/scsi/scsi_device.h: * @rcu: For kfree_rcu().
kernel/rcu/rcuscale.c:		pr_alert("CONFIG_RCU_LAZY is disabled, falling back to kfree_rcu() for delayed RCU kfree'ing\n");
kernel/rcu/tree.c: * struct kvfree_rcu_bulk_data - single block to store kvfree_rcu() pointers
kernel/rcu/tree.c: * @records: Array of the kvfree_rcu() pointers
kernel/rcu/tree.c: * struct kfree_rcu_cpu_work - single batch of kfree_rcu() requests
kernel/rcu/tree.c: * @head_free: List of kfree_rcu() objects waiting for a grace period
kernel/rcu/tree.c: * @bulk_head_free: Bulk-List of kvfree_rcu() objects waiting for a grace period
kernel/rcu/tree.c: * struct kfree_rcu_cpu - batch up kfree_rcu() requests for RCU grace period
kernel/rcu/tree.c: * @head: List of kfree_rcu() objects not yet waiting for a grace period
kernel/rcu/tree.c: * @bulk_head: Bulk-List of kvfree_rcu() objects not yet waiting for a grace period
kernel/rcu/tree.c: * @krw_arr: Array of batches of kfree_rcu() objects waiting for a grace period
kernel/rcu/tree.c:	 * double-argument of kvfree_rcu().  This happens when the
kernel/rcu/tree.c: * reduce the number of grace periods during heavy kfree_rcu()/kvfree_rcu() load.
kernel/rcu/tree.c:		// Probable double kfree_rcu(), just leak.
kernel/trace/trace_osnoise.c:	kvfree_rcu(inst);
kernel/trace/trace_probe.c:	kvfree_rcu(link);
net/core/pktgen.c:	/* Don't need rcu_barrier() due to use of kfree_rcu() */
net/core/sysctl_net_core.c:				kvfree_rcu(orig_sock_table);
net/core/sysctl_net_core.c:				kfree_rcu(cur);


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] checkpatch: Error out if deprecated RCU API used
  2023-03-07  5:22             ` Joe Perches
@ 2023-03-07  5:32               ` Joel Fernandes
  0 siblings, 0 replies; 9+ messages in thread
From: Joel Fernandes @ 2023-03-07  5:32 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, Andy Whitcroft, Dwaipayan Ray, Lukas Bulwahn,
	Paul E. McKenney, RCU, Oleksiy Avramchenko, Jens Axboe,
	Philipp Reisner, Bryan Tan, Steven Rostedt, Eric Dumazet,
	Bob Pearson, Ariel Levkovich, Theodore Ts'o, Julian Anastasov,
	Uladzislau Rezki

On Tue, Mar 7, 2023 at 12:22 AM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2023-03-07 at 00:11 -0500, Joel Fernandes wrote:
>
> > Do you mind sharing which tree you are looking at? I checked both
> > 6.3-rc1 and linux-next.
> >
> > Your grep returned:
> >
> > kernel/trace/trace_osnoise.c:   kvfree_rcu(inst);
> > kernel/trace/trace_probe.c:     kvfree_rcu(link);
> > lib/test_vmalloc.c:             kvfree_rcu(p);
> > mm/list_lru.c:   * We need kvfree_rcu() here. And the walking of the list
> > net/core/pktgen.c:      /* Don't need rcu_barrier() due to use of kfree_rcu() */
> > net/core/sysctl_net_core.c:
> > kvfree_rcu(orig_sock_table);
> > net/core/sysctl_net_core.c:                             kfree_rcu(cur);
> > net/mac802154/scan.c:   kfree_rcu(request);
> > net/mac802154/scan.c:   kfree_rcu(request);
>
> rather old.  I'm not subscribed and haven't been following much.
>
> Add linux-next specific files for 20230217

I am surprised though why you don't see the usage in trace_osnoise.c
though because that was added in 2021 (see diff below).

Anyway, I take it you are Ok with the checkpatch patch. If so, do
provide your Ack tag in advance. We can push from our side only if
needed. There is a chance that we may not need it if we are successful
in having made the conversions to the "good API" in time for the next
merge window.

diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
index 7520d43aed55..4719a848bf17 100644
--- a/kernel/trace/trace_osnoise.c
+++ b/kernel/trace/trace_osnoise.c
@@ -138,8 +138,7 @@ static void osnoise_unregister_instance(struct
trace_array *tr)
        if (!found)
                return;

-       synchronize_rcu();
-       kfree(inst);
+       kvfree_rcu(inst);
 }

 /*


>
> Updating to today's next:
>
> Add linux-next specific files for 20230307
>
> I get several instances:
>
> $ git grep -P '\bkv?free_rcu\s*\(' -- '*.[ch]' | grep -v -P 'kv?free_rcu\s*\([^,]+,.*\)'
> drivers/block/drbd/drbd_nl.c:   kvfree_rcu(old_disk_conf);
> drivers/block/drbd/drbd_nl.c:   kvfree_rcu(old_net_conf);
> drivers/block/drbd/drbd_nl.c:           kvfree_rcu(old_disk_conf);
> drivers/block/drbd/drbd_receiver.c:     kvfree_rcu(old_net_conf);
> drivers/block/drbd/drbd_receiver.c:                     kvfree_rcu(old_disk_conf);
> drivers/block/drbd/drbd_state.c:                kvfree_rcu(old_conf);
> drivers/infiniband/core/device.c:               kfree_rcu(container_of(dev->port_data, struct ib_port_data_rcu,
> drivers/infiniband/core/rdma_core.c:     * kfree_rcu(). However the object may still have been released and
> drivers/infiniband/sw/rxe/rxe_mr.c:     kfree_rcu(mr);
> drivers/misc/vmw_vmci/vmci_context.c:           kvfree_rcu(notifier);
> drivers/misc/vmw_vmci/vmci_event.c:     kvfree_rcu(s);
> drivers/net/ethernet/mellanox/mlx5/core/en/tc/int_port.c:       kfree_rcu(int_port);
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c:      kfree_rcu(tx_sa);
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c:      kfree_rcu(rx_sc);
> drivers/target/target_core_configfs.c:                   * callbacks to complete post kfree_rcu(), before allowing
> fs/ext4/super.c:                                kfree_rcu(qname);
> include/linux/rcupdate.h: *     kvfree_rcu(ptr);
> include/linux/rcupdate.h:#define kvfree_rcu(...) KVFREE_GET_MACRO(__VA_ARGS__,          \
> include/linux/rcutiny.h:        // kvfree_rcu(one_arg) call.
> include/rdma/ib_verbs.h:        struct rcu_head         rcu;            /* kfree_rcu() overhead */
> include/scsi/scsi_device.h: * @rcu: For kfree_rcu().
> kernel/rcu/rcuscale.c:          pr_alert("CONFIG_RCU_LAZY is disabled, falling back to kfree_rcu() for delayed RCU kfree'ing\n");
> kernel/rcu/tree.c: * struct kvfree_rcu_bulk_data - single block to store kvfree_rcu() pointers
> kernel/rcu/tree.c: * @records: Array of the kvfree_rcu() pointers
> kernel/rcu/tree.c: * struct kfree_rcu_cpu_work - single batch of kfree_rcu() requests
> kernel/rcu/tree.c: * @head_free: List of kfree_rcu() objects waiting for a grace period
> kernel/rcu/tree.c: * @bulk_head_free: Bulk-List of kvfree_rcu() objects waiting for a grace period
> kernel/rcu/tree.c: * struct kfree_rcu_cpu - batch up kfree_rcu() requests for RCU grace period
> kernel/rcu/tree.c: * @head: List of kfree_rcu() objects not yet waiting for a grace period
> kernel/rcu/tree.c: * @bulk_head: Bulk-List of kvfree_rcu() objects not yet waiting for a grace period
> kernel/rcu/tree.c: * @krw_arr: Array of batches of kfree_rcu() objects waiting for a grace period
> kernel/rcu/tree.c:       * double-argument of kvfree_rcu().  This happens when the
> kernel/rcu/tree.c: * reduce the number of grace periods during heavy kfree_rcu()/kvfree_rcu() load.
> kernel/rcu/tree.c:              // Probable double kfree_rcu(), just leak.
> kernel/trace/trace_osnoise.c:   kvfree_rcu(inst);
> kernel/trace/trace_probe.c:     kvfree_rcu(link);
> net/core/pktgen.c:      /* Don't need rcu_barrier() due to use of kfree_rcu() */
> net/core/sysctl_net_core.c:                             kvfree_rcu(orig_sock_table);
> net/core/sysctl_net_core.c:                             kfree_rcu(cur);
>

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-03-07  5:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-07  3:04 [PATCH] checkpatch: Error out if deprecated RCU API used Joel Fernandes (Google)
2023-03-07  3:08 ` Joe Perches
2023-03-07  3:10   ` Joel Fernandes
2023-03-07  3:23     ` Joe Perches
2023-03-07  4:41       ` Joel Fernandes
2023-03-07  4:53         ` Joe Perches
2023-03-07  5:11           ` Joel Fernandes
2023-03-07  5:22             ` Joe Perches
2023-03-07  5:32               ` Joel Fernandes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.