RE: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep()

Posted by Zhuo, Qiuxu 2 years, 6 months ago

> From: Frederic Weisbecker <frederic@kernel.org>
> Sent: Thursday, February 23, 2023 8:45 PM
> To: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Cc: LKML <linux-kernel@vger.kernel.org>; RCU <rcu@vger.kernel.org>; Paul
> E . McKenney <paulmck@kernel.org>; Oleksiy Avramchenko
> <oleksiy.avramchenko@sony.com>; Jens Axboe <axboe@kernel.dk>; Philipp
> Reisner <philipp.reisner@linbit.com>; Bryan Tan <bryantan@vmware.com>;
> Steven Rostedt <rostedt@goodmis.org>; Eric Dumazet
> <edumazet@google.com>; Bob Pearson <rpearsonhpe@gmail.com>; Ariel
> Levkovich <lariel@nvidia.com>; Theodore Ts'o <tytso@mit.edu>; Julian
> Anastasov <ja@ssi.bg>
> Subject: Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to
> k[v]free_rcu_mightsleep()
> 
> On Wed, Feb 01, 2023 at 04:08:06PM +0100, Uladzislau Rezki (Sony) wrote:
> > This small series is based on Paul's "dev" branch. Head is
> > 6002817348a1c610dc1b1c01ff81654cdec12be4
> > it renames a single argument of k[v]free_rcu() to its new
> k[v]free_rcu_mightsleep() name.
> >
> > 1.
> > The problem is that, recently we have run into a precedent when a user
> > intended to give a second argument to kfree_rcu() API but forgot to do
> > it in a code so a call became as a single argument of kfree_rcu() API.
> >
> > 2.
> > Such mistyping can lead to hidden bags where sleeping is forbidden.
> >
> > 3.
> > _mightsleep() prefix gives much more information for which contexts it
> > can be used for.
> >
> > Uladzislau Rezki (Sony) (13):
> >   rcu/kvfree: Add kvfree_rcu_mightsleep() and kfree_rcu_mightsleep()
> >   drbd: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> >   misc: vmw_vmci: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> >   tracing: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> >   lib/test_vmalloc.c: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> >   net/sysctl: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> >   RDMA/rxe: Rename kfree_rcu() to kfree_rcu_mightsleep()
> >   net/mlx5: Rename kfree_rcu() to kfree_rcu_mightsleep()
> >   ext4/super: Rename kfree_rcu() to kfree_rcu_mightsleep()
> >   ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()
> >   rcuscale: Rename kfree_rcu() to kfree_rcu_mightsleep()
> >   doc: Update whatisRCU.rst
> >   rcu/kvfree: Eliminate k[v]free_rcu() single argument macro
> 
> Hi,
> 
> Not sure if you guys noticed but on latest rcu/dev:
> 
> net/netfilter/ipvs/ip_vs_est.c: In function ‘ip_vs_stop_estimator’:
> net/netfilter/ipvs/ip_vs_est.c:552:15: error: macro "kfree_rcu" requires 2
> arguments, but only 1 given
>    kfree_rcu(td);
>                ^
> net/netfilter/ipvs/ip_vs_est.c:552:3: error: ‘kfree_rcu’ undeclared (first use in
> this function); did you mean ‘kfree_skb’?
>    kfree_rcu(td);
>    ^~~~~~~~~
>    kfree_skb
> net/netfilter/ipvs/ip_vs_est.c:552:3: note: each undeclared identifier is
> reported only once for each function it appears in
> 

Hi Frederic Weisbecker,

I encountered the same build error as yours. 
Per the discussion link below, the fix for this build error by Uladzislau Rezki will be picked up by some other maintainer's branch?
@Paul E . McKenney, please correct me if my understanding is wrong. 😊

    https://lore.kernel.org/rcu/Y9qc+lgR1CgdszKs@salvia/

-Qiuxu

Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep()

