[PATCHES] assorted fs cleanups

[PATCHES] assorted fs cleanups

[Al Viro]

	Assorted cleanups in various filesystems.  Currently
that pile sits in vfs.git #work.misc; if anyone wants to pick
these into relevant filesystem tree, I'll be glad to drop those
from the queue...

Diffstat:
 fs/affs/namei.c                |  3 ---
 fs/befs/linuxvfs.c             |  3 ---
 fs/bfs/dir.c                   |  5 -----
 fs/ceph/dir.c                  | 21 +++++++++++++--------
 fs/ceph/export.c               |  2 --
 fs/ext4/namei.c                |  2 --
 fs/gfs2/export.c               |  2 --
 fs/gfs2/super.c                | 12 +-----------
 fs/hostfs/hostfs_kern.c        |  8 ++------
 fs/kernfs/mount.c              |  3 ---
 fs/nfsd/nfsctl.c               |  4 ----
 fs/nilfs2/namei.c              |  7 +------
 fs/ocfs2/dcache.c              |  7 -------
 fs/ocfs2/dir.c                 |  9 ---------
 fs/orangefs/dir.c              | 32 ++++++++++++--------------------
 fs/proc/proc_sysctl.c          | 14 ++------------
 fs/reiserfs/namei.c            |  7 -------
 fs/udf/namei.c                 | 11 +----------
 fs/zonefs/super.c              |  2 --
 security/apparmor/apparmorfs.c |  7 +------
 20 files changed, 33 insertions(+), 128 deletions(-)

Shortlog:
Al Viro (22):
      hostfs: use d_splice_alias() calling conventions to simplify failure exits
      /proc/sys: use d_splice_alias() calling conventions to simplify failure exits
      zonefs: d_splice_alias() will do the right thing on ERR_PTR() inode
      udf: d_splice_alias() will do the right thing on ERR_PTR() inode
      affs: d_obtain_alias(ERR_PTR(...)) will do the right thing
      befs: d_obtain_alias(ERR_PTR(...)) will do the right thing
      ceph: d_obtain_{alias,root}(ERR_PTR(...)) will do the right thing
      gfs2: d_obtain_alias(ERR_PTR(...)) will do the right thing
      kernfs: d_obtain_alias(NULL) will do the right thing...
      nilfs2: d_obtain_alias(ERR_PTR(...)) will do the right thing...
      udf: d_obtain_alias(ERR_PTR(...)) will do the right thing...
      ext4_add_entry(): ->d_name.len is never 0
      bfs_add_entry(): get rid of pointless ->d_name.len checks
      __ocfs2_add_entry(), ocfs2_prepare_dir_for_insert()
      reiserfs_add_entry(): get rid of pointless namelen checks
      udf_fiiter_add_entry(): check for zero ->d_name.len is bogus...
      get rid of passing callbacks to ceph __dentry_leases_walk()
      nfsd: kill stale comment about simple_fill_super() requirements
      ocfs2_find_match(): there's no such thing as NULL or negative ->d_parent
      gfs2: use is_subdir()
      orangefs: saner arguments passing in readdir guts
      apparmorfs: don't duplicate kfree_link()

First group (1--11) is about making use of d_splice_alias() and d_obtain_alias()
calling conventions; a bunch of failure exits are pointless, since the primitive
will do the right thing.  Then (12--16) killing impossible checks for dentry
name length (never 0, never exceeds what ->lookup() would reject with ENAMETOOLONG).
The rest is really without any common theme...

As far as I can tell, nothing in there has conflicts in linux-next.
Individual patches in followups.  Please, review; again, if anything of
that gets picked by relevant filesystem tree, I'll gladly drop it from
this branch.

PATCH 01/22] hostfs: use d_splice_alias() calling conventions to simplify failure exits

[Al Viro]

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

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index ea87f24c6c3f..a73d27c4dd58 100644
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -637,12 +637,8 @@ static struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry,
 
 	inode = hostfs_iget(ino->i_sb, name);
 	__putname(name);
-	if (IS_ERR(inode)) {
-		if (PTR_ERR(inode) == -ENOENT)
-			inode = NULL;
-		else
-			return ERR_CAST(inode);
-	}
+	if (inode == ERR_PTR(-ENOENT))
+		inode = NULL;
 
 	return d_splice_alias(inode, dentry);
 }
-- 
2.39.2

[PATCH 02/22] /proc/sys: use d_splice_alias() calling conventions to simplify failure exits

[Al Viro]

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

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 8064ea76f80b..1ae6486dc7d4 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -534,13 +534,8 @@ static struct dentry *proc_sys_lookup(struct inode *dir, struct dentry *dentry,
 			goto out;
 	}
 
-	inode = proc_sys_make_inode(dir->i_sb, h ? h : head, p);
-	if (IS_ERR(inode)) {
-		err = ERR_CAST(inode);
-		goto out;
-	}
-
 	d_set_d_op(dentry, &proc_sys_dentry_operations);
+	inode = proc_sys_make_inode(dir->i_sb, h ? h : head, p);
 	err = d_splice_alias(inode, dentry);
 
 out:
@@ -698,13 +693,8 @@ static bool proc_sys_fill_cache(struct file *file,
 			return false;
 		if (d_in_lookup(child)) {
 			struct dentry *res;
-			inode = proc_sys_make_inode(dir->d_sb, head, table);
-			if (IS_ERR(inode)) {
-				d_lookup_done(child);
-				dput(child);
-				return false;
-			}
 			d_set_d_op(child, &proc_sys_dentry_operations);
+			inode = proc_sys_make_inode(dir->d_sb, head, table);
 			res = d_splice_alias(inode, child);
 			d_lookup_done(child);
 			if (unlikely(res)) {
-- 
2.39.2

[PATCH 03/22] zonefs: d_splice_alias() will do the right thing on ERR_PTR() inode

[Al Viro]

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

diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index e6a75401677d..93971742613a 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -747,8 +747,6 @@ static struct dentry *zonefs_lookup(struct inode *dir, struct dentry *dentry,
 		inode = zonefs_get_dir_inode(dir, dentry);
 	else
 		inode = zonefs_get_file_inode(dir, dentry);
-	if (IS_ERR(inode))
-		return ERR_CAST(inode);
 
 	return d_splice_alias(inode, dentry);
 }
-- 
2.39.2

[PATCH 04/22] udf: d_splice_alias() will do the right thing on ERR_PTR() inode

[Al Viro]

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

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 3508ac484da3..92f25e540430 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -125,8 +125,6 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
 		udf_fiiter_release(&iter);
 
 		inode = udf_iget(dir->i_sb, &loc);
-		if (IS_ERR(inode))
-			return ERR_CAST(inode);
 	}
 
 	return d_splice_alias(inode, dentry);
-- 
2.39.2

[PATCH 05/22] affs: d_obtain_alias(ERR_PTR(...)) will do the right thing

[Al Viro]

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

diff --git a/fs/affs/namei.c b/fs/affs/namei.c
index d6b9758ee23d..8c154490a2d6 100644
--- a/fs/affs/namei.c
+++ b/fs/affs/namei.c
@@ -532,9 +532,6 @@ static struct dentry *affs_get_parent(struct dentry *child)
 	parent = affs_iget(child->d_sb,
 			   be32_to_cpu(AFFS_TAIL(child->d_sb, bh)->parent));
 	brelse(bh);
-	if (IS_ERR(parent))
-		return ERR_CAST(parent);
-
 	return d_obtain_alias(parent);
 }
 
-- 
2.39.2

[PATCH 06/22] befs: d_obtain_alias(ERR_PTR(...)) will do the right thing

[Al Viro]

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

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index a93d76df8ed8..2b4dda047450 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -671,9 +671,6 @@ static struct dentry *befs_get_parent(struct dentry *child)
 
 	parent = befs_iget(child->d_sb,
 			   (unsigned long)befs_ino->i_parent.start);
-	if (IS_ERR(parent))
-		return ERR_CAST(parent);
-
 	return d_obtain_alias(parent);
 }
 
