All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHES] assorted dcache stuff
@ 2023-11-24  6:05 Al Viro
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
  2023-11-24 21:41 ` [PATCHES] assorted dcache stuff Linus Torvalds
  0 siblings, 2 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:05 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

	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().

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 01/20] selinux: saner handling of policy reloads
  2023-11-24  6:05 [PATCHES] assorted dcache stuff Al Viro
@ 2023-11-24  6:06 ` Al Viro
  2023-11-24  6:06   ` [PATCH 02/20] ovl: stop using d_alloc_anon()/d_instantiate_anon() Al Viro
                     ` (18 more replies)
  2023-11-24 21:41 ` [PATCHES] assorted dcache stuff Linus Torvalds
  1 sibling, 19 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <stephen.smalley.work@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[PM: applied some fixes from Al post merge]
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 security/selinux/selinuxfs.c | 144 ++++++++++++++++-------------------
 1 file changed, 66 insertions(+), 78 deletions(-)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 6c596ae7fef9..0619a1cbbfbe 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -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"
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 02/20] ovl: stop using d_alloc_anon()/d_instantiate_anon()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 03/20] struct dentry: get rid of randomize_layout idiocy Al Viro
                     ` (17 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

From: Amir Goldstein <amir73il@gmail.com>

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 <viro@zeniv.linux.org.uk>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/overlayfs/export.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 7e16bbcad95e..9e316d5f936e 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -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 */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/20] struct dentry: get rid of randomize_layout idiocy
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
  2023-11-24  6:06   ` [PATCH 02/20] ovl: stop using d_alloc_anon()/d_instantiate_anon() Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 04/20] get rid of __dget() Al Viro
                     ` (16 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/dcache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 3da2f0545d5d..1d9f7f132055 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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:
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 04/20] get rid of __dget()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
  2023-11-24  6:06   ` [PATCH 02/20] ovl: stop using d_alloc_anon()/d_instantiate_anon() Al Viro
  2023-11-24  6:06   ` [PATCH 03/20] struct dentry: get rid of randomize_layout idiocy Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 05/20] DCACHE_... ->d_flags bits: switch to BIT() Al Viro
                     ` (15 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

fold into the sole remaining caller

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index c82ae731df9a..b8f1b54a1492 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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;
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 05/20] DCACHE_... ->d_flags bits: switch to BIT()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (2 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 04/20] get rid of __dget() Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 06/20] DCACHE_COOKIE: RIP Al Viro
                     ` (14 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 include/linux/dcache.h | 76 +++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 1d9f7f132055..d9c314cc93b8 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/20] DCACHE_COOKIE: RIP
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (3 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 05/20] DCACHE_... ->d_flags bits: switch to BIT() Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 07/20] kill d_{is,set}_fallthru() Al Viro
                     ` (13 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

the last user gone in 2021...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/dcache.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index d9c314cc93b8..92c0b2a1ae2e 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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 */
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 07/20] kill d_{is,set}_fallthru()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (4 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 06/20] DCACHE_COOKIE: RIP Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 08/20] dentry.h: trim externs Al Viro
                     ` (12 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c            | 20 ++------------------
 include/linux/dcache.h | 17 ++++-------------
 2 files changed, 6 insertions(+), 31 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index b8f1b54a1492..9f5b2b5c1e6d 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 92c0b2a1ae2e..8cd937bb2292 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 08/20] dentry.h: trim externs
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (5 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 07/20] kill d_{is,set}_fallthru() Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment Al Viro
                     ` (11 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 fs/internal.h          | 4 ++++
 include/linux/dcache.h | 5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 58e43341aebf..9e9fc629f935 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -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
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 8cd937bb2292..9706bf1dc5de 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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)
 {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (6 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 08/20] dentry.h: trim externs Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  7:37     ` Amir Goldstein
  2023-11-24  6:06   ` [PATCH 10/20] kill d_backing_dentry() Al Viro
                     ` (10 subsequent siblings)
  18 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 include/linux/dcache.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 9706bf1dc5de..a5e5e274eee0 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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 *);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 10/20] kill d_backing_dentry()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (7 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 11/20] kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller Al Viro
                     ` (9 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

no users left

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/dcache.h | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index a5e5e274eee0..fa0414cff85c 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 11/20] kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (8 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 10/20] kill d_backing_dentry() Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 12/20] d_alloc_pseudo(): move setting ->d_op there from the (sole) caller Al Viro
                     ` (8 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c            | 88 ++++++++++++++++--------------------------
 include/linux/dcache.h |  1 -
 2 files changed, 33 insertions(+), 56 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 9f5b2b5c1e6d..6dde21dbdd1a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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;
 }
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index fa0414cff85c..8c5e3bdf1147 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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 *);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 12/20] d_alloc_pseudo(): move setting ->d_op there from the (sole) caller
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (9 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 11/20] kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 13/20] nsfs: use d_make_root() Al Viro
                     ` (7 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c     | 8 +++++++-
 fs/file_table.c | 5 -----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 6dde21dbdd1a..1296a3fa4f93 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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;
 }
 
diff --git a/fs/file_table.c b/fs/file_table.c
index de4a2915bfd4..8889cbee13f8 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -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);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 13/20] nsfs: use d_make_root()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (10 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 12/20] d_alloc_pseudo(): move setting ->d_op there from the (sole) caller Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 14/20] simple_fill_super(): don't bother with d_genocide() on failure Al Viro
                     ` (6 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 fs/nsfs.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/nsfs.c b/fs/nsfs.c
index 9a4b228d42fa..34e1e3e36733 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -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) {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 14/20] simple_fill_super(): don't bother with d_genocide() on failure
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (11 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 13/20] nsfs: use d_make_root() Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 15/20] d_genocide(): move the extern into fs/internal.h Al Viro
                     ` (5 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 fs/libfs.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index e9440d55073c..6fa8ad36049f 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -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);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 15/20] d_genocide(): move the extern into fs/internal.h
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (12 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 14/20] simple_fill_super(): don't bother with d_genocide() on failure Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:35     ` d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? " Cedric Blancher
  2023-11-24  6:06   ` [PATCH 16/20] get rid of DCACHE_GENOCIDE Al Viro
                     ` (4 subsequent siblings)
  18 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h          | 1 +
 include/linux/dcache.h | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 9e9fc629f935..d9a920e2636e 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -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
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 8c5e3bdf1147..b4324d47f249 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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 *);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 16/20] get rid of DCACHE_GENOCIDE
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (13 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 15/20] d_genocide(): move the extern into fs/internal.h Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 17/20] d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant Al Viro
                     ` (3 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c            | 5 +----
 include/linux/dcache.h | 1 -
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1296a3fa4f93..440b46851f52 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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;
 }
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index b4324d47f249..981f529c6cb5 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 17/20] d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (14 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 16/20] get rid of DCACHE_GENOCIDE Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 18/20] __d_unalias() doesn't use inode argument Al Viro
                     ` (2 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 440b46851f52..51e2f777a2c5 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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:
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 18/20] __d_unalias() doesn't use inode argument
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (15 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 17/20] d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 19/20] kill DCACHE_MAY_FREE Al Viro
  2023-11-24  6:06   ` [PATCH 20/20] dcache: remove unnecessary NULL check in dget_dlock() Al Viro
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

... and hasn't since 2015.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 51e2f777a2c5..b8f502a66e53 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 19/20] kill DCACHE_MAY_FREE
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (16 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 18/20] __d_unalias() doesn't use inode argument Al Viro
@ 2023-11-24  6:06   ` Al Viro
  2023-11-24  6:06   ` [PATCH 20/20] dcache: remove unnecessary NULL check in dget_dlock() Al Viro
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

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 <viro@zeniv.linux.org.uk>
---
 fs/dcache.c            | 6 ++----
 include/linux/dcache.h | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 26080f094508..64d8c1d36acb 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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);
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index b4449a1a47ff..48b393545ec2 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -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)
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 20/20] dcache: remove unnecessary NULL check in dget_dlock()
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
                     ` (17 preceding siblings ...)
  2023-11-24  6:06   ` [PATCH 19/20] kill DCACHE_MAY_FREE Al Viro
@ 2023-11-24  6:06   ` Al Viro
  18 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:06 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Linus Torvalds, Christian Brauner, linux-kernel

