2023-06-01 11:08:53

by Jan Kara

[permalink] [raw]
Subject: [PATCH v2 1/6] ext4: Remove ext4 locking of moved directory

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 <[email protected]>
CC: [email protected]
Signed-off-by: Jan Kara <[email protected]>
---
fs/ext4/namei.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)

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;
}

--
2.35.3



2023-06-01 14:59:41

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] ext4: Remove ext4 locking of moved directory

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

2023-06-01 15:30:22

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] ext4: Remove ext4 locking of moved directory

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
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-06-01 16:10:14

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] ext4: Remove ext4 locking of moved directory

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.