-- 
2.39.2

[PATCH 07/22] ceph: d_obtain_{alias,root}(ERR_PTR(...)) will do the right thing

[Al Viro]

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

diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index 726af69d4d62..a79f163ae4ed 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -286,8 +286,6 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
 		doutc(cl, "%llx.%llx parent %llx hash %x err=%d", vino.ino,
 		      vino.snap, sfh->parent_ino, sfh->hash, err);
 	}
-	if (IS_ERR(inode))
-		return ERR_CAST(inode);
 	/* see comments in ceph_get_parent() */
 	return unlinked ? d_obtain_root(inode) : d_obtain_alias(inode);
 }
-- 
2.39.2

[PATCH 08/22] gfs2: d_obtain_alias(ERR_PTR(...)) will do the right thing

[Al Viro]

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

diff --git a/fs/gfs2/export.c b/fs/gfs2/export.c
index cf40895233f5..3334c394ce9c 100644
--- a/fs/gfs2/export.c
+++ b/fs/gfs2/export.c
@@ -138,8 +138,6 @@ static struct dentry *gfs2_get_dentry(struct super_block *sb,
 		return ERR_PTR(-ESTALE);
 	inode = gfs2_lookup_by_inum(sdp, inum->no_addr, inum->no_formal_ino,
 				    GFS2_BLKST_DINODE);
-	if (IS_ERR(inode))
-		return ERR_CAST(inode);
 	return d_obtain_alias(inode);
 }
 
-- 
2.39.2

[PATCH 09/22] kernfs: d_obtain_alias(NULL) will do the right thing...

[Al Viro]

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

diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index 4628edde2e7e..0c93cad0f0ac 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -125,9 +125,6 @@ static struct dentry *__kernfs_fh_to_dentry(struct super_block *sb,
 
 	inode = kernfs_get_inode(sb, kn);
 	kernfs_put(kn);
-	if (!inode)
-		return ERR_PTR(-ESTALE);
-
 	return d_obtain_alias(inode);
 }
 
-- 
2.39.2

