2021-03-04 09:45:58

by Zhang Yi

Subject: [PATCH v1 2/2] ext4: Do not iput inode under running transaction in ext4_rename()

In ext4_rename(), when RENAME_WHITEOUT failed to add new entry into
directory, it ends up dropping new created whiteout inode under the
running transaction. After commit <9b88f9fb0d2> ("ext4: Do not iput inode
under running transaction"), we follow the assumptions that evict() does
not get called from a transaction context but in ext4_rename() it breaks
this suggestion. Although it's not a real problem, better to obey it, so
this patch add inode to orphan list and stop transaction before final
iput().

Signed-off-by: zhangyi (F) <[email protected]>
---
fs/ext4/namei.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 17d9400563fa..defdbf80ebfa 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3786,14 +3786,14 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
*/
retval = -ENOENT;
if (!old.bh || le32_to_cpu(old.de->inode) != old.inode->i_ino)
- goto end_rename;
+ goto release_bh;

new.bh = ext4_find_entry(new.dir, &new.dentry->d_name,
&new.de, &new.inlined);
if (IS_ERR(new.bh)) {
retval = PTR_ERR(new.bh);
new.bh = NULL;
- goto end_rename;
+ goto release_bh;
}
if (new.bh) {
if (!new.inode) {
@@ -3810,15 +3810,13 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
handle = ext4_journal_start(old.dir, EXT4_HT_DIR, credits);
if (IS_ERR(handle)) {
retval = PTR_ERR(handle);
- handle = NULL;
- goto end_rename;
+ goto release_bh;
}
} else {
whiteout = ext4_whiteout_for_rename(&old, credits, &handle);
if (IS_ERR(whiteout)) {
retval = PTR_ERR(whiteout);
- whiteout = NULL;
- goto end_rename;
+ goto release_bh;
}
}

@@ -3952,16 +3950,18 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
ext4_resetent(handle, &old,
old.inode->i_ino, old_file_type);
drop_nlink(whiteout);
+ ext4_orphan_add(handle, whiteout);
}
unlock_new_inode(whiteout);
+ ext4_journal_stop(handle);
iput(whiteout);
-
+ } else {
+ ext4_journal_stop(handle);
}
+release_bh:
brelse(old.dir_bh);
brelse(old.bh);
brelse(new.bh);
- if (handle)
- ext4_journal_stop(handle);
return retval;
}

--
2.25.4

2021-03-21 04:17:46

by Theodore Ts'o

Subject: Re: [PATCH v1 2/2] ext4: Do not iput inode under running transaction in ext4_rename()

On Wed, Mar 03, 2021 at 09:17:03PM +0800, zhangyi (F) wrote:
> In ext4_rename(), when RENAME_WHITEOUT failed to add new entry into
> directory, it ends up dropping new created whiteout inode under the
> running transaction. After commit <9b88f9fb0d2> ("ext4: Do not iput inode
> under running transaction"), we follow the assumptions that evict() does
> not get called from a transaction context but in ext4_rename() it breaks
> this suggestion. Although it's not a real problem, better to obey it, so
> this patch add inode to orphan list and stop transaction before final
> iput().
>
> Signed-off-by: zhangyi (F) <[email protected]>

Thanks, applied.

- Ted