Posted by Paul E. McKenney 2 years, 6 months ago

On Thu, Feb 23, 2023 at 02:29:42PM +0000, Zhuo, Qiuxu wrote:
> > From: Frederic Weisbecker <frederic@kernel.org>
> > Sent: Thursday, February 23, 2023 8:45 PM
> > To: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > Cc: LKML <linux-kernel@vger.kernel.org>; RCU <rcu@vger.kernel.org>; Paul
> > E . McKenney <paulmck@kernel.org>; Oleksiy Avramchenko
> > <oleksiy.avramchenko@sony.com>; Jens Axboe <axboe@kernel.dk>; Philipp
> > Reisner <philipp.reisner@linbit.com>; Bryan Tan <bryantan@vmware.com>;
> > Steven Rostedt <rostedt@goodmis.org>; Eric Dumazet
> > <edumazet@google.com>; Bob Pearson <rpearsonhpe@gmail.com>; Ariel
> > Levkovich <lariel@nvidia.com>; Theodore Ts'o <tytso@mit.edu>; Julian
> > Anastasov <ja@ssi.bg>
> > Subject: Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to
> > k[v]free_rcu_mightsleep()
> > 
> > On Wed, Feb 01, 2023 at 04:08:06PM +0100, Uladzislau Rezki (Sony) wrote:
> > > This small series is based on Paul's "dev" branch. Head is
> > > 6002817348a1c610dc1b1c01ff81654cdec12be4
> > > it renames a single argument of k[v]free_rcu() to its new
> > k[v]free_rcu_mightsleep() name.
> > >
> > > 1.
> > > The problem is that, recently we have run into a precedent when a user
> > > intended to give a second argument to kfree_rcu() API but forgot to do
> > > it in a code so a call became as a single argument of kfree_rcu() API.
> > >
> > > 2.
> > > Such mistyping can lead to hidden bags where sleeping is forbidden.
> > >
> > > 3.
> > > _mightsleep() prefix gives much more information for which contexts it
> > > can be used for.
> > >
> > > Uladzislau Rezki (Sony) (13):
> > >   rcu/kvfree: Add kvfree_rcu_mightsleep() and kfree_rcu_mightsleep()
> > >   drbd: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> > >   misc: vmw_vmci: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> > >   tracing: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> > >   lib/test_vmalloc.c: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> > >   net/sysctl: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
> > >   RDMA/rxe: Rename kfree_rcu() to kfree_rcu_mightsleep()
> > >   net/mlx5: Rename kfree_rcu() to kfree_rcu_mightsleep()
> > >   ext4/super: Rename kfree_rcu() to kfree_rcu_mightsleep()
> > >   ipvs: Rename kfree_rcu() to kfree_rcu_mightsleep()
> > >   rcuscale: Rename kfree_rcu() to kfree_rcu_mightsleep()
> > >   doc: Update whatisRCU.rst
> > >   rcu/kvfree: Eliminate k[v]free_rcu() single argument macro
> > 
> > Hi,
> > 
> > Not sure if you guys noticed but on latest rcu/dev:
> > 
> > net/netfilter/ipvs/ip_vs_est.c: In function ‘ip_vs_stop_estimator’:
> > net/netfilter/ipvs/ip_vs_est.c:552:15: error: macro "kfree_rcu" requires 2
> > arguments, but only 1 given
> >    kfree_rcu(td);
> >                ^
> > net/netfilter/ipvs/ip_vs_est.c:552:3: error: ‘kfree_rcu’ undeclared (first use in
> > this function); did you mean ‘kfree_skb’?
> >    kfree_rcu(td);
> >    ^~~~~~~~~
> >    kfree_skb
> > net/netfilter/ipvs/ip_vs_est.c:552:3: note: each undeclared identifier is
> > reported only once for each function it appears in
> 
> Hi Frederic Weisbecker,
> 
> I encountered the same build error as yours. 
> Per the discussion link below, the fix for this build error by Uladzislau Rezki will be picked up by some other maintainer's branch?
> @Paul E . McKenney, please correct me if my understanding is wrong. 😊
> 
>     https://lore.kernel.org/rcu/Y9qc+lgR1CgdszKs@salvia/

