* [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink
@ 2020-06-29 12:26 Yi Zhuang
2020-06-29 19:22 ` Andreas Dilger
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Yi Zhuang @ 2020-06-29 12:26 UTC (permalink / raw)
To: tytso, adilger.kernel; +Cc: linux-ext4
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@huawei.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink
2020-06-29 12:26 [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink Yi Zhuang
@ 2020-06-29 19:22 ` Andreas Dilger
2020-07-08 10:22 ` Ritesh Harjani
2020-08-06 4:56 ` tytso
2 siblings, 0 replies; 4+ messages in thread
From: Andreas Dilger @ 2020-06-29 19:22 UTC (permalink / raw)
To: Yi Zhuang; +Cc: Theodore Y. Ts'o, Ext4 Developers List
[-- Attachment #1: Type: text/plain, Size: 3267 bytes --]
> On Jun 29, 2020, at 6:26 AM, Yi Zhuang <zhuangyi1@huawei.com> 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@huawei.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> ---
> 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 #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink
2020-06-29 12:26 [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink Yi Zhuang
2020-06-29 19:22 ` Andreas Dilger
@ 2020-07-08 10:22 ` Ritesh Harjani
2020-08-06 4:56 ` tytso
2 siblings, 0 replies; 4+ messages in thread
From: Ritesh Harjani @ 2020-07-08 10:22 UTC (permalink / raw)
To: Yi Zhuang, tytso, adilger.kernel; +Cc: linux-ext4
On 6/29/20 5:56 PM, Yi Zhuang 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@huawei.com>
Nice cleanup. Feel free to add:
Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink
2020-06-29 12:26 [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink Yi Zhuang
2020-06-29 19:22 ` Andreas Dilger
2020-07-08 10:22 ` Ritesh Harjani
@ 2020-08-06 4:56 ` tytso
2 siblings, 0 replies; 4+ messages in thread
From: tytso @ 2020-08-06 4:56 UTC (permalink / raw)
To: Yi Zhuang; +Cc: adilger.kernel, linux-ext4
On Mon, Jun 29, 2020 at 08:26:21PM +0800, Yi Zhuang 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@huawei.com>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-06 11:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-29 12:26 [PATCH v2] ext4: lost matching-pair of trace in ext4_unlink Yi Zhuang
2020-06-29 19:22 ` Andreas Dilger
2020-07-08 10:22 ` Ritesh Harjani
2020-08-06 4:56 ` tytso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).