Thread (77 messages) 77 messages, 10 authors, 2024-10-09

Re: [PATCH 05/14] tracefs: replace call_rcu by kfree_rcu for simple kmem_cache_free callback

From: Thorsten Leemhuis <linux@leemhuis.info>
Date: 2024-06-11 09:05:41
Also in: kernel-janitors, lkml, workflows

On 11.06.24 10:42, Vlastimil Babka wrote:
On 6/11/24 8:23 AM, Greg KH wrote:
quoted
On Mon, Jun 10, 2024 at 11:40:54PM +0200, Vlastimil Babka wrote:
quoted
On 6/10/24 10:36 PM, Steven Rostedt wrote:
quoted
On Mon, 10 Jun 2024 08:46:42 -0700
"Paul E. McKenney" [off-list ref] wrote:
quoted
quoted
quoted
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: [off-list ref] # 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: [off-list ref] # 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: [off-list ref] # after -rc3" could work,
but well, as indicated, that's kinda ugly.

Ciao, Thorsten
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help