Pablo and Julian, how are things coming with that patch?

							Thanx, Paul

Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep()

Posted by Julian Anastasov 2 years, 6 months ago

	Hello,

On Thu, 23 Feb 2023, Paul E. McKenney wrote:

> > > Not sure if you guys noticed but on latest rcu/dev:
> > > 
> > > net/netfilter/ipvs/ip_vs_est.c: In function ‘ip_vs_stop_estimator’:
> > > net/netfilter/ipvs/ip_vs_est.c:552:15: error: macro "kfree_rcu" requires 2
> > > arguments, but only 1 given
> > >    kfree_rcu(td);
> > >                ^
> > > net/netfilter/ipvs/ip_vs_est.c:552:3: error: ‘kfree_rcu’ undeclared (first use in
> > > this function); did you mean ‘kfree_skb’?
> > >    kfree_rcu(td);
> > >    ^~~~~~~~~
> > >    kfree_skb
> > > net/netfilter/ipvs/ip_vs_est.c:552:3: note: each undeclared identifier is
> > > reported only once for each function it appears in
> > 
> > Hi Frederic Weisbecker,
> > 
> > I encountered the same build error as yours. 
> > Per the discussion link below, the fix for this build error by Uladzislau Rezki will be picked up by some other maintainer's branch?
> > @Paul E . McKenney, please correct me if my understanding is wrong. 😊
> > 
> >     https://lore.kernel.org/rcu/Y9qc+lgR1CgdszKs@salvia/
> 
> Pablo and Julian, how are things coming with that patch?

	Fix is already in net and net-next tree

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep()

Posted by Paul E. McKenney 2 years, 6 months ago

On Thu, Feb 23, 2023 at 06:21:46PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 23 Feb 2023, Paul E. McKenney wrote:
> 
> > > > Not sure if you guys noticed but on latest rcu/dev:
> > > > 
> > > > net/netfilter/ipvs/ip_vs_est.c: In function ‘ip_vs_stop_estimator’:
> > > > net/netfilter/ipvs/ip_vs_est.c:552:15: error: macro "kfree_rcu" requires 2
> > > > arguments, but only 1 given
> > > >    kfree_rcu(td);
> > > >                ^
> > > > net/netfilter/ipvs/ip_vs_est.c:552:3: error: ‘kfree_rcu’ undeclared (first use in
> > > > this function); did you mean ‘kfree_skb’?
> > > >    kfree_rcu(td);
> > > >    ^~~~~~~~~
> > > >    kfree_skb
> > > > net/netfilter/ipvs/ip_vs_est.c:552:3: note: each undeclared identifier is
> > > > reported only once for each function it appears in
> > > 
> > > Hi Frederic Weisbecker,
> > > 
> > > I encountered the same build error as yours. 
> > > Per the discussion link below, the fix for this build error by Uladzislau Rezki will be picked up by some other maintainer's branch?
> > > @Paul E . McKenney, please correct me if my understanding is wrong. 😊
> > > 
> > >     https://lore.kernel.org/rcu/Y9qc+lgR1CgdszKs@salvia/
> > 
> > Pablo and Julian, how are things coming with that patch?
> 
> 	Fix is already in net and net-next tree

Very good, thank you!  Is this going into this merge window or is it
expected to wait for v6.4?

							Thanx, Paul

Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep()

Posted by Pablo Neira Ayuso 2 years, 6 months ago

