> On Jun 29, 2020, at 6:26 AM, Yi Zhuang <zhuangyi1@xxxxxxxxxx> wrote:
>
> If dquot_initialize() return non-zero and trace of ext4_unlink_enter/exit
> enabled then the matching-pair of trace_exit will lost in log.
>
> v2:
> Change the new label to be "out_trace:", which makes it more clear that
> it is undoing the "trace" part of the code. At the same time, fix other
> similar problems in this function:
>
> bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
> if (IS_ERR(bh))
> return PTR_ERR(bh);
> if (!bh)
> goto end_unlink;
>
> According to Andreas' suggestion, split up the "end_unlink:" label becomes
> two separate labels, and then remove the "if (handle)" check, and then
> use out_bh: before the handle is started.
>
> Signed-off-by: Yi Zhuang <zhuangyi1@xxxxxxxxxx>
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> ---
> fs/ext4/namei.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 56738b538ddf..941f66f417f0 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3193,30 +3193,33 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
> * in separate transaction */
> retval = dquot_initialize(dir);
> if (retval)
> - return retval;
> + goto out_trace;
> retval = dquot_initialize(d_inode(dentry));
> if (retval)
> - return retval;
> + goto out_trace;
>
> - retval = -ENOENT;
> bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
> - if (IS_ERR(bh))
> - return PTR_ERR(bh);
> - if (!bh)
> - goto end_unlink;
> + if (IS_ERR(bh)) {
> + retval = PTR_ERR(bh);
> + goto out_trace;
> + }
> + if (!bh) {
> + retval = -ENOENT;
> + goto out_trace;
> + }
>
> inode = d_inode(dentry);
>
> - retval = -EFSCORRUPTED;
> - if (le32_to_cpu(de->inode) != inode->i_ino)
> - goto end_unlink;
> + if (le32_to_cpu(de->inode) != inode->i_ino) {
> + retval = -EFSCORRUPTED;
> + goto out_bh;
> + }
>
> handle = ext4_journal_start(dir, EXT4_HT_DIR,
> EXT4_DATA_TRANS_BLOCKS(dir->i_sb));
> if (IS_ERR(handle)) {
> retval = PTR_ERR(handle);
> - handle = NULL;
> - goto end_unlink;
> + goto out_bh;
> }
>
> if (IS_DIRSYNC(dir))
> @@ -3224,12 +3227,12 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
>
> retval = ext4_delete_entry(handle, dir, de, bh);
> if (retval)
> - goto end_unlink;
> + goto out_handle;
> dir->i_ctime = dir->i_mtime = current_time(dir);
> ext4_update_dx_flag(dir);
> retval = ext4_mark_inode_dirty(handle, dir);
> if (retval)
> - goto end_unlink;
> + goto out_handle;
> if (inode->i_nlink == 0)
> ext4_warning_inode(inode, "Deleting file '%.*s' with no links",
> dentry->d_name.len, dentry->d_name.name);
> @@ -3251,10 +3254,11 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
> d_invalidate(dentry);
> #endif
>
> -end_unlink:
> +out_handle:
> + ext4_journal_stop(handle);
> +out_bh:
> brelse(bh);
> - if (handle)
> - ext4_journal_stop(handle);
> +out_trace:
> trace_ext4_unlink_exit(dentry, retval);
> return retval;
> }
> --
> 2.26.0.106.g9fadedd
>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
