assorted dcache stuff

PATCH In review Al Viro 7 participants 32 messages Nov 24, 2023 – Dec 9, 2025

Subsystems (4)

Documentation VFS SELinux security module Overlay filesystem

lore

Share & tools

lore

Al Viro

author maintainer

2023-11-24 06:05 UTC #0 lore raw

PATCH 1/1 assorted dcache stuff

Assorted dcache cleanups. There's a couple of commits that live in
never-rebased branches shared with selinux and overlayfs trees resp.

Lives in #work.dcache-misc, #no-rebase-overlayfs, #merged-selinux
and #work.dcache (all pulled by the last one, as is #work.dcache2) in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git
Individual patches in followups. Review and testing would be very welcome; it survives the local beating, but the more the better...

Diffstat (sans #work.dcache2):

 fs/dcache.c                  | 157 +++++++++++++++----------------------------
 fs/file_table.c              |   5 --
 fs/internal.h                |   5 ++
 fs/libfs.c                   |  17 ++---
 fs/nsfs.c                    |   7 +-
 fs/overlayfs/export.c        |  23 +------
 include/linux/dcache.h       | 142 +++++++++++++++++---------------------
 security/selinux/selinuxfs.c | 144 ++++++++++++++++++---------------------
 8 files changed, 194 insertions(+), 306 deletions(-)

Shortlog (again, excluding #work.dcache2):

Al Viro (21):
selinux: saner handling of policy reloads
struct dentry: get rid of randomize_layout idiocy
get rid of __dget()
DCACHE_... ->d_flags bits: switch to BIT()
DCACHE_COOKIE: RIP
kill d_{is,set}_fallthru()
dentry.h: trim externs
[software coproarchaeology] dentry.h: kill a mysterious comment kill d_backing_dentry()
Merge branch 'no-rebase-overlayfs' into work.dcache-misc
kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller d_alloc_pseudo(): move setting ->d_op there from the (sole) caller
nsfs: use d_make_root()
Merge branch 'merged-selinux' into work.dcache-misc simple_fill_super(): don't bother with d_genocide() on failure d_genocide(): move the extern into fs/internal.h
get rid of DCACHE_GENOCIDE
d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant __d_unalias() doesn't use inode argument
Merge branch 'work.dcache2' into work.dcache
kill DCACHE_MAY_FREE

Amir Goldstein (1):
ovl: stop using d_alloc_anon()/d_instantiate_anon()

Vegard Nossum (1):
dcache: remove unnecessary NULL check in dget_dlock()

[merged-selinux]
1/#) selinux: saner handling of policy reloads
Fix abuses of lock_rename() and d_genocide() in there.

[no-rebase-overlayfs]
2/#) ovl: stop using d_alloc_anon()/d_instantiate_anon()

	Switch overlayfs to d_obtain_alias(); overlayfs used keep a data

structure (struct ovl_entry) hanging off a dentry, which prevented the use of d_obtain_alias() there.
We couldn't call d_obtain_alias() and attach struct ovl_entry to
resulting dentry - new dentry would be exposed as an inode alias while still not in a usable state.
These days ovl_entry got moved to their inodes and the rest
of the things done in ovl_obtain_alias() turned out to be not needed. The bottom line: ovl_obtain_alias() can use straight d_obtain_alias() and doesn't need an access to the guts of that primitive.

[work.dcache-misc]
[the first two used to be in #work.dcache2]
3/#) struct dentry: get rid of randomize_layout idiocy
This is beyond ridiculous. There is a reason why that thing
is cacheline-aligned...

4/#) get rid of __dget()
fold into the sole remaining caller

5/#) DCACHE_... ->d_flags bits: switch to BIT()

	For bits 20..22 (inode type cached in ->d_flags) turn the

definitions into expressions like (5 << 20); everything else turns into straight use of BIT()

6/#) DCACHE_COOKIE: RIP
the last user gone in 2021...

7/#) kill d_{is,set}_fallthru()
Introduced in 2015 and never had any in-tree users...

8/#) dentry.h: trim externs
d_instantiate_unique() had been gone for 7 years; __d_lookup...()
and shrink_dcache_for_umount() are fs/internal.h fodder.

9/#) [software coproarchaeology] dentry.h: kill a mysterious comment
there's a strange comment in front of d_lookup() declaration:
/* appendix may either be NULL or be used for transname suffixes */

	Looks like nobody had been curious enough to track its history;

it predates git, it predates bitkeeper and if you look through the pre-BK trees, you finally arrive at this in 2.1.44-for-davem:
/* appendix may either be NULL or be used for transname suffixes */
-extern struct dentry * d_lookup(struct inode * dir, struct qstr * name,
- struct qstr * appendix);
+extern struct dentry * d_lookup(struct dentry * dir, struct qstr * name);
In other words, it refers to the third argument d_lookup() used to have back then. It had been introduced in 2.1.43-pre, on June 12 1997, along with d_lookup(), only to be removed by July 4 1997, presumably when the Cthulhu-awful thing it used to be used for (look for CONFIG_TRANS_NAMES in 2.1.43-pre, and keep a heavy-duty barfbag ready) had been, er, noticed and recognized for what it had been. Despite the appendectomy, the comment remained. Some things really need to be put out of their misery...

10/#) kill d_backing_dentry()
no users left

11/#) kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller
now that the only user of d_instantiate_anon() is gone...

12/#) d_alloc_pseudo(): move setting ->d_op there from the (sole) caller
more obviously safe there...

13/#) nsfs: use d_make_root()

	Normally d_make_root() is used to create the root dentry of superblock;

here we use it for a different purpose, but... idiomatic or not, we need the same operation.

14/#) simple_fill_super(): don't bother with d_genocide() on failure

	Failing ->fill_super() will be followed by ->kill_sb(), which should

include kill_litter_super() if the call of simple_fill_super() had been asked to create anything besides the root dentry. So there's no need to empty the partially populated tree - it will be trimmed by inevitable kill_litter_super().

15/#) d_genocide(): move the extern into fs/internal.h
... now that the only user is in fs/super.c

16/#) get rid of DCACHE_GENOCIDE
... now that we never call d_genocide() other than from kill_litter_super()

17/#) d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant

	We only search in the damn thing under hlist_bl_lock(); RCU variant

of insertion was, IIRC, pretty much cargo-culted - mea culpa...

18/#) __d_unalias() doesn't use inode argument

	... and hasn't since 2015.

At that point #work.dcache-misc merges with #work.dcache2 into #work.dcache, with two followups:

19/#) kill DCACHE_MAY_FREE
Redundant with new ordering in __dentry_kill()

20/#) dcache: remove unnecessary NULL check in dget_dlock()

	dget_dlock() requires dentry->d_lock to be held when called,

yet contains a NULL check for dentry. After taking out the NULL check, dget_dlock() becomes almost identical to __dget_dlock(); the only difference is that dget_dlock() returns the dentry that was passed in. These are static inline helpers, so we can rely on the compiler to discard unused return values. We can therefore also remove __dget_dlock() and replace calls to it by dget_dlock().

Al Viro

author maintainer

2023-11-24 06:06 UTC #1 lore raw

PATCH 1/20 selinux: saner handling of policy reloads

On policy reload selinuxfs replaces two subdirectories (/booleans and /class) with new variants. Unfortunately, that's done with serious abuses of directory locking.

1) lock_rename() should be done to parents, not to objects being exchanged

2) there's a bunch of reasons why it should not be done for directories that do not have a common ancestor; most of those do not apply to selinuxfs, but even in the best case the proof is subtle and brittle.

3) failure halfway through the creation of /class will leak names and values arrays.

4) use of d_genocide() is also rather brittle; it's probably not much of a bug per se, but e.g. an overmount of /sys/fs/selinuxfs/classes/shm/index with any regular file will end up with leaked mount on policy reload. Sure, don't do it, but...

Let's stop messing with disconnected directories; just create a temporary (/.swapover) with no permissions for anyone (on the level of ->permission() returing -EPERM, no matter who's calling it) and build the new /booleans and /class in there; then lock_rename on root and that temporary directory and d_exchange() old and new both for class and booleans. Then unlock and use simple_recursive_removal() to take the temporary out; it's much more robust.

And instead of bothering with separate pathways for freeing new (on failure halfway through) and old (on success) names/values, do all freeing in one place. With temporaries swapped with the old ones when we are past all possible failures.

