Re: [PATCH][RFC] get rid of busy-wait in shrink_dcache_tree()

Linus Torvalds
2026-01-24T04:47:11Z

On Fri, 23 Jan 2026 at 20:34, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> >
> > In practice it doesn't really matter, but we don't want to initialize
> > that field to NULL - no good place for doing that. Sure, the entire
> > d_alias has been subject to hlist_del_init() or INIT_HLIST_NODE(), so
> > any pointer field unioned with it will end up being NULL without
> > any assignments to it, but... ugh. "We have a union of two-pointer
> > struct, a pointer and some other stuff; we'd set both members of that
> > struct member to NULL and count upon the pointer member of union
> > having been zeroed by that" leaves a bad taste.
>
> FWIW, there's another reason, but it's not one I'm fond of. There are few
> places where we use hlist_unhashed(&dentry->d_u.d_alias) as a debugging
> check. I _think_ that none of those are reachable for dentries in that state,
> but then all of them are of "it should evaluate true unless we have a kernel
> bug" kind.
>
Note that I'm not complaining about re-using the
'dentry->d_u.d_alias.next' field.

I'm complaining about re-using it WITH A CAST, AND WITHOUT DOCUMENTING
IT IN THE TYPE DECLARATION.

So by all means use that field, and use that value, but use it *BY*
making it a proper union member, and by documenting the lifetime
change of that union member.

None of this "randomly cast a different pointer to 'void *' where it
will then be implicitly cast to to 'struct select_data *'" garbage.

Just make a proper "struct select_data *" union member that aliases
that "d_alias.next" field, and the compiler will generate the EXACT
same code, except the source code will be cleaner, and you won't need
any hacky pointer casts.

And document how that field is NULL when the dentry is killed, and how
that NULL 'dentry->d_u.d_alias.next' field at that point becomes a
NULL 'dentry->d_u.d_select_data' field.

You don't need to describe 'struct select_data', you just need to
declare it. IOW, something like this:

--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -89,6 +89,11 @@ union shortname_store {
#define d_lock d_lockref.lock
#define d_iname d_shortname.string

+// The d_alias list becomes a 'struct select_data' pointer when
+// the dentry is killed (so the NULL d_alias.next pointer becomes
+// a NULL 'struct select_data *'
+struct select_data;
+
struct dentry {
/* RCU lookup touched fields */
unsigned int d_flags; /* protected by d_lock */
@@ -128,6 +133,7 @@ struct dentry {
struct hlist_node d_alias; /* inode alias list */
struct hlist_bl_node d_in_lookup_hash; /* only for
in-lookup ones */
struct rcu_head d_rcu;
+ struct select_data *d_select_data; /* only for killed
dentries */
} d_u;
};

but I obviously didn't test it.

Linus