diff mbox series

[v7,2/3] fs: introduce lock_rename_child() helper

Message ID 20220920224338.22217-3-linkinjeon@kernel.org (mailing list archive)
State New, archived
Headers show
Series ksmbd patches included vfs changes | expand

Commit Message

Namjae Jeon Sept. 20, 2022, 10:43 p.m. UTC 
From: Al Viro <viro@zeniv.linux.org.uk>

Pass the dentry of a source file and the dentry of a destination directory
to lock parent inodes for rename. As soon as this function returns,
->d_parent of the source file dentry is stable and inodes are properly
locked for calling vfs-rename. This helper is needed for ksmbd server.
rename request of SMB protocol has to rename an opened file, no matter
which directory it's in.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/namei.c            | 46 ++++++++++++++++++++++++++++++++-----------
 include/linux/namei.h |  1 +
 2 files changed, 36 insertions(+), 11 deletions(-)

Comments

Al Viro Oct. 2, 2022, 12:21 a.m. UTC | #1 
On Wed, Sep 21, 2022 at 07:43:37AM +0900, Namjae Jeon wrote:


FWIW, it probably needs a few comments:

// c1 and p2 should be on the same fs
> +struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
> +{
> +	if (READ_ONCE(c1->d_parent) == p2) {
		// hopefully won't need to touch ->s_vfs_rename_mutex at all.
> +		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
		// now that p2 is locked, nobody can move in or out of it,
		// so the test below is safe
> +		if (likely(c1->d_parent == p2))
> +			return NULL;
> +
		// c1 got moved out of p2 while we'd been taking locks;
		// unlock and fall back to slow case
> +		inode_unlock(p2->d_inode);
> +	}
> +
> +	mutex_lock(&c1->d_sb->s_vfs_rename_mutex);
	// nobody can move out of any directories on this fs
> +	if (likely(c1->d_parent != p2))
> +		return lock_two_directories(c1->d_parent, p2);
> +
	// c1 got moved into p2 while we were taking locks;
	// we need p2 locked and ->s_vfs_rename_mutex unlocked,
	// for consistency with lock_rename().
> +	inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
> +	mutex_unlock(&c1->d_sb->s_vfs_rename_mutex);
> +	return NULL;
> +}
> +EXPORT_SYMBOL(lock_rename_child);
Namjae Jeon Oct. 2, 2022, 2:22 a.m. UTC | #2 
2022-10-02 9:21 GMT+09:00, Al Viro <viro@zeniv.linux.org.uk>:
> On Wed, Sep 21, 2022 at 07:43:37AM +0900, Namjae Jeon wrote:
>
>
> FWIW, it probably needs a few comments:
I will add it on next spin.

Thanks for your review!
>
> // c1 and p2 should be on the same fs
>> +struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
>> +{
>> +	if (READ_ONCE(c1->d_parent) == p2) {
> 		// hopefully won't need to touch ->s_vfs_rename_mutex at all.
>> +		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
> 		// now that p2 is locked, nobody can move in or out of it,
> 		// so the test below is safe
>> +		if (likely(c1->d_parent == p2))
>> +			return NULL;
>> +
> 		// c1 got moved out of p2 while we'd been taking locks;
> 		// unlock and fall back to slow case
>> +		inode_unlock(p2->d_inode);
>> +	}
>> +
>> +	mutex_lock(&c1->d_sb->s_vfs_rename_mutex);
> 	// nobody can move out of any directories on this fs
>> +	if (likely(c1->d_parent != p2))
>> +		return lock_two_directories(c1->d_parent, p2);
>> +
> 	// c1 got moved into p2 while we were taking locks;
> 	// we need p2 locked and ->s_vfs_rename_mutex unlocked,
> 	// for consistency with lock_rename().
>> +	inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
>> +	mutex_unlock(&c1->d_sb->s_vfs_rename_mutex);
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL(lock_rename_child);
>
diff mbox series

Patch

diff --git a/fs/namei.c b/fs/namei.c
index 53b4bc094db2..5ff7f2a9e54e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2979,20 +2979,10 @@  static inline int may_create(struct user_namespace *mnt_userns,
 	return inode_permission(mnt_userns, dir, MAY_WRITE | MAY_EXEC);
 }
 
-/*
- * p1 and p2 should be directories on the same fs.
- */
-struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
+static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
 {
 	struct dentry *p;
 
-	if (p1 == p2) {
-		inode_lock_nested(p1->d_inode, I_MUTEX_PARENT);
-		return NULL;
-	}
-
-	mutex_lock(&p1->d_sb->s_vfs_rename_mutex);
-
 	p = d_ancestor(p2, p1);
 	if (p) {
 		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
@@ -3011,8 +3001,42 @@  struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
 	inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2);
 	return NULL;
 }
+
+/*
+ * p1 and p2 should be directories on the same fs.
+ */
+struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
+{
+	if (p1 == p2) {
+		inode_lock_nested(p1->d_inode, I_MUTEX_PARENT);
+		return NULL;
+	}
+
+	mutex_lock(&p1->d_sb->s_vfs_rename_mutex);
+	return lock_two_directories(p1, p2);
+}
 EXPORT_SYMBOL(lock_rename);
 
+struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
+{
+	if (READ_ONCE(c1->d_parent) == p2) {
+		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
+		if (likely(c1->d_parent == p2))
+			return NULL;
+
+		inode_unlock(p2->d_inode);
+	}
+
+	mutex_lock(&c1->d_sb->s_vfs_rename_mutex);
+	if (likely(c1->d_parent != p2))
+		return lock_two_directories(c1->d_parent, p2);
+
+	inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
+	mutex_unlock(&c1->d_sb->s_vfs_rename_mutex);
+	return NULL;
+}
+EXPORT_SYMBOL(lock_rename_child);
+
 void unlock_rename(struct dentry *p1, struct dentry *p2)
 {
 	inode_unlock(p1->d_inode);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 40c693525f79..7868732cce24 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -83,6 +83,7 @@  extern int follow_down(struct path *);
 extern int follow_up(struct path *);
 
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);
+extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
 
 extern int __must_check nd_jump_link(struct path *path);