On Thu, Feb 23, 2023 at 09:14:32AM -0800, Paul E. McKenney wrote:
> On Thu, Feb 23, 2023 at 06:21:46PM +0200, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Thu, 23 Feb 2023, Paul E. McKenney wrote:
> > 
> > > > > Not sure if you guys noticed but on latest rcu/dev:
> > > > > 
> > > > > net/netfilter/ipvs/ip_vs_est.c: In function ‘ip_vs_stop_estimator’:
> > > > > net/netfilter/ipvs/ip_vs_est.c:552:15: error: macro "kfree_rcu" requires 2
> > > > > arguments, but only 1 given
> > > > >    kfree_rcu(td);
> > > > >                ^
> > > > > net/netfilter/ipvs/ip_vs_est.c:552:3: error: ‘kfree_rcu’ undeclared (first use in
> > > > > this function); did you mean ‘kfree_skb’?
> > > > >    kfree_rcu(td);
> > > > >    ^~~~~~~~~
> > > > >    kfree_skb
> > > > > net/netfilter/ipvs/ip_vs_est.c:552:3: note: each undeclared identifier is
> > > > > reported only once for each function it appears in
> > > > 
> > > > Hi Frederic Weisbecker,
> > > > 
> > > > I encountered the same build error as yours. 
> > > > Per the discussion link below, the fix for this build error by Uladzislau Rezki will be picked up by some other maintainer's branch?
> > > > @Paul E . McKenney, please correct me if my understanding is wrong. 😊
> > > > 
> > > >     https://lore.kernel.org/rcu/Y9qc+lgR1CgdszKs@salvia/
> > > 
> > > Pablo and Julian, how are things coming with that patch?
> > 
> > 	Fix is already in net and net-next tree
> 
> Very good, thank you!  Is this going into this merge window or is it
> expected to wait for v6.4?

this merge window. 							Thanx, Paul

Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu_mightsleep()

Posted by Paul E. McKenney 2 years, 6 months ago

On Thu, Feb 23, 2023 at 06:36:26PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Feb 23, 2023 at 09:14:32AM -0800, Paul E. McKenney wrote:
> > On Thu, Feb 23, 2023 at 06:21:46PM +0200, Julian Anastasov wrote:
> > > 
> > > 	Hello,
> > > 
> > > On Thu, 23 Feb 2023, Paul E. McKenney wrote:
> > > 
> > > > > > Not sure if you guys noticed but on latest rcu/dev:
> > > > > > 
> > > > > > net/netfilter/ipvs/ip_vs_est.c: In function ‘ip_vs_stop_estimator’:
> > > > > > net/netfilter/ipvs/ip_vs_est.c:552:15: error: macro "kfree_rcu" requires 2
> > > > > > arguments, but only 1 given
> > > > > >    kfree_rcu(td);
> > > > > >                ^
> > > > > > net/netfilter/ipvs/ip_vs_est.c:552:3: error: ‘kfree_rcu’ undeclared (first use in
> > > > > > this function); did you mean ‘kfree_skb’?
> > > > > >    kfree_rcu(td);
> > > > > >    ^~~~~~~~~
> > > > > >    kfree_skb
> > > > > > net/netfilter/ipvs/ip_vs_est.c:552:3: note: each undeclared identifier is
> > > > > > reported only once for each function it appears in
> > > > > 
> > > > > Hi Frederic Weisbecker,
> > > > > 
> > > > > I encountered the same build error as yours. 
> > > > > Per the discussion link below, the fix for this build error by Uladzislau Rezki will be picked up by some other maintainer's branch?
> > > > > @Paul E . McKenney, please correct me if my understanding is wrong. 😊
> > > > > 
> > > > >     https://lore.kernel.org/rcu/Y9qc+lgR1CgdszKs@salvia/
> > > > 
> > > > Pablo and Julian, how are things coming with that patch?
> > > 
> > > 	Fix is already in net and net-next tree
> > 
> > Very good, thank you!  Is this going into this merge window or is it
> > expected to wait for v6.4?
> 
> this merge window. 							Thanx, Paul

Thank you, and looking forward to it!

								Thanx, Paul