From: Jan Kara <jack@suse.cz> To: Al Viro <viro@ZenIV.linux.org.uk> Cc: <linux-fsdevel@vger.kernel.org>, Christian Brauner <brauner@kernel.org>, Miklos Szeredi <miklos@szeredi.hu>, "Darrick J. Wong" <djwong@kernel.org>, Ted Tso <tytso@mit.edu>, Jaegeuk Kim <jaegeuk@kernel.org>, <linux-ext4@vger.kernel.org>, linux-f2fs-devel@lists.sourceforge.net, <linux-xfs@vger.kernel.org>, Jan Kara <jack@suse.cz>, stable@vger.kernel.org Subject: [PATCH 4/6] fs: Establish locking order for unrelated directories Date: Thu, 25 May 2023 12:16:10 +0200 [thread overview] Message-ID: <20230525101624.15814-4-jack@suse.cz> (raw) In-Reply-To: <20230525100654.15069-1-jack@suse.cz> Currently the locking order of inode locks for directories that are not in ancestor relationship is not defined because all operations that needed to lock two directories like this were serialized by sb->s_vfs_rename_mutex. However some filesystems need to lock two subdirectories for RENAME_EXCHANGE operations and for this we need the locking order established even for two tree-unrelated directories. Provide a helper function lock_two_inodes() that establishes lock ordering for any two inodes and use it in lock_two_directories(). CC: stable@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- fs/inode.c | 34 ++++++++++++++++++++++++++++++++++ fs/internal.h | 2 ++ fs/namei.c | 4 ++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 577799b7855f..2015fa50d34a 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -1103,6 +1103,40 @@ void discard_new_inode(struct inode *inode) } EXPORT_SYMBOL(discard_new_inode); +/** + * lock_two_inodes - lock two inodes (may be regular files but also dirs) + * + * Lock any non-NULL argument. The caller must make sure that if he is passing + * in two directories, one is not ancestor of the other. Zero, one or two + * objects may be locked by this function. + * + * @inode1: first inode to lock + * @inode2: second inode to lock + * @subclass1: inode lock subclass for the first lock obtained + * @subclass2: inode lock subclass for the second lock obtained + */ +void lock_two_inodes(struct inode *inode1, struct inode *inode2, + unsigned subclass1, unsigned subclass2) +{ + if (!inode1 || !inode2) + goto lock; + + /* + * If one object is directory and the other is not, we must make sure + * to lock directory first as the other object may be its child. + */ + if (S_ISDIR(inode2->i_mode) == S_ISDIR(inode1->i_mode)) { + if (inode1 > inode2) + swap(inode1, inode2); + } else if (!S_ISDIR(inode1->i_mode)) + swap(inode1, inode2); +lock: + if (inode1) + inode_lock_nested(inode1, subclass1); + if (inode2 && inode2 != inode1) + inode_lock_nested(inode2, subclass2); +} + /** * lock_two_nondirectories - take two i_mutexes on non-directory objects * diff --git a/fs/internal.h b/fs/internal.h index bd3b2810a36b..377030a50aca 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -152,6 +152,8 @@ extern long prune_icache_sb(struct super_block *sb, struct shrink_control *sc); int dentry_needs_remove_privs(struct mnt_idmap *, struct dentry *dentry); bool in_group_or_capable(struct mnt_idmap *idmap, const struct inode *inode, vfsgid_t vfsgid); +void lock_two_inodes(struct inode *inode1, struct inode *inode2, + unsigned subclass1, unsigned subclass2); /* * fs-writeback.c diff --git a/fs/namei.c b/fs/namei.c index e4fe0879ae55..148570aabe74 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3028,8 +3028,8 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2) return p; } - inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); - inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); + lock_two_inodes(p1->d_inode, p2->d_inode, + I_MUTEX_PARENT, I_MUTEX_PARENT2); return NULL; } -- 2.35.3
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz> To: Al Viro <viro@ZenIV.linux.org.uk> Cc: Christian Brauner <brauner@kernel.org>, Ted Tso <tytso@mit.edu>, Miklos Szeredi <miklos@szeredi.hu>, "Darrick J. Wong" <djwong@kernel.org>, Jan Kara <jack@suse.cz>, stable@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>, linux-ext4@vger.kernel.org Subject: [f2fs-dev] [PATCH 4/6] fs: Establish locking order for unrelated directories Date: Thu, 25 May 2023 12:16:10 +0200 [thread overview] Message-ID: <20230525101624.15814-4-jack@suse.cz> (raw) In-Reply-To: <20230525100654.15069-1-jack@suse.cz> Currently the locking order of inode locks for directories that are not in ancestor relationship is not defined because all operations that needed to lock two directories like this were serialized by sb->s_vfs_rename_mutex. However some filesystems need to lock two subdirectories for RENAME_EXCHANGE operations and for this we need the locking order established even for two tree-unrelated directories. Provide a helper function lock_two_inodes() that establishes lock ordering for any two inodes and use it in lock_two_directories(). CC: stable@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- fs/inode.c | 34 ++++++++++++++++++++++++++++++++++ fs/internal.h | 2 ++ fs/namei.c | 4 ++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 577799b7855f..2015fa50d34a 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -1103,6 +1103,40 @@ void discard_new_inode(struct inode *inode) } EXPORT_SYMBOL(discard_new_inode); +/** + * lock_two_inodes - lock two inodes (may be regular files but also dirs) + * + * Lock any non-NULL argument. The caller must make sure that if he is passing + * in two directories, one is not ancestor of the other. Zero, one or two + * objects may be locked by this function. + * + * @inode1: first inode to lock + * @inode2: second inode to lock + * @subclass1: inode lock subclass for the first lock obtained + * @subclass2: inode lock subclass for the second lock obtained + */ +void lock_two_inodes(struct inode *inode1, struct inode *inode2, + unsigned subclass1, unsigned subclass2) +{ + if (!inode1 || !inode2) + goto lock; + + /* + * If one object is directory and the other is not, we must make sure + * to lock directory first as the other object may be its child. + */ + if (S_ISDIR(inode2->i_mode) == S_ISDIR(inode1->i_mode)) { + if (inode1 > inode2) + swap(inode1, inode2); + } else if (!S_ISDIR(inode1->i_mode)) + swap(inode1, inode2); +lock: + if (inode1) + inode_lock_nested(inode1, subclass1); + if (inode2 && inode2 != inode1) + inode_lock_nested(inode2, subclass2); +} + /** * lock_two_nondirectories - take two i_mutexes on non-directory objects * diff --git a/fs/internal.h b/fs/internal.h index bd3b2810a36b..377030a50aca 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -152,6 +152,8 @@ extern long prune_icache_sb(struct super_block *sb, struct shrink_control *sc); int dentry_needs_remove_privs(struct mnt_idmap *, struct dentry *dentry); bool in_group_or_capable(struct mnt_idmap *idmap, const struct inode *inode, vfsgid_t vfsgid); +void lock_two_inodes(struct inode *inode1, struct inode *inode2, + unsigned subclass1, unsigned subclass2); /* * fs-writeback.c diff --git a/fs/namei.c b/fs/namei.c index e4fe0879ae55..148570aabe74 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3028,8 +3028,8 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2) return p; } - inode_lock_nested(p1->d_inode, I_MUTEX_PARENT); - inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2); + lock_two_inodes(p1->d_inode, p2->d_inode, + I_MUTEX_PARENT, I_MUTEX_PARENT2); return NULL; } -- 2.35.3 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2023-05-25 10:16 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-05-25 10:16 [PATCH 0/6] fs: Fix directory corruption when moving directories Jan Kara 2023-05-25 10:16 ` [f2fs-dev] " Jan Kara 2023-05-25 10:16 ` [PATCH 1/6] ext4: Remove ext4 locking of moved directory Jan Kara 2023-05-25 10:16 ` [f2fs-dev] " Jan Kara 2023-05-25 10:16 ` [PATCH 2/6] Revert "udf: Protect rename against modification of moved directory" Jan Kara 2023-05-25 10:16 ` [f2fs-dev] " Jan Kara 2023-05-25 10:16 ` [PATCH 3/6] Revert "f2fs: fix potential corruption when moving a directory" Jan Kara 2023-05-25 10:16 ` [f2fs-dev] " Jan Kara 2023-05-25 10:16 ` Jan Kara [this message] 2023-05-25 10:16 ` [f2fs-dev] [PATCH 4/6] fs: Establish locking order for unrelated directories Jan Kara 2023-05-26 9:45 ` Christian Brauner 2023-05-26 9:45 ` [f2fs-dev] " Christian Brauner 2023-05-29 12:41 ` Jan Kara 2023-05-29 12:41 ` [f2fs-dev] " Jan Kara 2023-05-30 12:42 ` Christian Brauner 2023-05-30 12:42 ` [f2fs-dev] " Christian Brauner 2023-05-25 10:16 ` [PATCH 5/6] fs: Lock moved directories Jan Kara 2023-05-25 10:16 ` [f2fs-dev] " Jan Kara 2023-05-25 10:16 ` [PATCH 6/6] fs: Restrict lock_two_nondirectories() to non-directory inodes Jan Kara 2023-05-25 10:16 ` [f2fs-dev] " Jan Kara 2023-05-26 12:13 ` Amir Goldstein 2023-05-26 12:13 ` [f2fs-dev] " Amir Goldstein 2023-05-29 12:42 ` Jan Kara 2023-05-29 12:42 ` [f2fs-dev] " Jan Kara 2023-05-26 15:58 ` [PATCH 0/6] fs: Fix directory corruption when moving directories Christian Brauner 2023-05-26 15:58 ` [f2fs-dev] " Christian Brauner 2023-05-31 14:09 ` Christian Brauner 2023-05-31 14:09 ` Christian Brauner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230525101624.15814-4-jack@suse.cz \ --to=jack@suse.cz \ --cc=brauner@kernel.org \ --cc=djwong@kernel.org \ --cc=jaegeuk@kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-xfs@vger.kernel.org \ --cc=miklos@szeredi.hu \ --cc=stable@vger.kernel.org \ --cc=tytso@mit.edu \ --cc=viro@ZenIV.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.