nilfs2: d_obtain_alias(ERR_PTR(...)) will do the right thing...

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/nilfs2/namei.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 2a4e7f4a8102..8e8c7c981a7a 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -439,7 +439,6 @@ static int nilfs_rename(struct mnt_idmap *idmap,
 static struct dentry *nilfs_get_parent(struct dentry *child)
 {
 	unsigned long ino;
-	struct inode *inode;
 	struct nilfs_root *root;
 
 	ino = nilfs_inode_by_name(d_inode(child), &dotdot_name);
@@ -448,11 +447,7 @@ static struct dentry *nilfs_get_parent(struct dentry *child)
 
 	root = NILFS_I(d_inode(child))->i_root;
 
-	inode = nilfs_iget(child->d_sb, root, ino);
-	if (IS_ERR(inode))
-		return ERR_CAST(inode);
-
-	return d_obtain_alias(inode);
+	return d_obtain_alias(nilfs_iget(child->d_sb, root, ino));
 }
 
 static struct dentry *nilfs_get_dentry(struct super_block *sb, u64 cno,
-- 
2.39.2

[PATCH 11/22] udf: d_obtain_alias(ERR_PTR(...)) will do the right thing...

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/udf/namei.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 92f25e540430..a64102d63781 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -897,7 +897,6 @@ static int udf_rename(struct mnt_idmap *idmap, struct inode *old_dir,
 static struct dentry *udf_get_parent(struct dentry *child)
 {
 	struct kernel_lb_addr tloc;
-	struct inode *inode = NULL;
 	struct udf_fileident_iter iter;
 	int err;
 
@@ -907,11 +906,7 @@ static struct dentry *udf_get_parent(struct dentry *child)
 
 	tloc = lelb_to_cpu(iter.fi.icb.extLocation);
 	udf_fiiter_release(&iter);
-	inode = udf_iget(child->d_sb, &tloc);
-	if (IS_ERR(inode))
-		return ERR_CAST(inode);
-
-	return d_obtain_alias(inode);
+	return d_obtain_alias(udf_iget(child->d_sb, &tloc));
 }
 
 
-- 
2.39.2

[PATCH 12/22] ext4_add_entry(): ->d_name.len is never 0

[Al Viro]

That bogosity goes back to the initial merge of ext3.  Once upon a time
ext2 used to have a similar check; that got taken out during the switch
to page cache (June 2001).  ext3 got merged into mainline 5 months later,
still using buffer cache for directories; removal of the pointless check
in ext2 should've been done as a separate patch, but it hadn't been,
so that thing got missed...

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

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index d252935f9c8a..fa8b8dd841b5 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2388,8 +2388,6 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
 
 	sb = dir->i_sb;
 	blocksize = sb->s_blocksize;
-	if (!dentry->d_name.len)
-		return -EINVAL;
 
 	if (fscrypt_is_nokey_name(dentry))
 		return -ENOKEY;
-- 
2.39.2

[PATCH 13/22] bfs_add_entry(): get rid of pointless ->d_name.len checks

[Al Viro]

First of all, any dentry getting here would have passed bfs_lookup(),
so it it passed ENAMETOOLONG check there, there's no need to
repeat it.  And we are not going to get dentries with zero name length -
that check ultimately comes from ext2 and it's as pointless here as it
used to be there.

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

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index fbc4ae80a4b2..c375e22c4c0c 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -275,11 +275,6 @@ static int bfs_add_entry(struct inode *dir, const struct qstr *child, int ino)
 
 	dprintf("name=%s, namelen=%d\n", name, namelen);
 
-	if (!namelen)
-		return -ENOENT;
-	if (namelen > BFS_NAMELEN)
-		return -ENAMETOOLONG;
-
 	sblock = BFS_I(dir)->i_sblock;
 	eblock = BFS_I(dir)->i_eblock;
 	for (block = sblock; block <= eblock; block++) {
-- 
2.39.2

[PATCH 14/22] __ocfs2_add_entry(), ocfs2_prepare_dir_for_insert(): namelen checks

[Al Viro]

namelen can't be zero; neither when it's coming from dentry name,
nor when dealing with orphans (in ocfs2_orphan_add() and
__ocfs2_prepare_orphan_dir()).  Rudiment of old ext2 pointless
check, long gone in ext2 itself...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/ocfs2/dir.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index a14c8fee6ee5..d620d4c53c6f 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1593,9 +1593,6 @@ int __ocfs2_add_entry(handle_t *handle,
 	struct buffer_head *insert_bh = lookup->dl_leaf_bh;
 	char *data_start = insert_bh->b_data;
 
-	if (!namelen)
-		return -EINVAL;
-
 	if (ocfs2_dir_indexed(dir)) {
 		struct buffer_head *bh;
 
@@ -4245,12 +4242,6 @@ int ocfs2_prepare_dir_for_insert(struct ocfs2_super *osb,
 	trace_ocfs2_prepare_dir_for_insert(
 		(unsigned long long)OCFS2_I(dir)->ip_blkno, namelen);
 
-	if (!namelen) {
-		ret = -EINVAL;
-		mlog_errno(ret);
-		goto out;
-	}
-
 	/*
 	 * Do this up front to reduce confusion.
 	 *
-- 
2.39.2

[PATCH 15/22] reiserfs_add_entry(): get rid of pointless namelen checks

[Al Viro]

In all cases namelen is ->d_name.len of some dentry; moreover, a dentry
that has passed ->lookup() without triggering ENAMETOOLONG check there.
The comment next to these checks is either a rudiment of some other
check that used to be there once upon a time, or an attempt to come up
with the possible reason for that check (well, more like "why does
ext3 do it?")

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/reiserfs/namei.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 994d6e6995ab..c5f233b4a27f 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -451,13 +451,6 @@ static int reiserfs_add_entry(struct reiserfs_transaction_handle *th,
 
 	BUG_ON(!th->t_trans_id);
 
-	/* cannot allow items to be added into a busy deleted directory */
-	if (!namelen)
-		return -EINVAL;
-
-	if (namelen > REISERFS_MAX_NAME(dir->i_sb->s_blocksize))
-		return -ENAMETOOLONG;
-
 	/* each entry has unique key. compose it */
 	make_cpu_key(&entry_key, dir,
 		     get_third_component(dir->i_sb, name, namelen),
-- 
2.39.2

[PATCH 16/22] udf_fiiter_add_entry(): check for zero ->d_name.len is bogus...

[Al Viro]

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

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index a64102d63781..b1674e07d5a5 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -228,8 +228,6 @@ static int udf_fiiter_add_entry(struct inode *dir, struct dentry *dentry,
 	char name[UDF_NAME_LEN_CS0];
 
 	if (dentry) {
-		if (!dentry->d_name.len)
-			return -EINVAL;
 		namelen = udf_put_filename(dir->i_sb, dentry->d_name.name,
 					   dentry->d_name.len,
 					   name, UDF_NAME_LEN_CS0);
-- 
2.39.2

[PATCH 17/22] get rid of passing callbacks to ceph __dentry_leases_walk()

[Al Viro]

__dentry_leases_walk() is gets a callback and calls it for
a bunch of denties; there are exactly two callers and
we already have a flag telling them apart - lwc->dir_lease.

Seeing that indirect calls are costly these days, let's
get rid of the callback and just call the right function
directly.  Has a side benefit of saner signatures...

Signed-off-by Al Viro <viro@zeniv.linux.org.uk>
---
 fs/ceph/dir.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 91709934c8b1..768158743750 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1593,10 +1593,12 @@ struct ceph_lease_walk_control {
 	unsigned long dir_lease_ttl;
 };
 
+static int __dir_lease_check(const struct dentry *, struct ceph_lease_walk_control *);
+static int __dentry_lease_check(const struct dentry *);
+
 static unsigned long
 __dentry_leases_walk(struct ceph_mds_client *mdsc,
-		     struct ceph_lease_walk_control *lwc,
-		     int (*check)(struct dentry*, void*))
+		     struct ceph_lease_walk_control *lwc)
 {
 	struct ceph_dentry_info *di, *tmp;
 	struct dentry *dentry, *last = NULL;
@@ -1624,7 +1626,10 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
 			goto next;
 		}
 
-		ret = check(dentry, lwc);
+		if (lwc->dir_lease)
+			ret = __dir_lease_check(dentry, lwc);
+		else
+			ret = __dentry_lease_check(dentry);
 		if (ret & TOUCH) {
 			/* move it into tail of dir lease list */
 			__dentry_dir_lease_touch(mdsc, di);
@@ -1681,7 +1686,7 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
 	return freed;
 }
 
-static int __dentry_lease_check(struct dentry *dentry, void *arg)
+static int __dentry_lease_check(const struct dentry *dentry)
 {
 	struct ceph_dentry_info *di = ceph_dentry(dentry);
 	int ret;
@@ -1696,9 +1701,9 @@ static int __dentry_lease_check(struct dentry *dentry, void *arg)
 	return DELETE;
 }
 
-static int __dir_lease_check(struct dentry *dentry, void *arg)
+static int __dir_lease_check(const struct dentry *dentry,
+			     struct ceph_lease_walk_control *lwc)
 {
-	struct ceph_lease_walk_control *lwc = arg;
 	struct ceph_dentry_info *di = ceph_dentry(dentry);
 
 	int ret = __dir_lease_try_check(dentry);
@@ -1737,7 +1742,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
 
 	lwc.dir_lease = false;
 	lwc.nr_to_scan  = CEPH_CAPS_PER_RELEASE * 2;
-	freed = __dentry_leases_walk(mdsc, &lwc, __dentry_lease_check);
+	freed = __dentry_leases_walk(mdsc, &lwc);
 	if (!lwc.nr_to_scan) /* more invalid leases */
 		return -EAGAIN;
 
@@ -1747,7 +1752,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
 	lwc.dir_lease = true;
 	lwc.expire_dir_lease = freed < count;
 	lwc.dir_lease_ttl = mdsc->fsc->mount_options->caps_wanted_delay_max * HZ;
-	freed +=__dentry_leases_walk(mdsc, &lwc, __dir_lease_check);
+	freed +=__dentry_leases_walk(mdsc, &lwc);
 	if (!lwc.nr_to_scan) /* more to check */
 		return -EAGAIN;
 
-- 
2.39.2

[PATCH 18/22] nfsd: kill stale comment about simple_fill_super() requirements

[Al Viro]

That went into the tree back in 2005; the comment used to be true for
predecessor of simple_fill_super() that happened to live in nfsd; that one
didn't take care to skip the array entries with NULL ->name, so it could
not tolerate any gaps.  That had been fixed in 2003 when nfsd_fill_super()
had been abstracted into simple_fill_super(); if Neil's patch lived out
of tree during that time, he probably replaced the name of function when
rebasing it and didn't notice that restriction in question was no longer
there.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/nfsd/nfsctl.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 3e15b72f421d..26a25e40c451 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -48,10 +48,6 @@ enum {
 	NFSD_MaxBlkSize,
 	NFSD_MaxConnections,
 	NFSD_Filecache,
-	/*
-	 * The below MUST come last.  Otherwise we leave a hole in nfsd_files[]
-	 * with !CONFIG_NFSD_V4 and simple_fill_super() goes oops
-	 */
 #ifdef CONFIG_NFSD_V4
 	NFSD_Leasetime,
 	NFSD_Gracetime,
-- 
2.39.2

[PATCH 19/22] ocfs2_find_match(): there's no such thing as NULL or negative ->d_parent

[Al Viro]

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

diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c
index 04fc8344063a..a9b8688aaf30 100644
--- a/fs/ocfs2/dcache.c
+++ b/fs/ocfs2/dcache.c
@@ -124,17 +124,10 @@ static int ocfs2_match_dentry(struct dentry *dentry,
 	if (!dentry->d_fsdata)
 		return 0;
 
-	if (!dentry->d_parent)
-		return 0;
-
 	if (skip_unhashed && d_unhashed(dentry))
 		return 0;
 
 	parent = d_inode(dentry->d_parent);
-	/* Negative parent dentry? */
-	if (!parent)
-		return 0;
-
 	/* Name is in a different directory. */
 	if (OCFS2_I(parent)->ip_blkno != parent_blkno)
 		return 0;
-- 
2.39.2

[PATCH 20/22] gfs2: use is_subdir()

[Al Viro]

... instead of reimplementing it with misguiding name (is_ancestor(x, y)
would normally imply "x is an ancestor of y", not the other way round).
With races, while we are at it...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/gfs2/super.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index d21c04a22d73..b5c75c8a8d62 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1065,16 +1065,6 @@ static int gfs2_drop_inode(struct inode *inode)
 	return generic_drop_inode(inode);
 }
 
-static int is_ancestor(const struct dentry *d1, const struct dentry *d2)
-{
-	do {
-		if (d1 == d2)
-			return 1;
-		d1 = d1->d_parent;
-	} while (!IS_ROOT(d1));
-	return 0;
-}
-
 /**
  * gfs2_show_options - Show mount options for /proc/mounts
  * @s: seq_file structure
@@ -1096,7 +1086,7 @@ static int gfs2_show_options(struct seq_file *s, struct dentry *root)
 	statfs_slow = sdp->sd_tune.gt_statfs_slow;
 	spin_unlock(&sdp->sd_tune.gt_spin);
 
-	if (is_ancestor(root, sdp->sd_master_dir))
+	if (is_subdir(root, sdp->sd_master_dir))
 		seq_puts(s, ",meta");
 	if (args->ar_lockproto[0])
 		seq_show_option(s, "lockproto", args->ar_lockproto);
-- 
2.39.2

[PATCH 21/22] orangefs: saner arguments passing in readdir guts

[Al Viro]

orangefs_dir_fill() doesn't use oi and dentry arguments at all
do_readdir() gets dentry, uses only dentry->d_inode; it also
gets oi, which is ORANGEFS_I(dentry->d_inode) (i.e. ->d_inode -
constant offset).
orangefs_dir_mode() gets dentry and oi, uses only to pass those
to do_readdir().
orangefs_dir_iterate() uses dentry and oi only to pass those to
orangefs_dir_fill() and orangefs_dir_more().

The only thing it really needs is ->d_inode; moreover, that's
better expressed as file_inode(file) - no need to go through
->f_path.dentry->d_inode.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/orangefs/dir.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c
index 9cacce5d55c1..6d1fbeca9d81 100644
--- a/fs/orangefs/dir.c
+++ b/fs/orangefs/dir.c
@@ -58,10 +58,10 @@ struct orangefs_dir {
  * first part of the part list.
  */
 
-static int do_readdir(struct orangefs_inode_s *oi,
-    struct orangefs_dir *od, struct dentry *dentry,
+static int do_readdir(struct orangefs_dir *od, struct inode *inode,
     struct orangefs_kernel_op_s *op)
 {
+	struct orangefs_inode_s *oi = ORANGEFS_I(inode);
 	struct orangefs_readdir_response_s *resp;
 	int bufi, r;
 
@@ -87,7 +87,7 @@ static int do_readdir(struct orangefs_inode_s *oi,
 	op->upcall.req.readdir.buf_index = bufi;
 
 	r = service_operation(op, "orangefs_readdir",
-	    get_interruptible_flag(dentry->d_inode));
+	    get_interruptible_flag(inode));
 
 	orangefs_readdir_index_put(bufi);
 
@@ -158,8 +158,7 @@ static int parse_readdir(struct orangefs_dir *od,
 	return 0;
 }
 
-static int orangefs_dir_more(struct orangefs_inode_s *oi,
-    struct orangefs_dir *od, struct dentry *dentry)
+static int orangefs_dir_more(struct orangefs_dir *od, struct inode *inode)
 {
 	struct orangefs_kernel_op_s *op;
 	int r;
@@ -169,7 +168,7 @@ static int orangefs_dir_more(struct orangefs_inode_s *oi,
 		od->error = -ENOMEM;
 		return -ENOMEM;
 	}
-	r = do_readdir(oi, od, dentry, op);
+	r = do_readdir(od, inode, op);
 	if (r) {
 		od->error = r;
 		goto out;
@@ -238,9 +237,7 @@ static int fill_from_part(struct orangefs_dir_part *part,
 	return 1;
 }
 
-static int orangefs_dir_fill(struct orangefs_inode_s *oi,
-    struct orangefs_dir *od, struct dentry *dentry,
-    struct dir_context *ctx)
+static int orangefs_dir_fill(struct orangefs_dir *od, struct dir_context *ctx)
 {
 	struct orangefs_dir_part *part;
 	size_t count;
@@ -304,15 +301,10 @@ static loff_t orangefs_dir_llseek(struct file *file, loff_t offset,
 static int orangefs_dir_iterate(struct file *file,
     struct dir_context *ctx)
 {
-	struct orangefs_inode_s *oi;
-	struct orangefs_dir *od;
-	struct dentry *dentry;
+	struct orangefs_dir *od = file->private_data;
+	struct inode *inode = file_inode(file);
 	int r;
 
-	dentry = file->f_path.dentry;
-	oi = ORANGEFS_I(dentry->d_inode);
-	od = file->private_data;
-
 	if (od->error)
 		return od->error;
 
@@ -342,7 +334,7 @@ static int orangefs_dir_iterate(struct file *file,
 	 */
 	while (od->token != ORANGEFS_ITERATE_END &&
 	    ctx->pos > od->end) {
-		r = orangefs_dir_more(oi, od, dentry);
+		r = orangefs_dir_more(od, inode);
 		if (r)
 			return r;
 	}
@@ -351,17 +343,17 @@ static int orangefs_dir_iterate(struct file *file,
 
 	/* Then try to fill if there's any left in the buffer. */
 	if (ctx->pos < od->end) {
-		r = orangefs_dir_fill(oi, od, dentry, ctx);
+		r = orangefs_dir_fill(od, ctx);
 		if (r)
 			return r;
 	}
 
 	/* Finally get some more and try to fill. */
 	if (od->token != ORANGEFS_ITERATE_END) {
-		r = orangefs_dir_more(oi, od, dentry);
+		r = orangefs_dir_more(od, inode);
 		if (r)
 			return r;
-		r = orangefs_dir_fill(oi, od, dentry, ctx);
+		r = orangefs_dir_fill(od, ctx);
 	}
 
 	return r;
-- 
2.39.2

[PATCH 22/22] apparmorfs: don't duplicate kfree_link()

[Al Viro]

rawdata_link_cb() is identical to it

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 security/apparmor/apparmorfs.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 38650e52ef57..63ca77103de9 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -1615,11 +1615,6 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname)
 	return buffer;
 }
 
-static void rawdata_link_cb(void *arg)
-{
-	kfree(arg);
-}
-
 static const char *rawdata_get_link_base(struct dentry *dentry,
 					 struct inode *inode,
 					 struct delayed_call *done,
@@ -1643,7 +1638,7 @@ static const char *rawdata_get_link_base(struct dentry *dentry,
 	if (IS_ERR(target))
 		return target;
 
-	set_delayed_call(done, rawdata_link_cb, target);
+	set_delayed_call(done, kfree_link, target);
 
 	return target;
 }
-- 
2.39.2

Re: [PATCH 03/22] zonefs: d_splice_alias() will do the right thing on ERR_PTR() inode

[Damien Le Moal]

On 12/20/23 14:17, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/zonefs/super.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> index e6a75401677d..93971742613a 100644
> --- a/fs/zonefs/super.c
> +++ b/fs/zonefs/super.c
> @@ -747,8 +747,6 @@ static struct dentry *zonefs_lookup(struct inode *dir, struct dentry *dentry,
>  		inode = zonefs_get_dir_inode(dir, dentry);
>  	else
>  		inode = zonefs_get_file_inode(dir, dentry);
> -	if (IS_ERR(inode))
> -		return ERR_CAST(inode);
>  
>  	return d_splice_alias(inode, dentry);
>  }

Acked-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 13/22] bfs_add_entry(): get rid of pointless ->d_name.len checks

[Tigran Aivazian]

Hi Al and All,

On Wed, 20 Dec 2023 at 05:25, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> First of all, any dentry getting here would have passed bfs_lookup(),
> so it it passed ENAMETOOLONG check there, there's no need to
> repeat it.  And we are not going to get dentries with zero name length -
> that check ultimately comes from ext2 and it's as pointless here as it
> used to be there.

Yes, you are absolutely right, of course -- I must have looked at ext3
(I think it was ext3, not ext2) code at the time I wrote this and
assumed that it was necessary.

Kind regards,
Tigran

Acknowledged-by: Tigran Aivazian <aivazian.tigran@gmail.com>

>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/bfs/dir.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
> index fbc4ae80a4b2..c375e22c4c0c 100644
> --- a/fs/bfs/dir.c
> +++ b/fs/bfs/dir.c
> @@ -275,11 +275,6 @@ static int bfs_add_entry(struct inode *dir, const struct qstr *child, int ino)
>
>         dprintf("name=%s, namelen=%d\n", name, namelen);
>
> -       if (!namelen)
> -               return -ENOENT;
> -       if (namelen > BFS_NAMELEN)
> -               return -ENAMETOOLONG;
> -
>         sblock = BFS_I(dir)->i_sblock;
>         eblock = BFS_I(dir)->i_eblock;
>         for (block = sblock; block <= eblock; block++) {
> --
> 2.39.2
>

Re: [PATCHES] assorted fs cleanups

[Andreas Grünbacher]

Al,

Am Mi., 20. Dez. 2023 um 06:13 Uhr schrieb Al Viro <viro@zeniv.linux.org.uk>:
>         Assorted cleanups in various filesystems.  Currently
> that pile sits in vfs.git #work.misc; if anyone wants to pick
> these into relevant filesystem tree, I'll be glad to drop those
> from the queue...

thanks, I've added the two gfs2 patches to our for-next branch.

Andreas

Re: [PATCH 18/22] nfsd: kill stale comment about simple_fill_super() requirements

[Chuck Lever]

On Wed, Dec 20, 2023 at 05:30:13AM +0000, Al Viro wrote:
> That went into the tree back in 2005; the comment used to be true for
> predecessor of simple_fill_super() that happened to live in nfsd; that one
> didn't take care to skip the array entries with NULL ->name, so it could
> not tolerate any gaps.  That had been fixed in 2003 when nfsd_fill_super()
> had been abstracted into simple_fill_super(); if Neil's patch lived out
> of tree during that time, he probably replaced the name of function when
> rebasing it and didn't notice that restriction in question was no longer
> there.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Chuck Lever <chuck.lever@oracle.com>


> ---
>  fs/nfsd/nfsctl.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 3e15b72f421d..26a25e40c451 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -48,10 +48,6 @@ enum {
>  	NFSD_MaxBlkSize,
>  	NFSD_MaxConnections,
>  	NFSD_Filecache,
> -	/*
> -	 * The below MUST come last.  Otherwise we leave a hole in nfsd_files[]
> -	 * with !CONFIG_NFSD_V4 and simple_fill_super() goes oops
> -	 */
>  #ifdef CONFIG_NFSD_V4
>  	NFSD_Leasetime,
>  	NFSD_Gracetime,
> -- 
> 2.39.2
> 
> 

-- 
Chuck Lever

Re: nilfs2: d_obtain_alias(ERR_PTR(...)) will do the right thing...

[Ryusuke Konishi]

On Wed, Dec 20, 2023 at 2:23 PM Al Viro wrote:
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/nilfs2/namei.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
> index 2a4e7f4a8102..8e8c7c981a7a 100644
> --- a/fs/nilfs2/namei.c
> +++ b/fs/nilfs2/namei.c
> @@ -439,7 +439,6 @@ static int nilfs_rename(struct mnt_idmap *idmap,
>  static struct dentry *nilfs_get_parent(struct dentry *child)
>  {
>         unsigned long ino;
> -       struct inode *inode;
>         struct nilfs_root *root;
>
>         ino = nilfs_inode_by_name(d_inode(child), &dotdot_name);
> @@ -448,11 +447,7 @@ static struct dentry *nilfs_get_parent(struct dentry *child)
>
>         root = NILFS_I(d_inode(child))->i_root;
>
> -       inode = nilfs_iget(child->d_sb, root, ino);
> -       if (IS_ERR(inode))
> -               return ERR_CAST(inode);
> -
> -       return d_obtain_alias(inode);
> +       return d_obtain_alias(nilfs_iget(child->d_sb, root, ino));
>  }
>
>  static struct dentry *nilfs_get_dentry(struct super_block *sb, u64 cno,
> --
> 2.39.2
>

Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>

Thanks,
Ryusuke Konishi

Re: [PATCHES] assorted fs cleanups

[Al Viro]

On Wed, Dec 20, 2023 at 01:16:31PM +0100, Andreas Grünbacher wrote:
> Al,
> 
> Am Mi., 20. Dez. 2023 um 06:13 Uhr schrieb Al Viro <viro@zeniv.linux.org.uk>:
> >         Assorted cleanups in various filesystems.  Currently
> > that pile sits in vfs.git #work.misc; if anyone wants to pick
> > these into relevant filesystem tree, I'll be glad to drop those
> > from the queue...
> 
> thanks, I've added the two gfs2 patches to our for-next branch.

Dropping those two...

Re: [PATCH 07/22] ceph: d_obtain_{alias,root}(ERR_PTR(...)) will do the right thing

[Xiubo Li]

On 12/20/23 13:20, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>   fs/ceph/export.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index 726af69d4d62..a79f163ae4ed 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -286,8 +286,6 @@ static struct dentry *__snapfh_to_dentry(struct super_block *sb,
>   		doutc(cl, "%llx.%llx parent %llx hash %x err=%d", vino.ino,
>   		      vino.snap, sfh->parent_ino, sfh->hash, err);
>   	}
> -	if (IS_ERR(inode))
> -		return ERR_CAST(inode);
>   	/* see comments in ceph_get_parent() */
>   	return unlinked ? d_obtain_root(inode) : d_obtain_alias(inode);
>   }

Reviewed-by: Xiubo Li <xiubli@redhat.com>

Thanks!

Re: [PATCH 17/22] get rid of passing callbacks to ceph __dentry_leases_walk()

[Xiubo Li]

On 12/20/23 13:29, Al Viro wrote:
> __dentry_leases_walk() is gets a callback and calls it for
> a bunch of denties; there are exactly two callers and
> we already have a flag telling them apart - lwc->dir_lease.
>
> Seeing that indirect calls are costly these days, let's
> get rid of the callback and just call the right function
> directly.  Has a side benefit of saner signatures...
>
> Signed-off-by Al Viro <viro@zeniv.linux.org.uk>
> ---
>   fs/ceph/dir.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 91709934c8b1..768158743750 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1593,10 +1593,12 @@ struct ceph_lease_walk_control {
>   	unsigned long dir_lease_ttl;
>   };
>   
> +static int __dir_lease_check(const struct dentry *, struct ceph_lease_walk_control *);
> +static int __dentry_lease_check(const struct dentry *);
> +
>   static unsigned long
>   __dentry_leases_walk(struct ceph_mds_client *mdsc,
> -		     struct ceph_lease_walk_control *lwc,
> -		     int (*check)(struct dentry*, void*))
> +		     struct ceph_lease_walk_control *lwc)
>   {
>   	struct ceph_dentry_info *di, *tmp;
>   	struct dentry *dentry, *last = NULL;
> @@ -1624,7 +1626,10 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
>   			goto next;
>   		}
>   
> -		ret = check(dentry, lwc);
> +		if (lwc->dir_lease)
> +			ret = __dir_lease_check(dentry, lwc);
> +		else
> +			ret = __dentry_lease_check(dentry);
>   		if (ret & TOUCH) {
>   			/* move it into tail of dir lease list */
>   			__dentry_dir_lease_touch(mdsc, di);
> @@ -1681,7 +1686,7 @@ __dentry_leases_walk(struct ceph_mds_client *mdsc,
>   	return freed;
>   }
>   
> -static int __dentry_lease_check(struct dentry *dentry, void *arg)
> +static int __dentry_lease_check(const struct dentry *dentry)
>   {
>   	struct ceph_dentry_info *di = ceph_dentry(dentry);
>   	int ret;
> @@ -1696,9 +1701,9 @@ static int __dentry_lease_check(struct dentry *dentry, void *arg)
>   	return DELETE;
>   }
>   
> -static int __dir_lease_check(struct dentry *dentry, void *arg)
> +static int __dir_lease_check(const struct dentry *dentry,
> +			     struct ceph_lease_walk_control *lwc)
>   {
> -	struct ceph_lease_walk_control *lwc = arg;
>   	struct ceph_dentry_info *di = ceph_dentry(dentry);
>   
>   	int ret = __dir_lease_try_check(dentry);
> @@ -1737,7 +1742,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
>   
>   	lwc.dir_lease = false;
>   	lwc.nr_to_scan  = CEPH_CAPS_PER_RELEASE * 2;
> -	freed = __dentry_leases_walk(mdsc, &lwc, __dentry_lease_check);
> +	freed = __dentry_leases_walk(mdsc, &lwc);
>   	if (!lwc.nr_to_scan) /* more invalid leases */
>   		return -EAGAIN;
>   
> @@ -1747,7 +1752,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
>   	lwc.dir_lease = true;
>   	lwc.expire_dir_lease = freed < count;
>   	lwc.dir_lease_ttl = mdsc->fsc->mount_options->caps_wanted_delay_max * HZ;
> -	freed +=__dentry_leases_walk(mdsc, &lwc, __dir_lease_check);
> +	freed +=__dentry_leases_walk(mdsc, &lwc);
>   	if (!lwc.nr_to_scan) /* more to check */
>   		return -EAGAIN;
>   

Reviewed-by: Xiubo Li <xiubli@redhat.com>

Al,

I think these two ceph patches won't be dependent by your following 
patches,  right ?

If so we can apply them to ceph-client tree and run more tests.


Thanks!

Re: [PATCH 17/22] get rid of passing callbacks to ceph __dentry_leases_walk()

[Al Viro]

On Thu, Dec 21, 2023 at 08:45:18AM +0800, Xiubo Li wrote:
> Al,
> 
> I think these two ceph patches won't be dependent by your following
> patches,  right ?
> 
> If so we can apply them to ceph-client tree and run more tests.

All of them are mutually independent, and if you are willing to take
them through your tree - all the better.

Re: [PATCH 17/22] get rid of passing callbacks to ceph __dentry_leases_walk()

[Xiubo Li]

On 12/21/23 09:12, Al Viro wrote:
> On Thu, Dec 21, 2023 at 08:45:18AM +0800, Xiubo Li wrote:
>> Al,
>>
>> I think these two ceph patches won't be dependent by your following
>> patches,  right ?
>>
>> If so we can apply them to ceph-client tree and run more tests.
> All of them are mutually independent, and if you are willing to take
> them through your tree - all the better.
>
Sure, I will apply them into the ceph-client repo. And will run the 
tests for them.

Thanks Al.

- Xiubo

Re: [PATCH 04/22] udf: d_splice_alias() will do the right thing on ERR_PTR() inode

[Jan Kara]

On Wed 20-12-23 05:18:28, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

I don't expect any conflicts in UDF tree so you can keep these patches in
VFS tree. Feel free to add:

Acked-by: Jan Kara <jack@suse.cz>

								Honza	

> ---
>  fs/udf/namei.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 3508ac484da3..92f25e540430 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -125,8 +125,6 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
>  		udf_fiiter_release(&iter);
>  
>  		inode = udf_iget(dir->i_sb, &loc);
> -		if (IS_ERR(inode))
> -			return ERR_CAST(inode);
>  	}
>  
>  	return d_splice_alias(inode, dentry);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 11/22] udf: d_obtain_alias(ERR_PTR(...)) will do the right thing...

[Jan Kara]

On Wed 20-12-23 05:23:59, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Looks good.

Acked-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/udf/namei.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 92f25e540430..a64102d63781 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -897,7 +897,6 @@ static int udf_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  static struct dentry *udf_get_parent(struct dentry *child)
>  {
>  	struct kernel_lb_addr tloc;
> -	struct inode *inode = NULL;
>  	struct udf_fileident_iter iter;
>  	int err;
>  
> @@ -907,11 +906,7 @@ static struct dentry *udf_get_parent(struct dentry *child)
>  
>  	tloc = lelb_to_cpu(iter.fi.icb.extLocation);
>  	udf_fiiter_release(&iter);
> -	inode = udf_iget(child->d_sb, &tloc);
> -	if (IS_ERR(inode))
> -		return ERR_CAST(inode);
> -
> -	return d_obtain_alias(inode);
> +	return d_obtain_alias(udf_iget(child->d_sb, &tloc));
>  }
>  
>  
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 16/22] udf_fiiter_add_entry(): check for zero ->d_name.len is bogus...

[Jan Kara]

On Wed 20-12-23 05:28:31, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Indeed.

Acked-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/udf/namei.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index a64102d63781..b1674e07d5a5 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -228,8 +228,6 @@ static int udf_fiiter_add_entry(struct inode *dir, struct dentry *dentry,
>  	char name[UDF_NAME_LEN_CS0];
>  
>  	if (dentry) {
> -		if (!dentry->d_name.len)
> -			return -EINVAL;
>  		namelen = udf_put_filename(dir->i_sb, dentry->d_name.name,
>  					   dentry->d_name.len,
>  					   name, UDF_NAME_LEN_CS0);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 21/22] orangefs: saner arguments passing in readdir guts

[Mike Marshall]

Howdy Al... I applied your orangefs patch to 6.7.0-rc6 and found one
xfstests failure that was not there when I ran against
xfstests-6.7.0-rc5. (generic/438)

I'm about to hit the road for a several day motorcycle ride in an hour
or so, I just wanted to give feedback before Ieft. I'll look into it
further when I get back.

-Mike

On Wed, Dec 20, 2023 at 12:33 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> orangefs_dir_fill() doesn't use oi and dentry arguments at all
> do_readdir() gets dentry, uses only dentry->d_inode; it also
> gets oi, which is ORANGEFS_I(dentry->d_inode) (i.e. ->d_inode -
> constant offset).
> orangefs_dir_mode() gets dentry and oi, uses only to pass those
> to do_readdir().
> orangefs_dir_iterate() uses dentry and oi only to pass those to
> orangefs_dir_fill() and orangefs_dir_more().
>
> The only thing it really needs is ->d_inode; moreover, that's
> better expressed as file_inode(file) - no need to go through
> ->f_path.dentry->d_inode.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/orangefs/dir.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c
> index 9cacce5d55c1..6d1fbeca9d81 100644
> --- a/fs/orangefs/dir.c
> +++ b/fs/orangefs/dir.c
> @@ -58,10 +58,10 @@ struct orangefs_dir {
>   * first part of the part list.
>   */
>
> -static int do_readdir(struct orangefs_inode_s *oi,
> -    struct orangefs_dir *od, struct dentry *dentry,
> +static int do_readdir(struct orangefs_dir *od, struct inode *inode,
>      struct orangefs_kernel_op_s *op)
>  {
> +       struct orangefs_inode_s *oi = ORANGEFS_I(inode);
>         struct orangefs_readdir_response_s *resp;
>         int bufi, r;
>
> @@ -87,7 +87,7 @@ static int do_readdir(struct orangefs_inode_s *oi,
>         op->upcall.req.readdir.buf_index = bufi;
>
>         r = service_operation(op, "orangefs_readdir",
> -           get_interruptible_flag(dentry->d_inode));
> +           get_interruptible_flag(inode));
>
>         orangefs_readdir_index_put(bufi);
>
> @@ -158,8 +158,7 @@ static int parse_readdir(struct orangefs_dir *od,
>         return 0;
>  }
>
> -static int orangefs_dir_more(struct orangefs_inode_s *oi,
> -    struct orangefs_dir *od, struct dentry *dentry)
> +static int orangefs_dir_more(struct orangefs_dir *od, struct inode *inode)
>  {
>         struct orangefs_kernel_op_s *op;
>         int r;
> @@ -169,7 +168,7 @@ static int orangefs_dir_more(struct orangefs_inode_s *oi,
>                 od->error = -ENOMEM;
>                 return -ENOMEM;
>         }
> -       r = do_readdir(oi, od, dentry, op);
> +       r = do_readdir(od, inode, op);
>         if (r) {
>                 od->error = r;
>                 goto out;
> @@ -238,9 +237,7 @@ static int fill_from_part(struct orangefs_dir_part *part,
>         return 1;
>  }
>
> -static int orangefs_dir_fill(struct orangefs_inode_s *oi,
> -    struct orangefs_dir *od, struct dentry *dentry,
> -    struct dir_context *ctx)
> +static int orangefs_dir_fill(struct orangefs_dir *od, struct dir_context *ctx)
>  {
>         struct orangefs_dir_part *part;
>         size_t count;
> @@ -304,15 +301,10 @@ static loff_t orangefs_dir_llseek(struct file *file, loff_t offset,
>  static int orangefs_dir_iterate(struct file *file,
>      struct dir_context *ctx)
>  {
> -       struct orangefs_inode_s *oi;
> -       struct orangefs_dir *od;
> -       struct dentry *dentry;
> +       struct orangefs_dir *od = file->private_data;
> +       struct inode *inode = file_inode(file);
>         int r;
>
> -       dentry = file->f_path.dentry;
> -       oi = ORANGEFS_I(dentry->d_inode);
> -       od = file->private_data;
> -
>         if (od->error)
>                 return od->error;
>
> @@ -342,7 +334,7 @@ static int orangefs_dir_iterate(struct file *file,
>          */
>         while (od->token != ORANGEFS_ITERATE_END &&
>             ctx->pos > od->end) {
> -               r = orangefs_dir_more(oi, od, dentry);
> +               r = orangefs_dir_more(od, inode);
>                 if (r)
>                         return r;
>         }
> @@ -351,17 +343,17 @@ static int orangefs_dir_iterate(struct file *file,
>
>         /* Then try to fill if there's any left in the buffer. */
>         if (ctx->pos < od->end) {
> -               r = orangefs_dir_fill(oi, od, dentry, ctx);
> +               r = orangefs_dir_fill(od, ctx);
>                 if (r)
>                         return r;
>         }
>
>         /* Finally get some more and try to fill. */
>         if (od->token != ORANGEFS_ITERATE_END) {
> -               r = orangefs_dir_more(oi, od, dentry);
> +               r = orangefs_dir_more(od, inode);
>                 if (r)
>                         return r;
> -               r = orangefs_dir_fill(oi, od, dentry, ctx);
> +               r = orangefs_dir_fill(od, ctx);
>         }
>
>         return r;
> --
> 2.39.2
>
>

Re: [PATCH 21/22] orangefs: saner arguments passing in readdir guts

[Mike Marshall]

Hi Al... without going into detail about me being clumsy,
there is no new regression. There is a problem I found
in orangefs and have sent it up to Walt Ligon. I had
then excluded that test from my xfstests runs and lost that
particular exclusion when I recently pulled/updated my
xfstests.

So... I have tested your patch and I think it is good.

Some parts of the motorcycle ride were chilly...

https://hubcapsc.com/misc/madison_georgia_motel_frost.jpg

-Mike

On Wed, Dec 27, 2023 at 7:05 AM Mike Marshall <hubcap@omnibond.com> wrote:
>
> Howdy Al... I applied your orangefs patch to 6.7.0-rc6 and found one
> xfstests failure that was not there when I ran against
> xfstests-6.7.0-rc5. (generic/438)
>
> I'm about to hit the road for a several day motorcycle ride in an hour
> or so, I just wanted to give feedback before Ieft. I'll look into it
> further when I get back.
>
> -Mike
>
> On Wed, Dec 20, 2023 at 12:33 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > orangefs_dir_fill() doesn't use oi and dentry arguments at all
> > do_readdir() gets dentry, uses only dentry->d_inode; it also
> > gets oi, which is ORANGEFS_I(dentry->d_inode) (i.e. ->d_inode -
> > constant offset).
> > orangefs_dir_mode() gets dentry and oi, uses only to pass those
> > to do_readdir().
> > orangefs_dir_iterate() uses dentry and oi only to pass those to
> > orangefs_dir_fill() and orangefs_dir_more().
> >
> > The only thing it really needs is ->d_inode; moreover, that's
> > better expressed as file_inode(file) - no need to go through
> > ->f_path.dentry->d_inode.
> >
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
> >  fs/orangefs/dir.c | 32 ++++++++++++--------------------
> >  1 file changed, 12 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/orangefs/dir.c b/fs/orangefs/dir.c
> > index 9cacce5d55c1..6d1fbeca9d81 100644
> > --- a/fs/orangefs/dir.c
> > +++ b/fs/orangefs/dir.c
> > @@ -58,10 +58,10 @@ struct orangefs_dir {
> >   * first part of the part list.
> >   */
> >
> > -static int do_readdir(struct orangefs_inode_s *oi,
> > -    struct orangefs_dir *od, struct dentry *dentry,
> > +static int do_readdir(struct orangefs_dir *od, struct inode *inode,
> >      struct orangefs_kernel_op_s *op)
> >  {
> > +       struct orangefs_inode_s *oi = ORANGEFS_I(inode);
> >         struct orangefs_readdir_response_s *resp;
> >         int bufi, r;
> >
> > @@ -87,7 +87,7 @@ static int do_readdir(struct orangefs_inode_s *oi,
> >         op->upcall.req.readdir.buf_index = bufi;
> >
> >         r = service_operation(op, "orangefs_readdir",
> > -           get_interruptible_flag(dentry->d_inode));
> > +           get_interruptible_flag(inode));
> >
> >         orangefs_readdir_index_put(bufi);
> >
> > @@ -158,8 +158,7 @@ static int parse_readdir(struct orangefs_dir *od,
> >         return 0;
> >  }
> >
> > -static int orangefs_dir_more(struct orangefs_inode_s *oi,
> > -    struct orangefs_dir *od, struct dentry *dentry)
> > +static int orangefs_dir_more(struct orangefs_dir *od, struct inode *inode)
> >  {
> >         struct orangefs_kernel_op_s *op;
> >         int r;
> > @@ -169,7 +168,7 @@ static int orangefs_dir_more(struct orangefs_inode_s *oi,
> >                 od->error = -ENOMEM;
> >                 return -ENOMEM;
> >         }
> > -       r = do_readdir(oi, od, dentry, op);
> > +       r = do_readdir(od, inode, op);
> >         if (r) {
> >                 od->error = r;
> >                 goto out;
> > @@ -238,9 +237,7 @@ static int fill_from_part(struct orangefs_dir_part *part,
> >         return 1;
> >  }
> >
> > -static int orangefs_dir_fill(struct orangefs_inode_s *oi,
> > -    struct orangefs_dir *od, struct dentry *dentry,
> > -    struct dir_context *ctx)
> > +static int orangefs_dir_fill(struct orangefs_dir *od, struct dir_context *ctx)
> >  {
> >         struct orangefs_dir_part *part;
> >         size_t count;
> > @@ -304,15 +301,10 @@ static loff_t orangefs_dir_llseek(struct file *file, loff_t offset,
> >  static int orangefs_dir_iterate(struct file *file,
> >      struct dir_context *ctx)
> >  {
> > -       struct orangefs_inode_s *oi;
> > -       struct orangefs_dir *od;
> > -       struct dentry *dentry;
> > +       struct orangefs_dir *od = file->private_data;
> > +       struct inode *inode = file_inode(file);
> >         int r;
> >
> > -       dentry = file->f_path.dentry;
> > -       oi = ORANGEFS_I(dentry->d_inode);
> > -       od = file->private_data;
> > -
> >         if (od->error)
> >                 return od->error;
> >
> > @@ -342,7 +334,7 @@ static int orangefs_dir_iterate(struct file *file,
> >          */
> >         while (od->token != ORANGEFS_ITERATE_END &&
> >             ctx->pos > od->end) {
> > -               r = orangefs_dir_more(oi, od, dentry);
> > +               r = orangefs_dir_more(od, inode);
> >                 if (r)
> >                         return r;
> >         }
> > @@ -351,17 +343,17 @@ static int orangefs_dir_iterate(struct file *file,
> >
> >         /* Then try to fill if there's any left in the buffer. */
> >         if (ctx->pos < od->end) {
> > -               r = orangefs_dir_fill(oi, od, dentry, ctx);
> > +               r = orangefs_dir_fill(od, ctx);
> >                 if (r)
> >                         return r;
> >         }
> >
> >         /* Finally get some more and try to fill. */
> >         if (od->token != ORANGEFS_ITERATE_END) {
> > -               r = orangefs_dir_more(oi, od, dentry);
> > +               r = orangefs_dir_more(od, inode);
> >                 if (r)
> >                         return r;
> > -               r = orangefs_dir_fill(oi, od, dentry, ctx);
> > +               r = orangefs_dir_fill(od, ctx);
> >         }
> >
> >         return r;
> > --
> > 2.39.2
> >
> >