The only user-visible difference is that /.swapover shows up (but isn't possible to open, look up into, etc.) for the duration of policy reload.

Reviewed-by Stephen Smalley <[email protected]>

Signed-off-by Al Viro <[email protected]>

[PM: applied some fixes from Al post merge]

Signed-off-by Paul Moore <[email protected]>

---
 security/selinux/selinuxfs.c | 144 ++++++++++++++++-------------------
 1 file changed, 66 insertions(+), 78 deletions(-)

security/selinux/selinuxfs.c +66 ~~-78~~

index 6c596ae7fef9..0619a1cbbfbe 100644
@@ -336,12 +336,9 @@ static struct dentry *sel_make_dir(struct dentry *dir, const char *name,
 			unsigned long *ino);
 
 /* declaration for sel_make_policy_nodes */
-static struct dentry *sel_make_disconnected_dir(struct super_block *sb,
+static struct dentry *sel_make_swapover_dir(struct super_block *sb,
 						unsigned long *ino);
 
-/* declaration for sel_make_policy_nodes */
-static void sel_remove_entries(struct dentry *de);
-
 static ssize_t sel_read_mls(struct file *filp, char __user *buf,
 				size_t count, loff_t *ppos)
 {

@@ -508,13 +505,13 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
 				struct selinux_policy *newpolicy)
 {
 	int ret = 0;
-	struct dentry *tmp_parent, *tmp_bool_dir, *tmp_class_dir, *old_dentry;
-	unsigned int tmp_bool_num, old_bool_num;
-	char **tmp_bool_names, **old_bool_names;
-	int *tmp_bool_values, *old_bool_values;
+	struct dentry *tmp_parent, *tmp_bool_dir, *tmp_class_dir;
+	unsigned int bool_num = 0;
+	char **bool_names = NULL;
+	int *bool_values = NULL;
 	unsigned long tmp_ino = fsi->last_ino; /* Don't increment last_ino in this function */
 
-	tmp_parent = sel_make_disconnected_dir(fsi->sb, &tmp_ino);
+	tmp_parent = sel_make_swapover_dir(fsi->sb, &tmp_ino);
 	if (IS_ERR(tmp_parent))
 		return PTR_ERR(tmp_parent);

@@ -532,8 +529,8 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
 		goto out;
 	}
 
-	ret = sel_make_bools(newpolicy, tmp_bool_dir, &tmp_bool_num,
-			     &tmp_bool_names, &tmp_bool_values);
+	ret = sel_make_bools(newpolicy, tmp_bool_dir, &bool_num,
+			     &bool_names, &bool_values);
 	if (ret)
 		goto out;

@@ -542,38 +539,30 @@ static int sel_make_policy_nodes(struct selinux_fs_info *fsi,
 	if (ret)
 		goto out;
 
+	lock_rename(tmp_parent, fsi->sb->s_root);
+
 	/* booleans */
-	old_dentry = fsi->bool_dir;
-	lock_rename(tmp_bool_dir, old_dentry);
 	d_exchange(tmp_bool_dir, fsi->bool_dir);
 
-	old_bool_num = fsi->bool_num;
-	old_bool_names = fsi->bool_pending_names;
-	old_bool_values = fsi->bool_pending_values;
-
-	fsi->bool_num = tmp_bool_num;
-	fsi->bool_pending_names = tmp_bool_names;
-	fsi->bool_pending_values = tmp_bool_values;
-
-	sel_remove_old_bool_data(old_bool_num, old_bool_names, old_bool_values);
+	swap(fsi->bool_num, bool_num);
+	swap(fsi->bool_pending_names, bool_names);
+	swap(fsi->bool_pending_values, bool_values);
 
 	fsi->bool_dir = tmp_bool_dir;
-	unlock_rename(tmp_bool_dir, old_dentry);
 
 	/* classes */
-	old_dentry = fsi->class_dir;
-	lock_rename(tmp_class_dir, old_dentry);
 	d_exchange(tmp_class_dir, fsi->class_dir);
 	fsi->class_dir = tmp_class_dir;
-	unlock_rename(tmp_class_dir, old_dentry);
+
+	unlock_rename(tmp_parent, fsi->sb->s_root);
 
 out:
+	sel_remove_old_bool_data(bool_num, bool_names, bool_values);
 	/* Since the other temporary dirs are children of tmp_parent
 	 * this will handle all the cleanup in the case of a failure before
 	 * the swapover
 	 */
-	sel_remove_entries(tmp_parent);
-	dput(tmp_parent); /* d_genocide() only handles the children */
+	simple_recursive_removal(tmp_parent, NULL);
 
 	return ret;
 }

@@ -1351,54 +1340,48 @@ static const struct file_operations sel_commit_bools_ops = {
 	.llseek		= generic_file_llseek,
 };
 
-static void sel_remove_entries(struct dentry *de)
-{
-	d_genocide(de);
-	shrink_dcache_parent(de);
-}
-
 static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_dir,
 			  unsigned int *bool_num, char ***bool_pending_names,
 			  int **bool_pending_values)
 {
 	int ret;
-	ssize_t len;
-	struct dentry *dentry = NULL;
-	struct inode *inode = NULL;
-	struct inode_security_struct *isec;
-	char **names = NULL, *page;
+	char **names, *page;
 	u32 i, num;
-	int *values = NULL;
-	u32 sid;
 
-	ret = -ENOMEM;
 	page = (char *)get_zeroed_page(GFP_KERNEL);
 	if (!page)
-		goto out;
+		return -ENOMEM;
 
-	ret = security_get_bools(newpolicy, &num, &names, &values);
+	ret = security_get_bools(newpolicy, &num, &names, bool_pending_values);
 	if (ret)
 		goto out;
 
+	*bool_num = num;
+	*bool_pending_names = names;
+
 	for (i = 0; i < num; i++) {
-		ret = -ENOMEM;
+		struct dentry *dentry;
+		struct inode *inode;
+		struct inode_security_struct *isec;
+		ssize_t len;
+		u32 sid;
+
+		len = snprintf(page, PAGE_SIZE, "/%s/%s", BOOL_DIR_NAME, names[i]);
+		if (len >= PAGE_SIZE) {
+			ret = -ENAMETOOLONG;
+			break;
+		}
 		dentry = d_alloc_name(bool_dir, names[i]);
-		if (!dentry)
-			goto out;
+		if (!dentry) {
+			ret = -ENOMEM;
+			break;
+		}
 
-		ret = -ENOMEM;
 		inode = sel_make_inode(bool_dir->d_sb, S_IFREG | S_IRUGO | S_IWUSR);
 		if (!inode) {
 			dput(dentry);
-			goto out;
-		}
-
-		ret = -ENAMETOOLONG;
-		len = snprintf(page, PAGE_SIZE, "/%s/%s", BOOL_DIR_NAME, names[i]);
-		if (len >= PAGE_SIZE) {
-			dput(dentry);
-			iput(inode);
-			goto out;
+			ret = -ENOMEM;
+			break;
 		}
 
 		isec = selinux_inode(inode);

@@ -1416,23 +1399,8 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_
 		inode->i_ino = i|SEL_BOOL_INO_OFFSET;
 		d_add(dentry, inode);
 	}
-	*bool_num = num;
-	*bool_pending_names = names;
-	*bool_pending_values = values;
-
-	free_page((unsigned long)page);
-	return 0;
 out:
 	free_page((unsigned long)page);
-
-	if (names) {
-		for (i = 0; i < num; i++)
-			kfree(names[i]);
-		kfree(names);
-	}
-	kfree(values);
-	sel_remove_entries(bool_dir);
-
 	return ret;
 }

@@ -1961,20 +1929,40 @@ static struct dentry *sel_make_dir(struct dentry *dir, const char *name,
 	return dentry;
 }
 
-static struct dentry *sel_make_disconnected_dir(struct super_block *sb,
+static int reject_all(struct mnt_idmap *idmap, struct inode *inode, int mask)
+{
+	return -EPERM;	// no access for anyone, root or no root.
+}
+
+static const struct inode_operations swapover_dir_inode_operations = {
+	.lookup		= simple_lookup,
+	.permission	= reject_all,
+};
+
+static struct dentry *sel_make_swapover_dir(struct super_block *sb,
 						unsigned long *ino)
 {
-	struct inode *inode = sel_make_inode(sb, S_IFDIR | S_IRUGO | S_IXUGO);
+	struct dentry *dentry = d_alloc_name(sb->s_root, ".swapover");
+	struct inode *inode;
 
-	if (!inode)
+	if (!dentry)
 		return ERR_PTR(-ENOMEM);
 
-	inode->i_op = &simple_dir_inode_operations;
-	inode->i_fop = &simple_dir_operations;
+	inode = sel_make_inode(sb, S_IFDIR);
+	if (!inode) {
+		dput(dentry);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	inode->i_op = &swapover_dir_inode_operations;
 	inode->i_ino = ++(*ino);
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
 	inc_nlink(inode);
-	return d_obtain_alias(inode);
+	inode_lock(sb->s_root->d_inode);
+	d_add(dentry, inode);
+	inc_nlink(sb->s_root->d_inode);
+	inode_unlock(sb->s_root->d_inode);
+	return dentry;
 }
 
 #define NULL_FILE_NAME "null"

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #2 lore raw

PATCH 2/20 ovl: stop using d_alloc_anon()/d_instantiate_anon()

From: Amir Goldstein <[email protected]>

Commit f9c34674bc60 ("vfs: factor out helpers d_instantiate_anon() and d_alloc_anon()") was introduced so overlayfs could initialize a non-dir disconnected overlay dentry before overlay inode is attached to it.

Since commit ("0af950f57fef ovl: move ovl_entry into ovl_inode"), all ovl_obtain_alias() can do is set DCACHE_OP_*REVALIDATE flags in ->d_flags and OVL_E_UPPER_ALIAS flag in ->d_fsdata.

The DCACHE_OP_*REVALIDATE flags and OVL_E_UPPER_ALIAS flag are irrelevant for a disconnected non-dir dentry, so it is better to use d_obtain_alias() instead of open coding it.

Suggested-by Al Viro <[email protected]>

Signed-off-by Amir Goldstein <[email protected]>

Signed-off-by Al Viro <[email protected]>

---
 fs/overlayfs/export.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

fs/overlayfs/export.c +1 ~~-22~~

index 7e16bbcad95e..9e316d5f936e 100644
@@ -289,7 +289,6 @@ static struct dentry *ovl_obtain_alias(struct super_block *sb,
 {
 	struct dentry *lower = lowerpath ? lowerpath->dentry : NULL;
 	struct dentry *upper = upper_alias ?: index;
-	struct dentry *dentry;
 	struct inode *inode = NULL;
 	struct ovl_entry *oe;
 	struct ovl_inode_params oip = {

@@ -320,27 +319,7 @@ static struct dentry *ovl_obtain_alias(struct super_block *sb,
 	if (upper)
 		ovl_set_flag(OVL_UPPERDATA, inode);
 
-	dentry = d_find_any_alias(inode);
-	if (dentry)
-		goto out_iput;
-
-	dentry = d_alloc_anon(inode->i_sb);
-	if (unlikely(!dentry))
-		goto nomem;
-
-	if (upper_alias)
-		ovl_dentry_set_upper_alias(dentry);
-
-	ovl_dentry_init_reval(dentry, upper, OVL_I_E(inode));
-
-	return d_instantiate_anon(dentry, inode);
-
-nomem:
-	dput(dentry);
-	dentry = ERR_PTR(-ENOMEM);
-out_iput:
-	iput(inode);
-	return dentry;
+	return d_obtain_alias(inode);
 }
 
 /* Get the upper or lower dentry in stack whose on layer @idx */

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #3 lore raw

PATCH 3/20 struct dentry: get rid of randomize_layout idiocy

In reply to Al Viro

This is beyond ridiculous. There is a reason why that thing is cacheline-aligned...

Reviewed-by Christian Brauner <[email protected]>

Signed-off-by Al Viro <[email protected]>

---
 include/linux/dcache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/dcache.h +1 -1

index 3da2f0545d5d..1d9f7f132055 100644
@@ -111,7 +111,7 @@ struct dentry {
 		struct hlist_bl_node d_in_lookup_hash;	/* only for in-lookup ones */
 	 	struct rcu_head d_rcu;
 	} d_u;
-} __randomize_layout;
+};
 
 /*
  * dentry->d_lock spinlock nesting subclasses:

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #4 lore raw

PATCH 4/20 get rid of __dget()

In reply to Al Viro

fold into the sole remaining caller

Reviewed-by Christian Brauner <[email protected]>

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

fs/dcache.c +1 -6

index c82ae731df9a..b8f1b54a1492 100644
@@ -948,11 +948,6 @@ static inline void __dget_dlock(struct dentry *dentry)
 	dentry->d_lockref.count++;
 }
 
-static inline void __dget(struct dentry *dentry)
-{
-	lockref_get(&dentry->d_lockref);
-}
-
 struct dentry *dget_parent(struct dentry *dentry)
 {
 	int gotref;

@@ -1002,7 +997,7 @@ static struct dentry * __d_find_any_alias(struct inode *inode)
 	if (hlist_empty(&inode->i_dentry))
 		return NULL;
 	alias = hlist_entry(inode->i_dentry.first, struct dentry, d_u.d_alias);
-	__dget(alias);
+	lockref_get(&alias->d_lockref);
 	return alias;
 }

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #5 lore raw

PATCH 5/20 DCACHE_... ->d_flags bits: switch to BIT()

In reply to Al Viro

For bits 20..22 (inode type cached in ->d_flags) turn the definitions into expressions like (5 << 20); everything else turns into straight use of BIT()

Signed-off-by Al Viro <[email protected]>

---
 include/linux/dcache.h | 76 +++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

include/linux/dcache.h +38 ~~-38~~

index 1d9f7f132055..d9c314cc93b8 100644
@@ -151,13 +151,13 @@ struct dentry_operations {
  */
 
 /* d_flags entries */
-#define DCACHE_OP_HASH			0x00000001
-#define DCACHE_OP_COMPARE		0x00000002
-#define DCACHE_OP_REVALIDATE		0x00000004
-#define DCACHE_OP_DELETE		0x00000008
-#define DCACHE_OP_PRUNE			0x00000010
+#define DCACHE_OP_HASH			BIT(0)
+#define DCACHE_OP_COMPARE		BIT(1)
+#define DCACHE_OP_REVALIDATE		BIT(2)
+#define DCACHE_OP_DELETE		BIT(3)
+#define DCACHE_OP_PRUNE			BIT(4)
 
-#define	DCACHE_DISCONNECTED		0x00000020
+#define	DCACHE_DISCONNECTED		BIT(5)
      /* This dentry is possibly not currently connected to the dcache tree, in
       * which case its parent will either be itself, or will have this flag as
       * well.  nfsd will not use a dentry with this bit set, but will first

@@ -168,50 +168,50 @@ struct dentry_operations {
       * dentry into place and return that dentry rather than the passed one,
       * typically using d_splice_alias. */
 
-#define DCACHE_REFERENCED		0x00000040 /* Recently used, don't discard. */
+#define DCACHE_REFERENCED		BIT(6) /* Recently used, don't discard. */
 
-#define DCACHE_DONTCACHE		0x00000080 /* Purge from memory on final dput() */
+#define DCACHE_DONTCACHE		BIT(7) /* Purge from memory on final dput() */
 
-#define DCACHE_CANT_MOUNT		0x00000100
-#define DCACHE_GENOCIDE			0x00000200
-#define DCACHE_SHRINK_LIST		0x00000400
+#define DCACHE_CANT_MOUNT		BIT(8)
+#define DCACHE_GENOCIDE			BIT(9)
+#define DCACHE_SHRINK_LIST		BIT(10)
 
-#define DCACHE_OP_WEAK_REVALIDATE	0x00000800
+#define DCACHE_OP_WEAK_REVALIDATE	BIT(11)
 
-#define DCACHE_NFSFS_RENAMED		0x00001000
+#define DCACHE_NFSFS_RENAMED		BIT(12)
      /* this dentry has been "silly renamed" and has to be deleted on the last
       * dput() */
-#define DCACHE_COOKIE			0x00002000 /* For use by dcookie subsystem */
-#define DCACHE_FSNOTIFY_PARENT_WATCHED	0x00004000
+#define DCACHE_COOKIE			BIT(13) /* For use by dcookie subsystem */
+#define DCACHE_FSNOTIFY_PARENT_WATCHED	BIT(14)
      /* Parent inode is watched by some fsnotify listener */
 
-#define DCACHE_DENTRY_KILLED		0x00008000
+#define DCACHE_DENTRY_KILLED		BIT(15)
 
-#define DCACHE_MOUNTED			0x00010000 /* is a mountpoint */
-#define DCACHE_NEED_AUTOMOUNT		0x00020000 /* handle automount on this dir */
-#define DCACHE_MANAGE_TRANSIT		0x00040000 /* manage transit from this dirent */
+#define DCACHE_MOUNTED			BIT(16) /* is a mountpoint */
+#define DCACHE_NEED_AUTOMOUNT		BIT(17) /* handle automount on this dir */
+#define DCACHE_MANAGE_TRANSIT		BIT(18) /* manage transit from this dirent */
 #define DCACHE_MANAGED_DENTRY \
 	(DCACHE_MOUNTED|DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT)
 
-#define DCACHE_LRU_LIST			0x00080000
-
-#define DCACHE_ENTRY_TYPE		0x00700000
-#define DCACHE_MISS_TYPE		0x00000000 /* Negative dentry (maybe fallthru to nowhere) */
-#define DCACHE_WHITEOUT_TYPE		0x00100000 /* Whiteout dentry (stop pathwalk) */
-#define DCACHE_DIRECTORY_TYPE		0x00200000 /* Normal directory */
-#define DCACHE_AUTODIR_TYPE		0x00300000 /* Lookupless directory (presumed automount) */
-#define DCACHE_REGULAR_TYPE		0x00400000 /* Regular file type (or fallthru to such) */
-#define DCACHE_SPECIAL_TYPE		0x00500000 /* Other file type (or fallthru to such) */
-#define DCACHE_SYMLINK_TYPE		0x00600000 /* Symlink (or fallthru to such) */
-
-#define DCACHE_MAY_FREE			0x00800000
-#define DCACHE_FALLTHRU			0x01000000 /* Fall through to lower layer */
-#define DCACHE_NOKEY_NAME		0x02000000 /* Encrypted name encoded without key */
-#define DCACHE_OP_REAL			0x04000000
-
-#define DCACHE_PAR_LOOKUP		0x10000000 /* being looked up (with parent locked shared) */
-#define DCACHE_DENTRY_CURSOR		0x20000000
-#define DCACHE_NORCU			0x40000000 /* No RCU delay for freeing */
+#define DCACHE_LRU_LIST			BIT(19)
+
+#define DCACHE_ENTRY_TYPE		(7 << 20) /* bits 20..22 are for storing type: */
+#define DCACHE_MISS_TYPE		(0 << 20) /* Negative dentry (maybe fallthru to nowhere) */
+#define DCACHE_WHITEOUT_TYPE		(1 << 20) /* Whiteout dentry (stop pathwalk) */
+#define DCACHE_DIRECTORY_TYPE		(2 << 20) /* Normal directory */
+#define DCACHE_AUTODIR_TYPE		(3 << 20) /* Lookupless directory (presumed automount) */
+#define DCACHE_REGULAR_TYPE		(4 << 20) /* Regular file type (or fallthru to such) */
+#define DCACHE_SPECIAL_TYPE		(5 << 20) /* Other file type (or fallthru to such) */
+#define DCACHE_SYMLINK_TYPE		(6 << 20) /* Symlink (or fallthru to such) */
+
+#define DCACHE_MAY_FREE			BIT(23)
+#define DCACHE_FALLTHRU			BIT(24) /* Fall through to lower layer */
+#define DCACHE_NOKEY_NAME		BIT(25) /* Encrypted name encoded without key */
+#define DCACHE_OP_REAL			BIT(26)
+
+#define DCACHE_PAR_LOOKUP		BIT(28) /* being looked up (with parent locked shared) */
+#define DCACHE_DENTRY_CURSOR		BIT(29)
+#define DCACHE_NORCU			BIT(30) /* No RCU delay for freeing */
 
 extern seqlock_t rename_lock;

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #6 lore raw

PATCH 6/20 DCACHE_COOKIE: RIP

In reply to Al Viro

the last user gone in 2021...

Signed-off-by Al Viro <[email protected]>

---
 include/linux/dcache.h | 1 -
 1 file changed, 1 deletion(-)

include/linux/dcache.h +0 -1

index d9c314cc93b8..92c0b2a1ae2e 100644
@@ -181,7 +181,6 @@ struct dentry_operations {
 #define DCACHE_NFSFS_RENAMED		BIT(12)
      /* this dentry has been "silly renamed" and has to be deleted on the last
       * dput() */
-#define DCACHE_COOKIE			BIT(13) /* For use by dcookie subsystem */
 #define DCACHE_FSNOTIFY_PARENT_WATCHED	BIT(14)
      /* Parent inode is watched by some fsnotify listener */

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #7 lore raw

PATCH 7/20 kill d_{is,set}_fallthru()

In reply to Al Viro

Introduced in 2015 and never had any in-tree users...

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c            | 20 ++------------------
 include/linux/dcache.h | 17 ++++-------------
 2 files changed, 6 insertions(+), 31 deletions(-)

fs/dcache.c +2 ~~-18~~

index b8f1b54a1492..9f5b2b5c1e6d 100644
@@ -344,7 +344,7 @@ static inline void __d_set_inode_and_type(struct dentry *dentry,
 
 	dentry->d_inode = inode;
 	flags = READ_ONCE(dentry->d_flags);
-	flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
+	flags &= ~DCACHE_ENTRY_TYPE;
 	flags |= type_flags;
 	smp_store_release(&dentry->d_flags, flags);
 }

@@ -353,7 +353,7 @@ static inline void __d_clear_type_and_inode(struct dentry *dentry)
 {
 	unsigned flags = READ_ONCE(dentry->d_flags);
 
-	flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU);
+	flags &= ~DCACHE_ENTRY_TYPE;
 	WRITE_ONCE(dentry->d_flags, flags);
 	dentry->d_inode = NULL;
 	if (dentry->d_flags & DCACHE_LRU_LIST)

@@ -1936,22 +1936,6 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
 }
 EXPORT_SYMBOL(d_set_d_op);
 
-
-/*
- * d_set_fallthru - Mark a dentry as falling through to a lower layer
- * @dentry - The dentry to mark
- *
- * Mark a dentry as falling through to the lower layer (as set with
- * d_pin_lower()).  This flag may be recorded on the medium.
- */
-void d_set_fallthru(struct dentry *dentry)
-{
-	spin_lock(&dentry->d_lock);
-	dentry->d_flags |= DCACHE_FALLTHRU;
-	spin_unlock(&dentry->d_lock);
-}
-EXPORT_SYMBOL(d_set_fallthru);
-
 static unsigned d_flags_for_inode(struct inode *inode)
 {
 	unsigned add_flags = DCACHE_REGULAR_TYPE;

include/linux/dcache.h +4 ~~-13~~

index 92c0b2a1ae2e..8cd937bb2292 100644
@@ -195,16 +195,15 @@ struct dentry_operations {
 #define DCACHE_LRU_LIST			BIT(19)
 
 #define DCACHE_ENTRY_TYPE		(7 << 20) /* bits 20..22 are for storing type: */
-#define DCACHE_MISS_TYPE		(0 << 20) /* Negative dentry (maybe fallthru to nowhere) */
+#define DCACHE_MISS_TYPE		(0 << 20) /* Negative dentry */
 #define DCACHE_WHITEOUT_TYPE		(1 << 20) /* Whiteout dentry (stop pathwalk) */
 #define DCACHE_DIRECTORY_TYPE		(2 << 20) /* Normal directory */
 #define DCACHE_AUTODIR_TYPE		(3 << 20) /* Lookupless directory (presumed automount) */
-#define DCACHE_REGULAR_TYPE		(4 << 20) /* Regular file type (or fallthru to such) */
-#define DCACHE_SPECIAL_TYPE		(5 << 20) /* Other file type (or fallthru to such) */
-#define DCACHE_SYMLINK_TYPE		(6 << 20) /* Symlink (or fallthru to such) */
+#define DCACHE_REGULAR_TYPE		(4 << 20) /* Regular file type */
+#define DCACHE_SPECIAL_TYPE		(5 << 20) /* Other file type */
+#define DCACHE_SYMLINK_TYPE		(6 << 20) /* Symlink */
 
 #define DCACHE_MAY_FREE			BIT(23)
-#define DCACHE_FALLTHRU			BIT(24) /* Fall through to lower layer */
 #define DCACHE_NOKEY_NAME		BIT(25) /* Encrypted name encoded without key */
 #define DCACHE_OP_REAL			BIT(26)

@@ -489,14 +488,6 @@ static inline int simple_positive(const struct dentry *dentry)
 	return d_really_is_positive(dentry) && !d_unhashed(dentry);
 }
 
-extern void d_set_fallthru(struct dentry *dentry);
-
-static inline bool d_is_fallthru(const struct dentry *dentry)
-{
-	return dentry->d_flags & DCACHE_FALLTHRU;
-}
-
-
 extern int sysctl_vfs_cache_pressure;
 
 static inline unsigned long vfs_pressure_ratio(unsigned long val)

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #8 lore raw

PATCH 8/20 dentry.h: trim externs

In reply to Al Viro

d_instantiate_unique() had been gone for 7 years; __d_lookup...() and shrink_dcache_for_umount() are fs/internal.h fodder.

Signed-off-by Al Viro <[email protected]>

---
 fs/internal.h          | 4 ++++
 include/linux/dcache.h | 5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)

fs/internal.h +4 -0

index 58e43341aebf..9e9fc629f935 100644
@@ -215,6 +215,10 @@ extern struct dentry * d_alloc_pseudo(struct super_block *, const struct qstr *)
 extern char *simple_dname(struct dentry *, char *, int);
 extern void dput_to_list(struct dentry *, struct list_head *);
 extern void shrink_dentry_list(struct list_head *);
+extern void shrink_dcache_for_umount(struct super_block *);
+extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *);
+extern struct dentry *__d_lookup_rcu(const struct dentry *parent,
+				const struct qstr *name, unsigned *seq);
 
 /*
  * pipe.c

include/linux/dcache.h +0 -5

index 8cd937bb2292..9706bf1dc5de 100644
@@ -218,7 +218,6 @@ extern seqlock_t rename_lock;
  */
 extern void d_instantiate(struct dentry *, struct inode *);
 extern void d_instantiate_new(struct dentry *, struct inode *);
-extern struct dentry * d_instantiate_unique(struct dentry *, struct inode *);
 extern struct dentry * d_instantiate_anon(struct dentry *, struct inode *);
 extern void __d_drop(struct dentry *dentry);
 extern void d_drop(struct dentry *dentry);

@@ -240,7 +239,6 @@ extern struct dentry * d_obtain_alias(struct inode *);
 extern struct dentry * d_obtain_root(struct inode *);
 extern void shrink_dcache_sb(struct super_block *);
 extern void shrink_dcache_parent(struct dentry *);
-extern void shrink_dcache_for_umount(struct super_block *);
 extern void d_invalidate(struct dentry *);
 
 /* only used at mount-time */

@@ -275,9 +273,6 @@ extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
 /* appendix may either be NULL or be used for transname suffixes */
 extern struct dentry *d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);
-extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *);
-extern struct dentry *__d_lookup_rcu(const struct dentry *parent,
-				const struct qstr *name, unsigned *seq);
 
 static inline unsigned d_count(const struct dentry *dentry)
 {

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #9 lore raw

PATCH 9/20 dentry.h: kill a mysterious comment

In reply to Al Viro

there's a strange comment in front of d_lookup() declaration:

/* appendix may either be NULL or be used for transname suffixes */

Looks like nobody had been curious enough to track its history; it predates git, it predates bitkeeper and if you look through the pre-BK trees, you finally arrive at this in 2.1.44-for-davem:
/* appendix may either be NULL or be used for transname suffixes */
-extern struct dentry * d_lookup(struct inode * dir, struct qstr * name,
- struct qstr * appendix);
+extern struct dentry * d_lookup(struct dentry * dir, struct qstr * name);
In other words, it refers to the third argument d_lookup() used to have back then. It had been introduced in 2.1.43-pre, on June 12 1997, along with d_lookup(), only to be removed by July 4 1997, presumably when the Cthulhu-awful thing it used to be used for (look for CONFIG_TRANS_NAMES in 2.1.43-pre, and keep a heavy-duty barfbag ready) had been, er, noticed and recognized for what it had been.

Despite the appendectomy, the comment remained. Some things really need to be put out of their misery...

Signed-off-by Al Viro <[email protected]>

---
 include/linux/dcache.h | 1 -
 1 file changed, 1 deletion(-)

include/linux/dcache.h +0 -1

index 9706bf1dc5de..a5e5e274eee0 100644
@@ -270,7 +270,6 @@ extern void d_move(struct dentry *, struct dentry *);
 extern void d_exchange(struct dentry *, struct dentry *);
 extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
 
-/* appendix may either be NULL or be used for transname suffixes */
 extern struct dentry *d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);

Signature · 1 line

2.39.2

Amir Goldstein

maintainer

2023-11-24 07:37 UTC #10 lore raw

Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment

Quoted 32 lines · Al Viro

there's a strange comment in front of d_lookup() declaration:
/* appendix may either be NULL or be used for transname suffixes */
Looks like nobody had been curious enough to track its history; it predates git, it predates bitkeeper and if you look through the pre-BK trees, you finally arrive at this in 2.1.44-for-davem:
/* appendix may either be NULL or be used for transname suffixes */
-extern struct dentry * d_lookup(struct inode * dir, struct qstr * name,
- struct qstr * appendix);
+extern struct dentry * d_lookup(struct dentry * dir, struct qstr * name);
In other words, it refers to the third argument d_lookup() used to have back then. It had been introduced in 2.1.43-pre, on June 12 1997, along with d_lookup(), only to be removed by July 4 1997, presumably when the Cthulhu-awful thing it used to be used for (look for CONFIG_TRANS_NAMES in 2.1.43-pre, and keep a heavy-duty barfbag ready) had been, er, noticed and recognized for what it had been.
Despite the appendectomy, the comment remained. Some things really need to be put out of their misery...
Signed-off-by Al Viro <[email protected]>
---
 include/linux/dcache.h | 1 -
 1 file changed, 1 deletion(-)
include/linux/dcache.h +0 -0
index 9706bf1dc5de..a5e5e274eee0 100644

include/linux/dcache.h extern void d_move(struct dentry *, struct dentry *); · line 275

Quoted 7 lines

include/linux/dcache.h +0 -1

@@ -270,7 +270,6 @@ extern void d_move(struct dentry *, struct dentry *);
 extern void d_exchange(struct dentry *, struct dentry *);
 extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
 
-/* appendix may either be NULL or be used for transname suffixes */
 extern struct dentry *d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);

