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