Re: [PATCH rcu 5/5] checkpatch: Complain about unexpected uses of RCU Tasks Trace

[Paul E. McKenney]

On Thu, Jul 20, 2023 at 12:43:55PM -0700, Paul E. McKenney wrote:
> On Thu, Jul 20, 2023 at 12:29:42AM -0700, Joe Perches wrote:
> > On Mon, 2023-07-17 at 16:34 -0700, Paul E. McKenney wrote:
> > > On Mon, Jul 17, 2023 at 03:34:14PM -0700, Joe Perches wrote:
> > > > On Mon, 2023-07-17 at 11:04 -0700, Paul E. McKenney wrote:
> > > > > RCU Tasks Trace is quite specialized, having been created specifically
> > > > > for sleepable BPF programs.  Because it allows general blocking within
> > > > > readers, any new use of RCU Tasks Trace must take current use cases into
> > > > > account.  Therefore, update checkpatch.pl to complain about use of any of
> > > > > the RCU Tasks Trace API members outside of BPF and outside of RCU itself.
> > > > > 
> > > > > Cc: Andy Whitcroft <apw@xxxxxxxxxxxxx> (maintainer:CHECKPATCH)
> > > > > Cc: Joe Perches <joe@xxxxxxxxxxx> (maintainer:CHECKPATCH)
> > > > > Cc: Dwaipayan Ray <dwaipayanray1@xxxxxxxxx> (reviewer:CHECKPATCH)
> > > > > Cc: Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx>
> > > > > Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> > > > > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> > > > > Cc: John Fastabend <john.fastabend@xxxxxxxxx>
> > > > > Cc: <bpf@xxxxxxxxxxxxxxx>
> > > > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > > > > ---
> > > > >  scripts/checkpatch.pl | 18 ++++++++++++++++++
> > > > >  1 file changed, 18 insertions(+)
> > > > > 
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > []
> > > > > @@ -7457,6 +7457,24 @@ sub process {
> > > > >  			}
> > > > >  		}
> > > > >  
> > > > > +# Complain about RCU Tasks Trace used outside of BPF (and of course, RCU).
> > > > > +		if ($line =~ /\brcu_read_lock_trace\s*\(/ ||
> > > > > +		    $line =~ /\brcu_read_lock_trace_held\s*\(/ ||
> > > > > +		    $line =~ /\brcu_read_unlock_trace\s*\(/ ||
> > > > > +		    $line =~ /\bcall_rcu_tasks_trace\s*\(/ ||
> > > > > +		    $line =~ /\bsynchronize_rcu_tasks_trace\s*\(/ ||
> > > > > +		    $line =~ /\brcu_barrier_tasks_trace\s*\(/ ||
> > > > > +		    $line =~ /\brcu_request_urgent_qs_task\s*\(/) {
> > > > > +			if ($realfile !~ m@^kernel/bpf@ &&
> > > > > +			    $realfile !~ m@^include/linux/bpf@ &&
> > > > > +			    $realfile !~ m@^net/bpf@ &&
> > > > > +			    $realfile !~ m@^kernel/rcu@ &&
> > > > > +			    $realfile !~ m@^include/linux/rcu@) {
> > > > 
> > > > Functions and paths like these tend to be accreted.
> > > > 
> > > > Please use a variable or 2 like:
> > > > 
> > > > our $rcu_trace_funcs = qr{(?x:
> > > > 	rcu_read_lock_trace |
> > > > 	rcu_read_lock_trace_held |
> > > > 	rcu_read_unlock_trace |
> > > > 	call_rcu_tasks_trace |
> > > > 	synchronize_rcu_tasks_trace |
> > > > 	rcu_barrier_tasks_trace |
> > > > 	rcu_request_urgent_qs_task
> > > > )};
> > > > our $rcu_trace_paths = qr{(?x:
> > > > 	kernel/bfp/ |
> > 		^^
> > 	kernel/bfp/ |
> > 
> > (umm, oops...)
> > I think my original suggestion works better when I don't typo the path.
> 
> Color me blind!  ;-)
> 
> That works much better, thank you!  I will update the patch on my
> next rebase.

As shown below.  Is this what you had in mind?

							Thanx, Paul

------------------------------------------------------------------------

commit 496aa3821b40459b107f4bbc14ca867daad21fb6
Author: Paul E. McKenney <paulmck@xxxxxxxxxx>
Date:   Thu Jul 6 11:48:07 2023 -0700

    checkpatch: Complain about unexpected uses of RCU Tasks Trace
    
    RCU Tasks Trace is quite specialized, having been created specifically
    for sleepable BPF programs.  Because it allows general blocking within
    readers, any new use of RCU Tasks Trace must take current use cases into
    account.  Therefore, update checkpatch.pl to complain about use of any of
    the RCU Tasks Trace API members outside of BPF and outside of RCU itself.
    
    [ paulmck: Apply Joe Perches feedback. ]
    
    Cc: Andy Whitcroft <apw@xxxxxxxxxxxxx> (maintainer:CHECKPATCH)
    Cc: Joe Perches <joe@xxxxxxxxxxx> (maintainer:CHECKPATCH)
    Cc: Dwaipayan Ray <dwaipayanray1@xxxxxxxxx> (reviewer:CHECKPATCH)
    Cc: Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx>
    Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
    Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
    Cc: John Fastabend <john.fastabend@xxxxxxxxx>
    Cc: <bpf@xxxxxxxxxxxxxxx>
    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 880fde13d9b8..a67e682e896c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7457,6 +7457,30 @@ sub process {
 			}
 		}
 
+# Complain about RCU Tasks Trace used outside of BPF (and of course, RCU).
+		our $rcu_trace_funcs = qr{(?x:
+			rcu_read_lock_trace |
+			rcu_read_lock_trace_held |
+			rcu_read_unlock_trace |
+			call_rcu_tasks_trace |
+			synchronize_rcu_tasks_trace |
+			rcu_barrier_tasks_trace |
+			rcu_request_urgent_qs_task
+		)};
+		our $rcu_trace_paths = qr{(?x:
+			kernel/bpf/ |
+			include/linux/bpf |
+			net/bpf/ |
+			kernel/rcu/ |
+			include/linux/rcu
+		)};
+		if ($line =~ /\b$rcu_trace_funcs\s*\(/) {
+			if ($realfile !~ m@^$rcu_trace_paths@) {
+				WARN("RCU_TASKS_TRACE",
+				     "use of RCU tasks trace is incorrect outside BPF or core RCU code\n" . $herecurr);
+			}
+		}
+
 # check for lockdep_set_novalidate_class
 		if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ ||
 		    $line =~ /__lockdep_no_validate__\s*\)/ ) {