Al,

Since you like pre-git archeology...

Mind digging up what this comment in fs.h is about:

/* needed for stackable file system support */
extern loff_t default_llseek(struct file *file, loff_t offset, int whence);

Or we can just remove it without digging up what the comment used to refer to ;)

Thanks,
Amir.

Al Viro

author maintainer

2023-11-24 08:11 UTC #11 lore raw

Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment

Quoted 6 lines · Amir Goldstein

Since you like pre-git archeology...
Mind digging up what this comment in fs.h is about:
/* needed for stackable file system support */
extern loff_t default_llseek(struct file *file, loff_t offset, int whence);

Umm... I think it was about ecryptfs and its ilk, but it was a long time ago...

<looks>

2.3.99pre6, along with exporting it:
-/* for stackable file systems (lofs, wrapfs, etc.) */ -EXPORT_SYMBOL(add_to_page_cache);
+/* for stackable file systems (lofs, wrapfs, cryptfs, etc.) */ +EXPORT_SYMBOL(default_llseek); +EXPORT_SYMBOL(dentry_open);

Back then ->llseek == NULL used to mean default_llseek; that had been changed much later. And that was before vfs_llseek() as well, so any layered filesystem had to open-code it - which required default_llseek().

The comment is certainly stale, though - stackable filesystems do *not* need it (vfs_llseek() is there), but every file_operation that used to have NULL ->llseek does.