From: Vegard Nossum <vegard.nossum@oracle.com>

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/20231022164520.915013-1-vegard.nossum@oracle.com/
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: linux-fsdevel@vger.kernel.org
Cc: Nick Piggin <npiggin@kernel.dk>
Cc: Waiman Long <Waiman.Long@hp.com>
Cc: linux-doc@vger.kernel.org
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c            | 16 ++++------------
 include/linux/dcache.h | 41 ++++++++++++++++++++++++++++++-----------
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 64d8c1d36acb..e771977992ae 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -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);
 		}
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 48b393545ec2..1666c387861f 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -287,20 +287,40 @@ extern char *dentry_path(const struct dentry *, char *, int);
 /* Allocation counts.. */
 
 /**
- *	dget, dget_dlock -	get a reference to a dentry
- *	@dentry: dentry to get a reference to
+ * dget_dlock -	get a reference to a dentry
+ * @dentry: dentry to get a reference to
  *
- *	Given a dentry or %NULL pointer increment the reference count
- *	if appropriate and return the dentry. A dentry will not be 
- *	destroyed when it has references.
+ * Given a live dentry, increment the reference count and return the dentry.
+ * Caller must hold @dentry->d_lock.  Making sure that dentry is alive is
+ * caller's resonsibility.  There are many conditions sufficient to guarantee
+ * that; e.g. anything with non-negative refcount is alive, so's anything
+ * hashed, anything positive, anyone's parent, etc.
  */
 static inline struct dentry *dget_dlock(struct dentry *dentry)
 {
-	if (dentry)
-		dentry->d_lockref.count++;
+	dentry->d_lockref.count++;
 	return dentry;
 }
 
