| Message ID | 20230601105830.13168-1-jack@suse.cz (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3658840cd363f2be094f5dfd2f0b174a9055dd0f |
| Headers | show |
| Series | fs: Fix directory corruption when moving directories | expand |
On Thu, Jun 01, 2023 at 12:58:21PM +0200, Jan Kara wrote: > Remove locking of moved directory in ext4_rename2(). We will take care > of it in VFS instead. This effectively reverts commit 0813299c586b > ("ext4: Fix possible corruption when moving a directory") and followup > fixes. Remind me --- commit 0813299c586b is not actually causing any problems; it's just not fully effective at solving the problem. Is that correct? In other words, is there a rush in trying to get this revert to Linus during this cycle as a regression fix? I think the answer is no, and we can just let this full patch series go in via the vfs branch during the next merge window, but I just wanted to make sure. Thanks! - Ted
On Thu 01-06-23 10:52:22, Theodore Ts'o wrote: > On Thu, Jun 01, 2023 at 12:58:21PM +0200, Jan Kara wrote: > > Remove locking of moved directory in ext4_rename2(). We will take care > > of it in VFS instead. This effectively reverts commit 0813299c586b > > ("ext4: Fix possible corruption when moving a directory") and followup > > fixes. > > Remind me --- commit 0813299c586b is not actually causing any > problems; it's just not fully effective at solving the problem. Is > that correct? Yes, correct. > In other words, is there a rush in trying to get this revert to Linus > during this cycle as a regression fix? > > I think the answer is no, and we can just let this full patch series > go in via the vfs branch during the next merge window, but I just > wanted to make sure. Exactly, that's my plan as well. Honza
On Thu, Jun 01, 2023 at 05:27:46PM +0200, Jan Kara wrote: > On Thu 01-06-23 10:52:22, Theodore Ts'o wrote: > > On Thu, Jun 01, 2023 at 12:58:21PM +0200, Jan Kara wrote: > > > Remove locking of moved directory in ext4_rename2(). We will take care > > > of it in VFS instead. This effectively reverts commit 0813299c586b > > > ("ext4: Fix possible corruption when moving a directory") and followup > > > fixes. > > > > Remind me --- commit 0813299c586b is not actually causing any > > problems; it's just not fully effective at solving the problem. Is > > that correct? > > Yes, correct. > > > In other words, is there a rush in trying to get this revert to Linus > > during this cycle as a regression fix? > > > > I think the answer is no, and we can just let this full patch series > > go in via the vfs branch during the next merge window, but I just > > wanted to make sure. > > Exactly, that's my plan as well. Yeah, we'll have time and ideally this should soak in -next for a good while also gives others time to take a look.
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 45b579805c95..0caf6c730ce3 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -3834,19 +3834,10 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir, return retval; } - /* - * We need to protect against old.inode directory getting converted - * from inline directory format into a normal one. - */ - if (S_ISDIR(old.inode->i_mode)) - inode_lock_nested(old.inode, I_MUTEX_NONDIR2); - old.bh = ext4_find_entry(old.dir, &old.dentry->d_name, &old.de, &old.inlined); - if (IS_ERR(old.bh)) { - retval = PTR_ERR(old.bh); - goto unlock_moved_dir; - } + if (IS_ERR(old.bh)) + return PTR_ERR(old.bh); /* * Check for inode number is _not_ due to possible IO errors. @@ -4043,10 +4034,6 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir, brelse(old.bh); brelse(new.bh); -unlock_moved_dir: - if (S_ISDIR(old.inode->i_mode)) - inode_unlock(old.inode); - return retval; }
Remove locking of moved directory in ext4_rename2(). We will take care of it in VFS instead. This effectively reverts commit 0813299c586b ("ext4: Fix possible corruption when moving a directory") and followup fixes. CC: Ted Tso <tytso@mit.edu> CC: stable@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz> --- fs/ext4/namei.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)