Re: [RFC PATCH 13/14] kill configfs_drop_dentry()

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, May 19, 2026 at 03:12:56PM +0200, Jan Kara wrote:

> Why is it safe to call dget_dlock() here? We hold only configfs_dirent_lock
> so cannot we race with somebody doing dget() on the 'child' dentry? Even if
> it is indeed safe for some reason I'm missing, it would deserve a comment I
> guess?

Because of the braindamage on conflict resolution.  Thanks for spotting,
fixed and force-pushed.  Updated version of commit:

commit 85863f5773c15f02ff6518ad034e3b380b78ca53
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Tue May 12 12:53:35 2026 -0400

    kill configfs_drop_dentry()
    
    Fold into the only remaining user, don't bother with the timestamps
    of parent - we are going to rmdir it shortly anyway, which will
    override those.
    
    Fix the locking of inode, while we are at it - updating the link
    count and timestamps ought to be done with the inode locked.
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index 0b969d0eb8ff..acdeea8e2d69 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -76,7 +76,6 @@ extern int configfs_make_dirent(struct configfs_dirent *, struct dentry *,
 extern int configfs_dirent_is_ready(struct configfs_dirent *);
 
 extern const unsigned char * configfs_get_name(struct configfs_dirent *sd);
-extern void configfs_drop_dentry(struct configfs_dirent *sd, struct dentry *parent);
 extern int configfs_setattr(struct mnt_idmap *idmap,
 			    struct dentry *dentry, struct iattr *iattr);
 
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 740ea2c115bd..a68b4a9241c0 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -562,12 +562,33 @@ static void detach_attrs(struct dentry *dentry)
 
 	parent_sd = dentry->d_fsdata;
 	list_for_each_entry_safe(sd, tmp, &parent_sd->s_children, s_sibling) {
+		struct dentry *child;
 		if (!sd->s_element || !(sd->s_type & CONFIGFS_NOT_PINNED))
 			continue;
 		spin_lock(&configfs_dirent_lock);
 		list_del_init(&sd->s_sibling);
+		child = sd->s_dentry;
+		if (child) {
+			spin_lock(&child->d_lock);
+			if (simple_positive(child)) {
+				dget_dlock(child);
+				spin_unlock(&child->d_lock);
+			} else {
+				spin_unlock(&child->d_lock);
+				child = NULL;
+			}
+		}
 		spin_unlock(&configfs_dirent_lock);
-		configfs_drop_dentry(sd, dentry);
+		if (child) {
+			struct inode *inode = child->d_inode;
+			d_drop(child);
+
+			inode_lock_nested(inode, I_MUTEX_NONDIR2);
+			inode_set_ctime_current(inode);
+			drop_nlink(inode);
+			inode_unlock(inode);
+			dput(child);
+		}
 		configfs_put(sd);
 	}
 }
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index bc507b720a01..3c26b6c443be 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -195,25 +195,3 @@ const unsigned char * configfs_get_name(struct configfs_dirent *sd)
 	}
 	return NULL;
 }
-
-
-/*
- * Unhashes the dentry corresponding to given configfs_dirent
- * Called with parent inode's i_mutex held.
- */
-void configfs_drop_dentry(struct configfs_dirent * sd, struct dentry * parent)
-{
-	struct dentry * dentry = sd->s_dentry;
-
-	if (dentry) {
-		spin_lock(&dentry->d_lock);
-		if (simple_positive(dentry)) {
-			dget_dlock(dentry);
-			__d_drop(dentry);
-			spin_unlock(&dentry->d_lock);
-			__simple_unlink(d_inode(parent), dentry);
-			dput(dentry);
-		} else
-			spin_unlock(&dentry->d_lock);
-	}
-}