| Message ID | 20240609082726.32742-6-Julia.Lawall@inria.fr (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | replace call_rcu by kfree_rcu for simple kmem_cache_free callback | expand |
On Sun, 9 Jun 2024 10:27:17 +0200 Julia Lawall <Julia.Lawall@inria.fr> wrote: > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c > index 7c29f4afc23d..338c52168e61 100644 > --- a/fs/tracefs/inode.c > +++ b/fs/tracefs/inode.c > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) > return &ti->vfs_inode; > } > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) > -{ > - struct tracefs_inode *ti; > - > - ti = container_of(rcu, struct tracefs_inode, rcu); > - kmem_cache_free(tracefs_inode_cachep, ti); Does this work? tracefs needs to be freed via the tracefs_inode_cachep. Does kfree_rcu() handle specific frees for objects that were not allocated via kmalloc()? -- Steve > -} > - > static void tracefs_free_inode(struct inode *inode) > { > struct tracefs_inode *ti = get_tracefs(inode); > @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode) > list_del_rcu(&ti->list); > spin_unlock_irqrestore(&tracefs_inode_lock, flags); > > - call_rcu(&ti->rcu, tracefs_free_inode_rcu); > + kfree_rcu(ti, rcu); > } > > static ssize_t default_read_file(struct file *file, char __user *buf,
On Mon, Jun 10, 2024 at 11:22:23AM -0400, Steven Rostedt wrote: > On Sun, 9 Jun 2024 10:27:17 +0200 > Julia Lawall <Julia.Lawall@inria.fr> wrote: > > > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c > > index 7c29f4afc23d..338c52168e61 100644 > > --- a/fs/tracefs/inode.c > > +++ b/fs/tracefs/inode.c > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) > > return &ti->vfs_inode; > > } > > > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) > > -{ > > - struct tracefs_inode *ti; > > - > > - ti = container_of(rcu, struct tracefs_inode, rcu); > > - kmem_cache_free(tracefs_inode_cachep, ti); > > Does this work? > > tracefs needs to be freed via the tracefs_inode_cachep. Does > kfree_rcu() handle specific frees for objects that were not allocated > via kmalloc()? A recent change to kfree() allows it to correctly handle memory allocated via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) Thanx, Paul > -- Steve > > > > -} > > - > > static void tracefs_free_inode(struct inode *inode) > > { > > struct tracefs_inode *ti = get_tracefs(inode); > > @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode) > > list_del_rcu(&ti->list); > > spin_unlock_irqrestore(&tracefs_inode_lock, flags); > > > > - call_rcu(&ti->rcu, tracefs_free_inode_rcu); > > + kfree_rcu(ti, rcu); > > } > > > > static ssize_t default_read_file(struct file *file, char __user *buf, >
On Mon, 10 Jun 2024 08:46:42 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > index 7c29f4afc23d..338c52168e61 100644 > > > --- a/fs/tracefs/inode.c > > > +++ b/fs/tracefs/inode.c > > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) > > > return &ti->vfs_inode; > > > } > > > > > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) > > > -{ > > > - struct tracefs_inode *ti; > > > - > > > - ti = container_of(rcu, struct tracefs_inode, rcu); > > > - kmem_cache_free(tracefs_inode_cachep, ti); > > > > Does this work? > > > > tracefs needs to be freed via the tracefs_inode_cachep. Does > > kfree_rcu() handle specific frees for objects that were not allocated > > via kmalloc()? > > A recent change to kfree() allows it to correctly handle memory allocated > via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) If that's the case then: Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> Do we have a way to add a "Depends-on" tag so that anyone backporting this will know that it requires the change to whatever allowed that to happen? Or we need to update the change log to explicitly state that this should *not* be backported. -- Steve
On 6/10/24 5:46 PM, Paul E. McKenney wrote: > On Mon, Jun 10, 2024 at 11:22:23AM -0400, Steven Rostedt wrote: >> On Sun, 9 Jun 2024 10:27:17 +0200 >> Julia Lawall <Julia.Lawall@inria.fr> wrote: >> >> > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c >> > index 7c29f4afc23d..338c52168e61 100644 >> > --- a/fs/tracefs/inode.c >> > +++ b/fs/tracefs/inode.c >> > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) >> > return &ti->vfs_inode; >> > } >> > >> > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) >> > -{ >> > - struct tracefs_inode *ti; >> > - >> > - ti = container_of(rcu, struct tracefs_inode, rcu); >> > - kmem_cache_free(tracefs_inode_cachep, ti); >> >> Does this work? >> >> tracefs needs to be freed via the tracefs_inode_cachep. Does >> kfree_rcu() handle specific frees for objects that were not allocated >> via kmalloc()? > > A recent change to kfree() allows it to correctly handle memory allocated > via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) Hey, I did try not to keep that a secret :) https://lore.kernel.org/all/20230310103210.22372-8-vbabka@suse.cz/ > Thanx, Paul > >> -- Steve >> >> >> > -} >> > - >> > static void tracefs_free_inode(struct inode *inode) >> > { >> > struct tracefs_inode *ti = get_tracefs(inode); >> > @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode) >> > list_del_rcu(&ti->list); >> > spin_unlock_irqrestore(&tracefs_inode_lock, flags); >> > >> > - call_rcu(&ti->rcu, tracefs_free_inode_rcu); >> > + kfree_rcu(ti, rcu); >> > } >> > >> > static ssize_t default_read_file(struct file *file, char __user *buf, >>
On Mon, 10 Jun 2024 22:42:30 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > On 6/10/24 5:46 PM, Paul E. McKenney wrote: > > On Mon, Jun 10, 2024 at 11:22:23AM -0400, Steven Rostedt wrote: > >> On Sun, 9 Jun 2024 10:27:17 +0200 > >> Julia Lawall <Julia.Lawall@inria.fr> wrote: > >> > >> > diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c > >> > index 7c29f4afc23d..338c52168e61 100644 > >> > --- a/fs/tracefs/inode.c > >> > +++ b/fs/tracefs/inode.c > >> > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) > >> > return &ti->vfs_inode; > >> > } > >> > > >> > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) > >> > -{ > >> > - struct tracefs_inode *ti; > >> > - > >> > - ti = container_of(rcu, struct tracefs_inode, rcu); > >> > - kmem_cache_free(tracefs_inode_cachep, ti); > >> > >> Does this work? > >> > >> tracefs needs to be freed via the tracefs_inode_cachep. Does > >> kfree_rcu() handle specific frees for objects that were not allocated > >> via kmalloc()? > > > > A recent change to kfree() allows it to correctly handle memory allocated > > via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) > > Hey, I did try not to keep that a secret :) > https://lore.kernel.org/all/20230310103210.22372-8-vbabka@suse.cz/ > Heh, I didn't look at that patch very deeply. -- Steve
On 6/10/24 10:36 PM, Steven Rostedt wrote: > On Mon, 10 Jun 2024 08:46:42 -0700 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > >> > > index 7c29f4afc23d..338c52168e61 100644 >> > > --- a/fs/tracefs/inode.c >> > > +++ b/fs/tracefs/inode.c >> > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) >> > > return &ti->vfs_inode; >> > > } >> > > >> > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) >> > > -{ >> > > - struct tracefs_inode *ti; >> > > - >> > > - ti = container_of(rcu, struct tracefs_inode, rcu); >> > > - kmem_cache_free(tracefs_inode_cachep, ti); >> > >> > Does this work? >> > >> > tracefs needs to be freed via the tracefs_inode_cachep. Does >> > kfree_rcu() handle specific frees for objects that were not allocated >> > via kmalloc()? >> >> A recent change to kfree() allows it to correctly handle memory allocated >> via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) > > If that's the case then: > > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > Do we have a way to add a "Depends-on" tag so that anyone backporting this > will know that it requires the change to whatever allowed that to happen? Looks like people use that tag, although no grep hits in Documentation, so Cc'ing workflows@ and Thorsten. In this case it would be Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB") > Or we need to update the change log to explicitly state that this should > *not* be backported. > > -- Steve
On Mon, Jun 10, 2024 at 11:40:54PM +0200, Vlastimil Babka wrote: > On 6/10/24 10:36 PM, Steven Rostedt wrote: > > On Mon, 10 Jun 2024 08:46:42 -0700 > > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > >> > > index 7c29f4afc23d..338c52168e61 100644 > >> > > --- a/fs/tracefs/inode.c > >> > > +++ b/fs/tracefs/inode.c > >> > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) > >> > > return &ti->vfs_inode; > >> > > } > >> > > > >> > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) > >> > > -{ > >> > > - struct tracefs_inode *ti; > >> > > - > >> > > - ti = container_of(rcu, struct tracefs_inode, rcu); > >> > > - kmem_cache_free(tracefs_inode_cachep, ti); > >> > > >> > Does this work? > >> > > >> > tracefs needs to be freed via the tracefs_inode_cachep. Does > >> > kfree_rcu() handle specific frees for objects that were not allocated > >> > via kmalloc()? > >> > >> A recent change to kfree() allows it to correctly handle memory allocated > >> via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) > > > > If that's the case then: > > > > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > > > Do we have a way to add a "Depends-on" tag so that anyone backporting this > > will know that it requires the change to whatever allowed that to happen? > > Looks like people use that tag, although no grep hits in Documentation, so > Cc'ing workflows@ and Thorsten. > > In this case it would be > > Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB") Ick, no, use the documented way of handling this as described in the stable kernel rules file. thanks, greg k-h
On 6/11/24 8:23 AM, Greg KH wrote: > On Mon, Jun 10, 2024 at 11:40:54PM +0200, Vlastimil Babka wrote: >> On 6/10/24 10:36 PM, Steven Rostedt wrote: >> > On Mon, 10 Jun 2024 08:46:42 -0700 >> > "Paul E. McKenney" <paulmck@kernel.org> wrote: >> > >> >> > > index 7c29f4afc23d..338c52168e61 100644 >> >> > > --- a/fs/tracefs/inode.c >> >> > > +++ b/fs/tracefs/inode.c >> >> > > @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) >> >> > > return &ti->vfs_inode; >> >> > > } >> >> > > >> >> > > -static void tracefs_free_inode_rcu(struct rcu_head *rcu) >> >> > > -{ >> >> > > - struct tracefs_inode *ti; >> >> > > - >> >> > > - ti = container_of(rcu, struct tracefs_inode, rcu); >> >> > > - kmem_cache_free(tracefs_inode_cachep, ti); >> >> > >> >> > Does this work? >> >> > >> >> > tracefs needs to be freed via the tracefs_inode_cachep. Does >> >> > kfree_rcu() handle specific frees for objects that were not allocated >> >> > via kmalloc()? >> >> >> >> A recent change to kfree() allows it to correctly handle memory allocated >> >> via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) >> > >> > If that's the case then: >> > >> > Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> >> > >> > Do we have a way to add a "Depends-on" tag so that anyone backporting this >> > will know that it requires the change to whatever allowed that to happen? >> >> Looks like people use that tag, although no grep hits in Documentation, so >> Cc'ing workflows@ and Thorsten. >> >> In this case it would be >> >> Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB") > > Ick, no, use the documented way of handling this as described in the > stable kernel rules file. AFAICS that documented way is for a different situation? I assume you mean this part: * Specify any additional patch prerequisites for cherry picking:: Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle But that would assume we actively want to backport this cleanup patch in the first place. But as I understand Steven's intention, we want just to make sure that if in the future this patch is backported (i.e. as a dependency of something else) it won't be forgotten to also backport c9929f0e344a ("mm/slob: remove CONFIG_SLOB"). How to express that without actively marking this patch for backport at the same time? > thanks, > > greg k-h
On 11.06.24 10:42, Vlastimil Babka wrote: > On 6/11/24 8:23 AM, Greg KH wrote: >> On Mon, Jun 10, 2024 at 11:40:54PM +0200, Vlastimil Babka wrote: >>> On 6/10/24 10:36 PM, Steven Rostedt wrote: >>>> On Mon, 10 Jun 2024 08:46:42 -0700 >>>> "Paul E. McKenney" <paulmck@kernel.org> wrote: >>>> >>>>>>> index 7c29f4afc23d..338c52168e61 100644 >>>>>>> --- a/fs/tracefs/inode.c >>>>>>> +++ b/fs/tracefs/inode.c >>>>>>> @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) >>>>>>> return &ti->vfs_inode; >>>>>>> } >>>>>>> >>>>>>> -static void tracefs_free_inode_rcu(struct rcu_head *rcu) >>>>>>> -{ >>>>>>> - struct tracefs_inode *ti; >>>>>>> - >>>>>>> - ti = container_of(rcu, struct tracefs_inode, rcu); >>>>>>> - kmem_cache_free(tracefs_inode_cachep, ti); >>>>>> >>>>>> Does this work? >>>>>> >>>>>> tracefs needs to be freed via the tracefs_inode_cachep. Does >>>>>> kfree_rcu() handle specific frees for objects that were not allocated >>>>>> via kmalloc()? >>>>> >>>>> A recent change to kfree() allows it to correctly handle memory allocated >>>>> via kmem_cache_alloc(). News to me as of a few weeks ago. ;-) >>>> >>>> If that's the case then: >>>> >>>> Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> >>>> >>>> Do we have a way to add a "Depends-on" tag so that anyone backporting this >>>> will know that it requires the change to whatever allowed that to happen? >>> >>> Looks like people use that tag, although no grep hits in Documentation, so >>> Cc'ing workflows@ and Thorsten. >>> >>> In this case it would be >>> >>> Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB") >> >> Ick, no, use the documented way of handling this as described in the >> stable kernel rules file. > > AFAICS that documented way is for a different situation? I assume you mean > this part: > > * Specify any additional patch prerequisites for cherry picking:: > > Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle > > But that would assume we actively want to backport this cleanup patch in the > first place. But as I understand Steven's intention, we want just to make > sure that if in the future this patch is backported (i.e. as a dependency of > something else) it won't be forgotten to also backport c9929f0e344a > ("mm/slob: remove CONFIG_SLOB"). How to express that without actively > marking this patch for backport at the same time? Hah, waiting a bit spared me the time to write a similar reply. :-D Writing one now anyway to broaden the scope: I recently noticed we have the same problem when it comes to the "delayed backporting" aspect, e.g. this part: """ * Delay pick up of patches:: Cc: <stable@vger.kernel.org> # after -rc3 """ I'll bring this up in a maintainers summit proposal I'm currently preparing. But I have no idea how to solve this in an elegant way. "Cc: <stable+INeitherKnowNorCare@kernel.org> # after -rc3" could work, but well, as indicated, that's kinda ugly. Ciao, Thorsten
On Tue, 11 Jun 2024 08:23:11 +0200 Greg KH <gregkh@linuxfoundation.org> wrote: > > Depends-on: c9929f0e344a ("mm/slob: remove CONFIG_SLOB") > > Ick, no, use the documented way of handling this as described in the > stable kernel rules file. You mentioned this before, I guess you mean this: > To send additional instructions to the stable team, use a shell-style inline > comment to pass arbitrary or predefined notes: > > * Specify any additional patch prerequisites for cherry picking:: > > Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle > Cc: <stable@vger.kernel.org> # 3.3.x: 1b9508f: sched: Rate-limit newidle > Cc: <stable@vger.kernel.org> # 3.3.x: fd21073: sched: Fix affinity logic > Cc: <stable@vger.kernel.org> # 3.3.x > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > The tag sequence has the meaning of:: > > git cherry-pick a1f84a3 > git cherry-pick 1b9508f > git cherry-pick fd21073 > git cherry-pick <this commit> > > Note that for a patch series, you do not have to list as prerequisites the > patches present in the series itself. For example, if you have the following > patch series:: > > patch1 > patch2 > > where patch2 depends on patch1, you do not have to list patch1 as > prerequisite of patch2 if you have already marked patch1 for stable > inclusion. What's with the "3.3.x"? Isn't that obsolete? And honestly, I find the above much more "ick" than "Depends-on:". That's because I like to read human readable tags and not machine processing tags. I'm a human, not a machine. -- Steve
On Tue, 11 Jun 2024 10:42:28 +0200 Vlastimil Babka <vbabka@suse.cz> wrote: > AFAICS that documented way is for a different situation? I assume you mean > this part: > > * Specify any additional patch prerequisites for cherry picking:: > > Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle > > But that would assume we actively want to backport this cleanup patch in the > first place. But as I understand Steven's intention, we want just to make > sure that if in the future this patch is backported (i.e. as a dependency of > something else) it won't be forgotten to also backport c9929f0e344a > ("mm/slob: remove CONFIG_SLOB"). How to express that without actively > marking this patch for backport at the same time? Exactly! This isn't to be tagged as stable. It's just a way to say "if you need this patch for any reason, you also need patch X". I think "Depends-on" is the way to go, as it is *not* a stable thing, and what is in stable rules is only about stable patches. -- Steve
On Tue, Jun 11, 2024 at 10:14:58AM -0400, Steven Rostedt wrote: > On Tue, 11 Jun 2024 10:42:28 +0200 > Vlastimil Babka <vbabka@suse.cz> wrote: > > > AFAICS that documented way is for a different situation? I assume you mean > > this part: > > > > * Specify any additional patch prerequisites for cherry picking:: > > > > Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle > > > > But that would assume we actively want to backport this cleanup patch in the > > first place. But as I understand Steven's intention, we want just to make > > sure that if in the future this patch is backported (i.e. as a dependency of > > something else) it won't be forgotten to also backport c9929f0e344a > > ("mm/slob: remove CONFIG_SLOB"). How to express that without actively > > marking this patch for backport at the same time? > > Exactly! This isn't to be tagged as stable. It's just a way to say "if you > need this patch for any reason, you also need patch X". > > I think "Depends-on" is the way to go, as it is *not* a stable thing, and > what is in stable rules is only about stable patches. How does "Depends-on" not spiral out of control? There's a *lot* of "Depends-on" relations one could express in commit series and such. Of course a lot of git itself is designed to show some subset of these relationships. It seems like in most cases, the "Cc: stable@v.g.o # x.y.z+" notation expresses the backporting safety correctly. What is the purpose of saying, "if you need this patch for any reason, you also need patch X"? Who is the intended audience, and are you sure they need this? I ask these questions because I wind up doing a lot of work backporting patches to stable and marking things properly for that or submitting manually backported stable patches and so forth, and in general, patch applicability for stable things is something I wind up devoting a lot of time to. If I have to *additionally* start caring about the theoretical possibility that somebody in the future, outside of the stable flow, might not understand the context of a given patch and blindly apply it to some random tree here or there, that sounds like a lot of extra brain cycles to consider. So, is this actually necessary, and how does it not spiral out of control?
On Wed, 12 Jun 2024 16:09:40 +0200 "Jason A. Donenfeld" <Jason@zx2c4.com> wrote: > > > > I think "Depends-on" is the way to go, as it is *not* a stable thing, and > > what is in stable rules is only about stable patches. > > How does "Depends-on" not spiral out of control? There's a *lot* of > "Depends-on" relations one could express in commit series and such. Of > course a lot of git itself is designed to show some subset of these > relationships. If a change occurs because a recent change happened that allows the current change to work, then I think a Depends-on is appropriate. Like in this example. I thought this change was broken, and it would have been except for a recent change. Having the dependency listed is useful, especially if the dependency is subtle (doesn't break the build and may not show the bug immediately). > > It seems like in most cases, the "Cc: stable@v.g.o # x.y.z+" notation > expresses the backporting safety correctly. What is the purpose of > saying, "if you need this patch for any reason, you also need patch X"? > Who is the intended audience, and are you sure they need this? The intended audience is someone backporting features and not fixes. > > I ask these questions because I wind up doing a lot of work backporting > patches to stable and marking things properly for that or submitting > manually backported stable patches and so forth, and in general, patch > applicability for stable things is something I wind up devoting a lot of > time to. If I have to *additionally* start caring about the theoretical > possibility that somebody in the future, outside of the stable flow, > might not understand the context of a given patch and blindly apply it > to some random tree here or there, that sounds like a lot of extra brain > cycles to consider. > > So, is this actually necessary, and how does it not spiral out of > control? How would you see it going out of control? And "Depends-on" would only be used for non stable relationships. If stable backports, we can keep with the current method. -- Steve
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c index 7c29f4afc23d..338c52168e61 100644 --- a/fs/tracefs/inode.c +++ b/fs/tracefs/inode.c @@ -53,14 +53,6 @@ static struct inode *tracefs_alloc_inode(struct super_block *sb) return &ti->vfs_inode; } -static void tracefs_free_inode_rcu(struct rcu_head *rcu) -{ - struct tracefs_inode *ti; - - ti = container_of(rcu, struct tracefs_inode, rcu); - kmem_cache_free(tracefs_inode_cachep, ti); -} - static void tracefs_free_inode(struct inode *inode) { struct tracefs_inode *ti = get_tracefs(inode); @@ -70,7 +62,7 @@ static void tracefs_free_inode(struct inode *inode) list_del_rcu(&ti->list); spin_unlock_irqrestore(&tracefs_inode_lock, flags); - call_rcu(&ti->rcu, tracefs_free_inode_rcu); + kfree_rcu(ti, rcu); } static ssize_t default_read_file(struct file *file, char __user *buf,
Since SLOB was removed, it is not necessary to use call_rcu when the callback only performs kmem_cache_free. Use kfree_rcu() directly. The changes were done using the following Coccinelle semantic patch. This semantic patch is designed to ignore cases where the callback function is used in another way. // <smpl> @r@ expression e; local idexpression e2; identifier cb,f; position p; @@ ( call_rcu(...,e2) | call_rcu(&e->f,cb@p) ) @r1@ type T; identifier x,r.cb; @@ cb(...) { ( kmem_cache_free(...); | T x = ...; kmem_cache_free(...,x); | T x; x = ...; kmem_cache_free(...,x); ) } @s depends on r1@ position p != r.p; identifier r.cb; @@ cb@p @script:ocaml@ cb << r.cb; p << s.p; @@ Printf.eprintf "Other use of %s at %s:%d\n" cb (List.hd p).file (List.hd p).line @depends on r1 && !s@ expression e; identifier r.cb,f; position r.p; @@ - call_rcu(&e->f,cb@p) + kfree_rcu(e,f) @r1a depends on !s@ type T; identifier x,r.cb; @@ - cb(...) { ( - kmem_cache_free(...); | - T x = ...; - kmem_cache_free(...,x); | - T x; - x = ...; - kmem_cache_free(...,x); ) - } // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> --- fs/tracefs/inode.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)