Quoted 2 lines

Or we can just remove it without digging up what the comment used to refer to ;)

Too late - it will have to be removed with that ;-)

Al Viro

author maintainer

2023-11-24 08:26 UTC #12 lore raw

Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment

Quoted 26 lines · Al Viro

Quoted 6 lines · Amir Goldstein
Since you like pre-git archeology...
Mind digging up what this comment in fs.h is about:
/* needed for stackable file system support */
extern loff_t default_llseek(struct file *file, loff_t offset, int whence);
Umm... I think it was about ecryptfs and its ilk, but it was a long time ago...
<looks>
2.3.99pre6, along with exporting it:
(quoted diff) +3 -2
-/* for stackable file systems (lofs, wrapfs, etc.) */
-EXPORT_SYMBOL(add_to_page_cache);
+/* for stackable file systems (lofs, wrapfs, cryptfs, etc.) */
+EXPORT_SYMBOL(default_llseek);
+EXPORT_SYMBOL(dentry_open);
Back then ->llseek == NULL used to mean default_llseek; that had been changed much later. And that was before vfs_llseek() as well, so any layered filesystem had to open-code it - which required default_llseek().

Quoted 8 lines

The comment is certainly stale, though - stackable filesystems do *not* need it (vfs_llseek() is there), but every file_operation that used to have NULL ->llseek does.
Quoted 2 lines
Or we can just remove it without digging up what the comment used to refer to ;)
Too late - it will have to be removed with that ;-)