+
+/**
+ * dget - get a reference to a dentry
+ * @dentry: dentry to get a reference to
+ *
+ * Given a dentry or %NULL pointer increment the reference count
+ * if appropriate and return the dentry.  A dentry will not be
+ * destroyed when it has references.  Conversely, a dentry with
+ * no references can disappear for any number of reasons, starting
+ * with memory pressure.  In other words, that primitive is
+ * used to clone an existing reference; using it on something with
+ * zero refcount is a bug.
+ *
+ * NOTE: it will spin if @dentry->d_lock is held.  From the deadlock
+ * avoidance point of view it is equivalent to spin_lock()/increment
+ * refcount/spin_unlock(), so calling it under @dentry->d_lock is
+ * always a bug; so's calling it under ->d_lock on any of its descendents.
+ *
+ */
 static inline struct dentry *dget(struct dentry *dentry)
 {
 	if (dentry)
@@ -311,12 +331,11 @@ static inline struct dentry *dget(struct dentry *dentry)
 extern struct dentry *dget_parent(struct dentry *dentry);
 
 /**
- *	d_unhashed -	is dentry hashed
- *	@dentry: entry to check
+ * d_unhashed - is dentry hashed
+ * @dentry: entry to check
  *
- *	Returns true if the dentry passed is not currently hashed.
+ * Returns true if the dentry passed is not currently hashed.
  */
- 
 static inline int d_unhashed(const struct dentry *dentry)
 {
 	return hlist_bl_unhashed(&dentry->d_hash);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? Re: [PATCH 15/20] d_genocide(): move the extern into fs/internal.h
  2023-11-24  6:06   ` [PATCH 15/20] d_genocide(): move the extern into fs/internal.h Al Viro
@ 2023-11-24  6:35     ` Cedric Blancher
  2023-11-24  6:57       ` Al Viro
  0 siblings, 1 reply; 31+ messages in thread
From: Cedric Blancher @ 2023-11-24  6:35 UTC (permalink / raw)
  To: linux-fsdevel, Linux Kernel Mailing List

On Fri, 24 Nov 2023 at 07:08, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/internal.h          | 1 +
>  include/linux/dcache.h | 3 ---
>  2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/internal.h b/fs/internal.h
> index 9e9fc629f935..d9a920e2636e 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -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

>
>  /*
>   * pipe.c
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 8c5e3bdf1147..b4324d47f249 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -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 *);
>
> --
> 2.39.2
>
>


--
Cedric Blancher <cedric.blancher@gmail.com>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur

^ permalink raw reply	[flat|nested] 31+ messages in thread

* 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
  2023-11-24  6:35     ` d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? " Cedric Blancher
@ 2023-11-24  6:57       ` Al Viro
  2023-11-24  7:48         ` Al Viro
  0 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2023-11-24  6:57 UTC (permalink / raw)
  To: Cedric Blancher; +Cc: linux-fsdevel, Linux Kernel Mailing List

On Fri, Nov 24, 2023 at 07:35:34AM +0100, Cedric Blancher wrote:
> On Fri, 24 Nov 2023 at 07:08, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
> >  fs/internal.h          | 1 +
> >  include/linux/dcache.h | 3 ---
> >  2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/fs/internal.h b/fs/internal.h
> > index 9e9fc629f935..d9a920e2636e 100644
> > --- a/fs/internal.h
> > +++ b/fs/internal.h
> > @@ -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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment
  2023-11-24  6:06   ` [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment Al Viro
@ 2023-11-24  7:37     ` Amir Goldstein
  2023-11-24  8:11       ` Al Viro
  0 siblings, 1 reply; 31+ messages in thread
From: Amir Goldstein @ 2023-11-24  7:37 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Linus Torvalds, Christian Brauner, linux-kernel

On Fri, Nov 24, 2023 at 8:07 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> 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 <viro@zeniv.linux.org.uk>
> ---
>  include/linux/dcache.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 9706bf1dc5de..a5e5e274eee0 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* 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
  2023-11-24  6:57       ` Al Viro
@ 2023-11-24  7:48         ` Al Viro
  2023-11-24  9:36           ` Martin Steigerwald
  0 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2023-11-24  7:48 UTC (permalink / raw)
  To: Cedric Blancher; +Cc: linux-fsdevel, Linux Kernel Mailing List

On Fri, Nov 24, 2023 at 06:57:59AM +0000, Al Viro wrote:
> > > +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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment
  2023-11-24  7:37     ` Amir Goldstein
@ 2023-11-24  8:11       ` Al Viro
  2023-11-24  8:26         ` Al Viro
  0 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2023-11-24  8:11 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: linux-fsdevel, Linus Torvalds, Christian Brauner, linux-kernel

On Fri, Nov 24, 2023 at 09:37:16AM +0200, Amir Goldstein wrote:

> 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.

> 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 ;-)

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment
  2023-11-24  8:11       ` Al Viro
@ 2023-11-24  8:26         ` Al Viro
  2023-11-24  8:29           ` Christian Brauner
  0 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2023-11-24  8:26 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: linux-fsdevel, Linus Torvalds, Christian Brauner, linux-kernel

On Fri, Nov 24, 2023 at 08:11:41AM +0000, Al Viro wrote:
> On Fri, Nov 24, 2023 at 09:37:16AM +0200, Amir Goldstein wrote:
> 
> > 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.
> 
> > 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/

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment
  2023-11-24  8:26         ` Al Viro
@ 2023-11-24  8:29           ` Christian Brauner
  0 siblings, 0 replies; 31+ messages in thread
From: Christian Brauner @ 2023-11-24  8:29 UTC (permalink / raw)
  To: Al Viro; +Cc: Amir Goldstein, linux-fsdevel, Linus Torvalds, linux-kernel

> 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...

^ permalink raw reply	[flat|nested] 31+ messages in thread

* 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
  2023-11-24  7:48         ` Al Viro
@ 2023-11-24  9:36           ` Martin Steigerwald
  2023-11-24 18:21             ` identifiers Al Viro
  0 siblings, 1 reply; 31+ messages in thread
From: Martin Steigerwald @ 2023-11-24  9:36 UTC (permalink / raw)
  To: Cedric Blancher, Al Viro; +Cc: linux-fsdevel, Linux Kernel Mailing List

Al Viro - 24.11.23, 08:48:57 CET:
> On Fri, Nov 24, 2023 at 06:57:59AM +0000, Al Viro wrote:
> > > > +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 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?

-- 
Martin



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: identifiers
  2023-11-24  9:36           ` Martin Steigerwald
@ 2023-11-24 18:21             ` Al Viro
  0 siblings, 0 replies; 31+ messages in thread
From: Al Viro @ 2023-11-24 18:21 UTC (permalink / raw)
  To: Martin Steigerwald
  Cc: Cedric Blancher, linux-fsdevel, Linux Kernel Mailing List

[search bait removed from subject]
On Fri, Nov 24, 2023 at 10:36:05AM +0100, Martin Steigerwald wrote:
> Al Viro - 24.11.23, 08:48:57 CET:
> > 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.

> 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.

> 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.

> 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.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCHES] assorted dcache stuff
  2023-11-24  6:05 [PATCHES] assorted dcache stuff Al Viro
  2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
@ 2023-11-24 21:41 ` Linus Torvalds
  1 sibling, 0 replies; 31+ messages in thread
From: Linus Torvalds @ 2023-11-24 21:41 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, Christian Brauner, linux-kernel

On Thu, 23 Nov 2023 at 22:05, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>         Assorted dcache cleanups.

Looks obvious enough to me.

Famous last words.

                  Linus

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2023-11-24 21:42 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-24  6:05 [PATCHES] assorted dcache stuff Al Viro
2023-11-24  6:06 ` [PATCH 01/20] selinux: saner handling of policy reloads Al Viro
2023-11-24  6:06   ` [PATCH 02/20] ovl: stop using d_alloc_anon()/d_instantiate_anon() Al Viro
2023-11-24  6:06   ` [PATCH 03/20] struct dentry: get rid of randomize_layout idiocy Al Viro
2023-11-24  6:06   ` [PATCH 04/20] get rid of __dget() Al Viro
2023-11-24  6:06   ` [PATCH 05/20] DCACHE_... ->d_flags bits: switch to BIT() Al Viro
2023-11-24  6:06   ` [PATCH 06/20] DCACHE_COOKIE: RIP Al Viro
2023-11-24  6:06   ` [PATCH 07/20] kill d_{is,set}_fallthru() Al Viro
2023-11-24  6:06   ` [PATCH 08/20] dentry.h: trim externs Al Viro
2023-11-24  6:06   ` [PATCH 09/20] [software coproarchaeology] dentry.h: kill a mysterious comment Al Viro
2023-11-24  7:37     ` Amir Goldstein
2023-11-24  8:11       ` Al Viro
2023-11-24  8:26         ` Al Viro
2023-11-24  8:29           ` Christian Brauner
2023-11-24  6:06   ` [PATCH 10/20] kill d_backing_dentry() Al Viro
2023-11-24  6:06   ` [PATCH 11/20] kill d_instantate_anon(), fold __d_instantiate_anon() into remaining caller Al Viro
2023-11-24  6:06   ` [PATCH 12/20] d_alloc_pseudo(): move setting ->d_op there from the (sole) caller Al Viro
2023-11-24  6:06   ` [PATCH 13/20] nsfs: use d_make_root() Al Viro
2023-11-24  6:06   ` [PATCH 14/20] simple_fill_super(): don't bother with d_genocide() on failure Al Viro
2023-11-24  6:06   ` [PATCH 15/20] d_genocide(): move the extern into fs/internal.h Al Viro
2023-11-24  6:35     ` d_genocide()? What about d_holodomor(), d_massmurder(), d_execute_warcrimes()? " Cedric Blancher
2023-11-24  6:57       ` Al Viro
2023-11-24  7:48         ` Al Viro
2023-11-24  9:36           ` Martin Steigerwald
2023-11-24 18:21             ` identifiers Al Viro
2023-11-24  6:06   ` [PATCH 16/20] get rid of DCACHE_GENOCIDE Al Viro
2023-11-24  6:06   ` [PATCH 17/20] d_alloc_parallel(): in-lookup hash insertion doesn't need an RCU variant Al Viro
2023-11-24  6:06   ` [PATCH 18/20] __d_unalias() doesn't use inode argument Al Viro
2023-11-24  6:06   ` [PATCH 19/20] kill DCACHE_MAY_FREE Al Viro
2023-11-24  6:06   ` [PATCH 20/20] dcache: remove unnecessary NULL check in dget_dlock() Al Viro
2023-11-24 21:41 ` [PATCHES] assorted dcache stuff Linus Torvalds

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.