BTW, there's this, covering more than just BK times:

git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/

Christian Brauner

maintainer

2023-11-24 08:29 UTC #13 lore raw

Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment

Quoted 3 lines

BTW, there's this, covering more than just BK times:
git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/

An absolute life-saver at times this repo for the ones of us who haven't been around for 20+ years...

Al Viro

author maintainer

2023-11-24 06:06 UTC #14 lore raw

PATCH 10/20 kill d_backing_dentry()

In reply to Al Viro

no users left

Signed-off-by Al Viro <[email protected]>

---
 include/linux/dcache.h | 15 ---------------
 1 file changed, 15 deletions(-)

include/linux/dcache.h +0 ~~-15~~

index a5e5e274eee0..fa0414cff85c 100644
@@ -530,21 +530,6 @@ static inline struct inode *d_backing_inode(const struct dentry *upper)
 	return inode;
 }
 
-/**
- * d_backing_dentry - Get upper or lower dentry we should be using
- * @upper: The upper layer
- *
- * This is the helper that should be used to get the dentry of the inode that
- * will be used if this dentry were opened as a file.  It may be the upper
- * dentry or it may be a lower dentry pinned by the upper.
- *
- * Normal filesystems should not use this to access their own dentries.
- */
-static inline struct dentry *d_backing_dentry(struct dentry *upper)
-{
-	return upper;
-}
-
 /**
  * d_real - Return the real dentry
  * @dentry: the dentry to query

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #15 lore raw

PATCH 11/20 kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller

In reply to Al Viro

now that the only user of d_instantiate_anon() is gone...

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c            | 88 ++++++++++++++++--------------------------
 include/linux/dcache.h |  1 -
 2 files changed, 33 insertions(+), 56 deletions(-)

fs/dcache.c +33 ~~-55~~

index 9f5b2b5c1e6d..6dde21dbdd1a 100644
@@ -2054,75 +2054,53 @@ struct dentry *d_make_root(struct inode *root_inode)
 }
 EXPORT_SYMBOL(d_make_root);
 
-static struct dentry *__d_instantiate_anon(struct dentry *dentry,
-					   struct inode *inode,
-					   bool disconnected)
-{
-	struct dentry *res;
-	unsigned add_flags;
-
-	security_d_instantiate(dentry, inode);
-	spin_lock(&inode->i_lock);
-	res = __d_find_any_alias(inode);
-	if (res) {
-		spin_unlock(&inode->i_lock);
-		dput(dentry);
-		goto out_iput;
-	}
-
-	/* attach a disconnected dentry */
-	add_flags = d_flags_for_inode(inode);
-
-	if (disconnected)
-		add_flags |= DCACHE_DISCONNECTED;
-
-	spin_lock(&dentry->d_lock);
-	__d_set_inode_and_type(dentry, inode, add_flags);
-	hlist_add_head(&dentry->d_u.d_alias, &inode->i_dentry);
-	if (!disconnected) {
-		hlist_bl_lock(&dentry->d_sb->s_roots);
-		hlist_bl_add_head(&dentry->d_hash, &dentry->d_sb->s_roots);
-		hlist_bl_unlock(&dentry->d_sb->s_roots);
-	}
-	spin_unlock(&dentry->d_lock);
-	spin_unlock(&inode->i_lock);
-
-	return dentry;
-
- out_iput:
-	iput(inode);
-	return res;
-}
-
-struct dentry *d_instantiate_anon(struct dentry *dentry, struct inode *inode)
-{
-	return __d_instantiate_anon(dentry, inode, true);
-}
-EXPORT_SYMBOL(d_instantiate_anon);
-
 static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
 {
-	struct dentry *tmp;
-	struct dentry *res;
+	struct super_block *sb = inode->i_sb;
+	struct dentry *new, *res;
 
 	if (!inode)
 		return ERR_PTR(-ESTALE);
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
 
-	res = d_find_any_alias(inode);
+	res = d_find_any_alias(inode); /* existing alias? */
 	if (res)
-		goto out_iput;
+		goto out;
 
-	tmp = d_alloc_anon(inode->i_sb);
-	if (!tmp) {
+	new = d_alloc_anon(sb);
+	if (!new) {
 		res = ERR_PTR(-ENOMEM);
-		goto out_iput;
+		goto out;
 	}
 
-	return __d_instantiate_anon(tmp, inode, disconnected);
+	security_d_instantiate(new, inode);
+	spin_lock(&inode->i_lock);
+	res = __d_find_any_alias(inode); /* recheck under lock */
+	if (likely(!res)) { /* still no alias, attach a disconnected dentry */
+		unsigned add_flags = d_flags_for_inode(inode);
+
+		if (disconnected)
+			add_flags |= DCACHE_DISCONNECTED;
+
+		spin_lock(&new->d_lock);
+		__d_set_inode_and_type(new, inode, add_flags);
+		hlist_add_head(&new->d_u.d_alias, &inode->i_dentry);
+		if (!disconnected) {
+			hlist_bl_lock(&sb->s_roots);
+			hlist_bl_add_head(&new->d_hash, &sb->s_roots);
+			hlist_bl_unlock(&sb->s_roots);
+		}
+		spin_unlock(&new->d_lock);
+		spin_unlock(&inode->i_lock);
+		inode = NULL; /* consumed by new->d_inode */
+		res = new;
+	} else {
+		spin_unlock(&inode->i_lock);
+		dput(new);
+	}
 
-out_iput:
+ out:
 	iput(inode);
 	return res;
 }

include/linux/dcache.h +0 -1

index fa0414cff85c..8c5e3bdf1147 100644
@@ -218,7 +218,6 @@ extern seqlock_t rename_lock;
  */
 extern void d_instantiate(struct dentry *, struct inode *);
 extern void d_instantiate_new(struct dentry *, struct inode *);
-extern struct dentry * d_instantiate_anon(struct dentry *, struct inode *);
 extern void __d_drop(struct dentry *dentry);
 extern void d_drop(struct dentry *dentry);
 extern void d_delete(struct dentry *);

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #16 lore raw

PATCH 12/20 d_alloc_pseudo(): move setting ->d_op there from the (sole) caller

In reply to Al Viro

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c     | 8 +++++++-
 fs/file_table.c | 5 -----
 2 files changed, 7 insertions(+), 6 deletions(-)

fs/dcache.c +7 -1

index 6dde21dbdd1a..1296a3fa4f93 100644
@@ -1890,9 +1890,15 @@ struct dentry *d_alloc_cursor(struct dentry * parent)
  */
 struct dentry *d_alloc_pseudo(struct super_block *sb, const struct qstr *name)
 {
+	static const struct dentry_operations anon_ops = {
+		.d_dname = simple_dname
+	};
 	struct dentry *dentry = __d_alloc(sb, name);
-	if (likely(dentry))
+	if (likely(dentry)) {
 		dentry->d_flags |= DCACHE_NORCU;
+		if (!sb->s_d_op)
+			d_set_d_op(dentry, &anon_ops);
+	}
 	return dentry;
 }

fs/file_table.c +0 -5

index de4a2915bfd4..8889cbee13f8 100644
@@ -329,9 +329,6 @@ struct file *alloc_file_pseudo(struct inode *inode, struct vfsmount *mnt,
 				const char *name, int flags,
 				const struct file_operations *fops)
 {
-	static const struct dentry_operations anon_ops = {
-		.d_dname = simple_dname
-	};
 	struct qstr this = QSTR_INIT(name, strlen(name));
 	struct path path;
 	struct file *file;

@@ -339,8 +336,6 @@ struct file *alloc_file_pseudo(struct inode *inode, struct vfsmount *mnt,
 	path.dentry = d_alloc_pseudo(mnt->mnt_sb, &this);
 	if (!path.dentry)
 		return ERR_PTR(-ENOMEM);
-	if (!mnt->mnt_sb->s_d_op)
-		d_set_d_op(path.dentry, &anon_ops);
 	path.mnt = mntget(mnt);
 	d_instantiate(path.dentry, inode);
 	file = alloc_file(&path, flags, fops);

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #17 lore raw

PATCH 13/20 nsfs: use d_make_root()

In reply to Al Viro

Normally d_make_root() is used to create the root dentry of superblock; here we use it for a different purpose, but... idiomatic or not, we need the same operation.

Signed-off-by Al Viro <[email protected]>

---
 fs/nsfs.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

fs/nsfs.c +2 -5

index 9a4b228d42fa..34e1e3e36733 100644
@@ -90,12 +90,9 @@ static int __ns_get_path(struct path *path, struct ns_common *ns)
 	inode->i_fop = &ns_file_operations;
 	inode->i_private = ns;
 
-	dentry = d_alloc_anon(mnt->mnt_sb);
-	if (!dentry) {
-		iput(inode);
+	dentry = d_make_root(inode);	/* not the normal use, but... */
+	if (!dentry)
 		return -ENOMEM;
-	}
-	d_instantiate(dentry, inode);
 	dentry->d_fsdata = (void *)ns->ops;
 	d = atomic_long_cmpxchg(&ns->stashed, 0, (unsigned long)dentry);
 	if (d) {

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #18 lore raw

PATCH 14/20 simple_fill_super(): don't bother with d_genocide() on failure

In reply to Al Viro

Failing ->fill_super() will be followed by ->kill_sb(), which should include kill_litter_super() if the call of simple_fill_super() had been asked to create anything besides the root dentry. So there's no need to empty the partially populated tree - it will be trimmed by inevitable kill_litter_super().

Signed-off-by Al Viro <[email protected]>

---
 fs/libfs.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

fs/libfs.c +5 ~~-12~~

index e9440d55073c..6fa8ad36049f 100644
@@ -912,7 +912,6 @@ int simple_fill_super(struct super_block *s, unsigned long magic,
 		      const struct tree_descr *files)
 {
 	struct inode *inode;
-	struct dentry *root;
 	struct dentry *dentry;
 	int i;

@@ -935,8 +934,8 @@ int simple_fill_super(struct super_block *s, unsigned long magic,
 	inode->i_op = &simple_dir_inode_operations;
 	inode->i_fop = &simple_dir_operations;
 	set_nlink(inode, 2);
-	root = d_make_root(inode);
-	if (!root)
+	s->s_root = d_make_root(inode);
+	if (!s->s_root)
 		return -ENOMEM;
 	for (i = 0; !files->name || files->name[0]; i++, files++) {
 		if (!files->name)

@@ -948,13 +947,13 @@ int simple_fill_super(struct super_block *s, unsigned long magic,
 				"with an index of 1!\n", __func__,
 				s->s_type->name);
 
-		dentry = d_alloc_name(root, files->name);
+		dentry = d_alloc_name(s->s_root, files->name);
 		if (!dentry)
-			goto out;
+			return -ENOMEM;
 		inode = new_inode(s);
 		if (!inode) {
 			dput(dentry);
-			goto out;
+			return -ENOMEM;
 		}
 		inode->i_mode = S_IFREG | files->mode;
 		simple_inode_init_ts(inode);

@@ -962,13 +961,7 @@ int simple_fill_super(struct super_block *s, unsigned long magic,
 		inode->i_ino = i;
 		d_add(dentry, inode);
 	}
-	s->s_root = root;
 	return 0;
-out:
-	d_genocide(root);
-	shrink_dcache_parent(root);
-	dput(root);
-	return -ENOMEM;
 }
 EXPORT_SYMBOL(simple_fill_super);

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #19 lore raw

PATCH 15/20 d_genocide(): move the extern into fs/internal.h

In reply to Al Viro

Signed-off-by Al Viro <[email protected]>

---
 fs/internal.h          | 1 +
 include/linux/dcache.h | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

fs/internal.h +1 -0

index 9e9fc629f935..d9a920e2636e 100644
@@ -219,6 +219,7 @@ extern void shrink_dcache_for_umount(struct super_block *);
 extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *__d_lookup_rcu(const struct dentry *parent,
 				const struct qstr *name, unsigned *seq);
+extern void d_genocide(struct dentry *);
 
 /*
  * pipe.c

include/linux/dcache.h +0 -3

index 8c5e3bdf1147..b4324d47f249 100644
@@ -243,9 +243,6 @@ extern void d_invalidate(struct dentry *);
 /* only used at mount-time */
 extern struct dentry * d_make_root(struct inode *);
 
-/* <clickety>-<click> the ramfs-type tree */
-extern void d_genocide(struct dentry *);
-
 extern void d_mark_tmpfile(struct file *, struct inode *);
 extern void d_tmpfile(struct file *, struct inode *);

Signature · 1 line

2.39.2

Cedric Blancher

2023-11-24 06:36 UTC #20 lore raw

d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? Re: [PATCH 15/20] d_genocide(): move the extern into fs/internal.h

Quoted 12 lines · Al Viro

Signed-off-by Al Viro <[email protected]>

---
 fs/internal.h          | 1 +
 include/linux/dcache.h | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

fs/internal.h +0 -0

index 9e9fc629f935..d9a920e2636e 100644

fs/internal.h extern void shrink_dcache_for_umount(struct super_block *); · line 222

Quoted 4 lines

fs/internal.h +1 -0

@@ -219,6 +219,7 @@ extern void shrink_dcache_for_umount(struct super_block *);
 extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *__d_lookup_rcu(const struct dentry *parent,
                                const struct qstr *name, unsigned *seq);
+extern void d_genocide(struct dentry *);

Seriously, who came up with THAT name? "Genocide" is not a nice term, not even if you ignore political correctness.
Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()?

Ced

Quoted 18 lines

fs/internal.h +0 -0

 /*
  * pipe.c

include/linux/dcache.h +0 -4

index 8c5e3bdf1147..b4324d47f249 100644
@@ -243,9 +243,6 @@ extern void d_invalidate(struct dentry *);
 /* only used at mount-time */
 extern struct dentry * d_make_root(struct inode *);
 
-/* <clickety>-<click> the ramfs-type tree */
-extern void d_genocide(struct dentry *);
-
 extern void d_mark_tmpfile(struct file *, struct inode *);
 extern void d_tmpfile(struct file *, struct inode *);
 
--

Quoted 3 lines

2.39.2

--
Cedric Blancher <[email protected]>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur

Al Viro

author maintainer

2023-11-24 06:58 UTC #21 lore raw

Re: d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? Re: [PATCH 15/20] d_genocide(): move the extern into fs/internal.h

Quoted 8 lines · Cedric Blancher

Quoted 7 lines · Al Viro
Signed-off-by Al Viro <[email protected]>
---
 fs/internal.h          | 1 +
 include/linux/dcache.h | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

Quoted 13 lines

fs/internal.h extern void shrink_dcache_for_umount(struct super_block *); · line 222
Quoted 9 lines
fs/internal.h +1 -0
index 9e9fc629f935..d9a920e2636e 100644
@@ -219,6 +219,7 @@ extern void shrink_dcache_for_umount(struct super_block *);
 extern struct dentry *__d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *__d_lookup_rcu(const struct dentry *parent,
                                const struct qstr *name, unsigned *seq);
+extern void d_genocide(struct dentry *);
Seriously, who came up with THAT name? "Genocide" is not a nice term, not even if you ignore political correctness.
Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()?

kill_them_all(), on the account of that being what it's doing?

I can explain the problems with each of your suggested identifiers, if you really wish that, but I would stronly suggest taking that off-list.

As for the bad words... google "jesux" someday. Yes, we do have identifiers like "kill", "abort", etc. and those are really not going anywhere; live with it.

Al Viro

author maintainer

2023-11-24 07:48 UTC #22 lore raw

Re: d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? Re: [PATCH 15/20] d_genocide(): move the extern into fs/internal.h

Quoted 7 lines · Al Viro

Quoted 5 lines
Quoted 1 line
+extern void d_genocide(struct dentry *);
Seriously, who came up with THAT name? "Genocide" is not a nice term, not even if you ignore political correctness.
Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()?
kill_them_all(), on the account of that being what it's doing?

To elaborate a bit: what that function does (well, tries to do - it has serious limitations, which is why there is only one caller remaining and that one is used only when nothing else can access the filesystem anymore) is "kill given dentry, along with all its children, all their children, etc."

I sincerely doubt that you will be able to come up with _any_ word describing such action in any real-world context that would not come with very nasty associations.

Context: a bunch of filesystems have directory tree entirely in dcache; creating a file or directory bumps the reference count of dentry in question, so instead of going back to 0 after e.g. mkdir(2) returns it is left with refcount 1, which prevents its eviction. In effect, all positive dentries in there are artificially kept busy. On rmdir(2) or unlink(2) that extra reference is dropped and they get evicted.

For filesystems like e.g. tmpfs that's a fairly obvious approach - they don't *have* any backing store, so dcache is not just caching the underlying objects - it's all there is.

For such filesystems there is a quick way to do an equivalent of rm -rf - simply go over the subtree you want to remove and decrement refcounts of everything positive. That's fine on filesystem shutdown, but for anything in use it is *too* quick - you'd better not do that if there are mountpoints in the subtree you are removing, etc.

At the moment we have 3 callers in the kernel; one in selinuxfs, removing stale directories on policy reload (not quite safe, but if attacker can do selinux policy reload, you are beyond lost), another in simple_fill_super() failure handling (safe, since filesystem is not mounted at the time, but actually pointless - normal cleanup after failure will take them out just fine) and the last one in kill_litter_super(). That one is actually fine - we are shutting the filesystem down and nobody can access it at that point unless the kernel is deeply broken.

By the end of this series only that one caller remains, which is reason for taking the declaration from include/linux/dcache.h to fs/internal.h - making sure no new callers get added. Not because of the identifier having nasty connotations, but because it's pretty hard to use correctly.

Martin Steigerwald

2023-11-24 09:41 UTC #23 lore raw

Re: d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? Re: [PATCH 15/20] d_genocide(): move the extern into fs/internal.h

Al Viro - 24.11.23, 08:48:57 CET:

Quoted 8 lines · Al Viro

Quoted 7 lines
Quoted 7 lines
Quoted 1 line
+extern void d_genocide(struct dentry *);
Seriously, who came up with THAT name? "Genocide" is not a nice term,
not even if you ignore political correctness.
Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()?>

Quoted 7 lines

Quoted 1 line
kill_them_all(), on the account of that being what it's doing?
To elaborate a bit: what that function does (well, tries to do - it has serious limitations, which is why there is only one caller remaining and that one is used only when nothing else can access the filesystem anymore) is "kill given dentry, along with all its children, all their children, etc."

I never got why in the context of computers anything is ever being killed. It does not live to begin with.

You can stop something, remove it, delete it, destroy it, pause it, resume it, overwrite it and you can do it really quickly or (almost) instantly or slowly or recursively or some combination of those, but kill? You cannot kill what does not live.

d_delete/destroy/remove_recursively() could be a suitable function name. Pick one.

Similar it is with the term children or parent. There are no children in computer software. Period. But here it may be more difficult to find alternative wording. Would still be good to find something, cause I was quite taken aback by the wording of the OOM killer. (Actually I was taken aback that an operating system could even have something that forcefully quits a process without saving data. It never matched my expectations of reliability and stability.)

So how about stopping to put meaning into computer software source code that simply is not there to begin with? How about starting to use terms that describe what is actually being done and what is actually there?

Signature · 1 line

Martin

Al Viro

author maintainer

2023-11-24 18:21 UTC #24 lore raw

Re: identifiers

[search bait removed from subject]

Quoted 9 lines · Martin Steigerwald

Al Viro - 24.11.23, 08:48:57 CET:
Quoted 5 lines
To elaborate a bit: what that function does (well, tries to do - it has serious limitations, which is why there is only one caller remaining and that one is used only when nothing else can access the filesystem anymore) is "kill given dentry, along with all its children, all their children, etc."
I never got why in the context of computers anything is ever being killed. It does not live to begin with.

Simple - one deals with objects that have complex lifecycle, with very different possible behaviour at various stages. And about the only example of such that would be well-covered in natural languages is just that - both in adjectives for states and verbs for transitions between those.

Note that the word "lifecycle" itself is rather commonly used outside of biological context.

Quoted 4 lines

You can stop something, remove it, delete it, destroy it, pause it, resume it, overwrite it and you can do it really quickly or (almost) instantly or slowly or recursively or some combination of those, but kill? You cannot kill what does not live.

Why? "Do something that changes the state of target into one in which the target gradually becomes incapable of normal activity until it goes completely inert and eventually disappears, with its parts recycled for unrelated objects" vs. "kill the target", with associated transitional state being refered to as "dying"?

Your suggestions all refer to operation rather than state transition.

Quoted 2 lines

d_delete/destroy/remove_recursively() could be a suitable function name. Pick one.

Thanks, but no thanks. d_delete() already exists and refers to rather different operation; "destroy" in such contexts would be loaded with an existing technical meaning, and that would be actively confusing; "remove_recursively"? Guess what the better-behaving replacement (far too heavy-weight for the only remaining use) is called? "simple_recursive_removal". It does more than this one, though.

Quoted 2 lines

Similar it is with the term children or parent. There are no children in computer software. Period.

Well-asserted. Unfortunately, the statement is wrong - "parents" and "children" have specific meanings when applied to nodes of directed graphs. And there's a plenty of those dealt with by software. Directory tree, in particular.

Askar Safin

2025-12-09 22:29 UTC #25 lore raw

Re: d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? Re: [PATCH 15/20] d_genocide(): move the extern into fs/internal.h

In reply to Cedric Blancher

Cedric Blancher <[email protected]>:

Quoted 5 lines

Quoted 1 line
+extern void d_genocide(struct dentry *);
Seriously, who came up with THAT name? "Genocide" is not a nice term, not even if you ignore political correctness.
Or what will be next? d_holodomor()? d_massmurder()? d_execute_warcrimes()?

Good news! :) Term "genocide" finally gone from kernel some days ago!

In this patchset: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7cd122b55283d3ceef71a5b723ccaa03a72284b4
( https://lore.kernel.org/all/[email protected]/ ).

Signature · 1 line

Askar Safin

Al Viro

author maintainer

2023-11-24 06:06 UTC #26 lore raw

PATCH 16/20 get rid of DCACHE_GENOCIDE

In reply to Al Viro

... now that we never call d_genocide() other than from kill_litter_super()

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c            | 5 +----
 include/linux/dcache.h | 1 -
 2 files changed, 1 insertion(+), 5 deletions(-)

fs/dcache.c +1 -4

index 1296a3fa4f93..440b46851f52 100644
@@ -3196,10 +3196,7 @@ static enum d_walk_ret d_genocide_kill(void *data, struct dentry *dentry)
 		if (d_unhashed(dentry) || !dentry->d_inode)
 			return D_WALK_SKIP;
 
-		if (!(dentry->d_flags & DCACHE_GENOCIDE)) {
-			dentry->d_flags |= DCACHE_GENOCIDE;
-			dentry->d_lockref.count--;
-		}
+		dentry->d_lockref.count--;
 	}
 	return D_WALK_CONTINUE;
 }

include/linux/dcache.h +0 -1

index b4324d47f249..981f529c6cb5 100644
@@ -173,7 +173,6 @@ struct dentry_operations {
 #define DCACHE_DONTCACHE		BIT(7) /* Purge from memory on final dput() */
 
 #define DCACHE_CANT_MOUNT		BIT(8)
-#define DCACHE_GENOCIDE			BIT(9)
 #define DCACHE_SHRINK_LIST		BIT(10)
 
 #define DCACHE_OP_WEAK_REVALIDATE	BIT(11)

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #27 lore raw

PATCH 17/20 d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant

In reply to Al Viro

We only search in the damn thing under hlist_bl_lock(); RCU variant of insertion was, IIRC, pretty much cargo-culted - mea culpa...

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/dcache.c +1 -1

index 440b46851f52..51e2f777a2c5 100644
@@ -2690,7 +2690,7 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
 	/* we can't take ->d_lock here; it's OK, though. */
 	new->d_flags |= DCACHE_PAR_LOOKUP;
 	new->d_wait = wq;
-	hlist_bl_add_head_rcu(&new->d_u.d_in_lookup_hash, b);
+	hlist_bl_add_head(&new->d_u.d_in_lookup_hash, b);
 	hlist_bl_unlock(b);
 	return new;
 mismatch:

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #28 lore raw

PATCH 18/20 __d_unalias() doesn't use inode argument

In reply to Al Viro

... and hasn't since 2015.

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

fs/dcache.c +2 -3

index 51e2f777a2c5..b8f502a66e53 100644
@@ -3043,8 +3043,7 @@ struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2)
  * Note: If ever the locking in lock_rename() changes, then please
  * remember to update this too...
  */
-static int __d_unalias(struct inode *inode,
-		struct dentry *dentry, struct dentry *alias)
+static int __d_unalias(struct dentry *dentry, struct dentry *alias)
 {
 	struct mutex *m1 = NULL;
 	struct rw_semaphore *m2 = NULL;

@@ -3125,7 +3124,7 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
 					inode->i_sb->s_id);
 			} else if (!IS_ROOT(new)) {
 				struct dentry *old_parent = dget(new->d_parent);
-				int err = __d_unalias(inode, dentry, new);
+				int err = __d_unalias(dentry, new);
 				write_sequnlock(&rename_lock);
 				if (err) {
 					dput(new);

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #29 lore raw

PATCH 19/20 kill DCACHE_MAY_FREE

In reply to Al Viro

With the new ordering in __dentry_kill() it has become redundant - it's set if and only if both DCACHE_DENTRY_KILLED and DCACHE_SHRINK_LIST are set.

We set it in __dentry_kill(), after having set DCACHE_DENTRY_KILLED with the only condition being that DCACHE_SHRINK_LIST is there; all of that is done without dropping ->d_lock and the only place that checks that flag (shrink_dentry_list()) does so under ->d_lock, after having found the victim on its shrink list. Since DCACHE_SHRINK_LIST is set only when placing dentry into shrink list and removed only by shrink_dentry_list() itself, a check for DCACHE_DENTRY_KILLED in there would be equivalent to check for DCACHE_MAY_FREE.

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c            | 6 ++----
 include/linux/dcache.h | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

fs/dcache.c +2 -4

index 26080f094508..64d8c1d36acb 100644
@@ -614,10 +614,8 @@ static struct dentry *__dentry_kill(struct dentry *dentry)
 	}
 	spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
 	dentry_unlist(dentry);
-	if (dentry->d_flags & DCACHE_SHRINK_LIST) {
-		dentry->d_flags |= DCACHE_MAY_FREE;
+	if (dentry->d_flags & DCACHE_SHRINK_LIST)
 		can_free = false;
-	}
 	spin_unlock(&dentry->d_lock);
 	if (likely(can_free))
 		dentry_free(dentry);

@@ -1072,7 +1070,7 @@ void shrink_dentry_list(struct list_head *list)
 			bool can_free;
 			rcu_read_unlock();
 			d_shrink_del(dentry);
-			can_free = dentry->d_flags & DCACHE_MAY_FREE;
+			can_free = dentry->d_flags & DCACHE_DENTRY_KILLED;
 			spin_unlock(&dentry->d_lock);
 			if (can_free)
 				dentry_free(dentry);

include/linux/dcache.h +0 -1

index b4449a1a47ff..48b393545ec2 100644
@@ -202,7 +202,6 @@ struct dentry_operations {
 #define DCACHE_SPECIAL_TYPE		(5 << 20) /* Other file type */
 #define DCACHE_SYMLINK_TYPE		(6 << 20) /* Symlink */
 
-#define DCACHE_MAY_FREE			BIT(23)
 #define DCACHE_NOKEY_NAME		BIT(25) /* Encrypted name encoded without key */
 #define DCACHE_OP_REAL			BIT(26)

Signature · 1 line

2.39.2

Al Viro

author maintainer

2023-11-24 06:06 UTC #30 lore raw

PATCH 20/20 dcache: remove unnecessary NULL check in dget_dlock()

In reply to Al Viro

From: Vegard Nossum <[email protected]>

dget_dlock() requires dentry->d_lock to be held when called, yet contains a NULL check for dentry.

An audit of all calls to dget_dlock() shows that it is never called with a NULL pointer (as spin_lock()/spin_unlock() would crash in these cases):

  $ git grep -W '\<dget_dlock\>'

  arch/powerpc/platforms/cell/spufs/inode.c-              spin_lock(&dentry->d_lock);
  arch/powerpc/platforms/cell/spufs/inode.c-              if (simple_positive(dentry)) {
  arch/powerpc/platforms/cell/spufs/inode.c:                      dget_dlock(dentry);

  fs/autofs/expire.c-             spin_lock_nested(&child->d_lock, DENTRY_D_LOCK_NESTED);
  fs/autofs/expire.c-             if (simple_positive(child)) {
  fs/autofs/expire.c:                     dget_dlock(child);

  fs/autofs/root.c:                       dget_dlock(active);
  fs/autofs/root.c-                       spin_unlock(&active->d_lock);

  fs/autofs/root.c:                       dget_dlock(expiring);
  fs/autofs/root.c-                       spin_unlock(&expiring->d_lock);

  fs/ceph/dir.c-          if (!spin_trylock(&dentry->d_lock))
  fs/ceph/dir.c-                  continue;
  [...]
  fs/ceph/dir.c:                          dget_dlock(dentry);

  fs/ceph/mds_client.c-           spin_lock(&alias->d_lock);
  [...]
  fs/ceph/mds_client.c:                   dn = dget_dlock(alias);

  fs/configfs/inode.c-            spin_lock(&dentry->d_lock);
  fs/configfs/inode.c-            if (simple_positive(dentry)) {
  fs/configfs/inode.c:                    dget_dlock(dentry);

  fs/libfs.c:                             found = dget_dlock(d);
  fs/libfs.c-                     spin_unlock(&d->d_lock);

  fs/libfs.c:             found = dget_dlock(child);
  fs/libfs.c-     spin_unlock(&child->d_lock);

  fs/libfs.c:                             child = dget_dlock(d);
  fs/libfs.c-                     spin_unlock(&d->d_lock);

  fs/ocfs2/dcache.c:                      dget_dlock(dentry);
  fs/ocfs2/dcache.c-                      spin_unlock(&dentry->d_lock);

include/linux/dcache.h:static inline struct dentry *dget_dlock(struct dentry *dentry)

After taking out the NULL check, dget_dlock() becomes almost identical to __dget_dlock(); the only difference is that dget_dlock() returns the dentry that was passed in. These are static inline helpers, so we can rely on the compiler to discard unused return values. We can therefore also remove __dget_dlock() and replace calls to it by dget_dlock().

Also fix up and improve the kerneldoc comments while we're at it.

Al Viro pointed out that we can also clean up some of the callers to make use of the returned value and provided a bit more info for the kerneldoc.

While preparing v2 I also noticed that the tabs used in the kerneldoc comments were causing the kerneldoc to get parsed incorrectly so I also fixed this up (including for d_unhashed, which is otherwise unrelated).

Testing: x86 defconfig build + boot; make htmldocs for the kerneldoc warning. objdump shows there are code generation changes.

Link https://lore.kernel.org/all/[email protected]/ open here

Cc Alexander Viro <[email protected]>

Cc Christian Brauner <[email protected]>

Cc [email protected]

Cc Nick Piggin <[email protected]>

Cc Waiman Long <[email protected]>

Cc [email protected]

Signed-off-by Vegard Nossum <[email protected]>

Signed-off-by Al Viro <[email protected]>

---
 fs/dcache.c            | 16 ++++------------
 include/linux/dcache.h | 41 ++++++++++++++++++++++++++++++-----------
 2 files changed, 34 insertions(+), 23 deletions(-)

fs/dcache.c +4 ~~-12~~

index 64d8c1d36acb..e771977992ae 100644
@@ -877,12 +877,6 @@ void dput_to_list(struct dentry *dentry, struct list_head *list)
 	spin_unlock(&dentry->d_lock);
 }
 
-/* This must be called with d_lock held */
-static inline void __dget_dlock(struct dentry *dentry)
-{
-	dentry->d_lockref.count++;
-}
-
 struct dentry *dget_parent(struct dentry *dentry)
 {
 	int gotref;

@@ -964,7 +958,7 @@ static struct dentry *__d_find_alias(struct inode *inode)
 	hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
 		spin_lock(&alias->d_lock);
  		if (!d_unhashed(alias)) {
-			__dget_dlock(alias);
+			dget_dlock(alias);
 			spin_unlock(&alias->d_lock);
 			return alias;
 		}

@@ -1569,8 +1563,7 @@ static enum d_walk_ret find_submount(void *_data, struct dentry *dentry)
 {
 	struct dentry **victim = _data;
 	if (d_mountpoint(dentry)) {
-		__dget_dlock(dentry);
-		*victim = dentry;
+		*victim = dget_dlock(dentry);
 		return D_WALK_QUIT;
 	}
 	return D_WALK_CONTINUE;

@@ -1715,8 +1708,7 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name)
 	 * don't need child lock because it is not subject
 	 * to concurrency here
 	 */
-	__dget_dlock(parent);
-	dentry->d_parent = parent;
+	dentry->d_parent = dget_dlock(parent);
 	hlist_add_head(&dentry->d_sib, &parent->d_children);
 	spin_unlock(&parent->d_lock);

@@ -2681,7 +2673,7 @@ struct dentry *d_exact_alias(struct dentry *entry, struct inode *inode)
 			spin_unlock(&alias->d_lock);
 			alias = NULL;
 		} else {
-			__dget_dlock(alias);
+			dget_dlock(alias);
 			__d_rehash(alias);
 			spin_unlock(&alias->d_lock);
 		}

include/linux/dcache.h +30 ~~-11~~

62 lines not on this page · raw message

Signature · 1 line

2.39.2

Linus Torvalds

2023-11-24 21:41 UTC #31 lore raw

In reply to Al Viro

Quoted 2 lines · Al Viro

Assorted dcache cleanups.

Looks obvious enough to me.

Famous last words.

Linus