From: Chao Yu <yuchao0@huawei.com> To: Jaegeuk Kim <jaegeuk@kernel.org> Cc: <linux-kernel@vger.kernel.org>, <linux-f2fs-devel@lists.sourceforge.net> Subject: Re: [f2fs-dev] [PATCH 6/6] f2fs: set I_LINKABLE early to avoid wrong access by vfs Date: Wed, 11 Dec 2019 09:23:34 +0800 [thread overview] Message-ID: <00ced682-9522-236d-4078-4c8f2e348d39@huawei.com> (raw) In-Reply-To: <20191211012121.GA52962@jaegeuk-macbookpro.roam.corp.google.com> On 2019/12/11 9:21, Jaegeuk Kim wrote: > On 12/10, Chao Yu wrote: >> On 2019/12/10 6:23, Jaegeuk Kim wrote: >>> This patch moves setting I_LINKABLE early in rename2(whiteout) to avoid the >>> below warning. >>> >>> [ 3189.163385] WARNING: CPU: 3 PID: 59523 at fs/inode.c:358 inc_nlink+0x32/0x40 >>> [ 3189.246979] Call Trace: >>> [ 3189.248707] f2fs_init_inode_metadata+0x2d6/0x440 [f2fs] >>> [ 3189.251399] f2fs_add_inline_entry+0x162/0x8c0 [f2fs] >>> [ 3189.254010] f2fs_add_dentry+0x69/0xe0 [f2fs] >>> [ 3189.256353] f2fs_do_add_link+0xc5/0x100 [f2fs] >>> [ 3189.258774] f2fs_rename2+0xabf/0x1010 [f2fs] >>> [ 3189.261079] vfs_rename+0x3f8/0xaa0 >>> [ 3189.263056] ? tomoyo_path_rename+0x44/0x60 >>> [ 3189.265283] ? do_renameat2+0x49b/0x550 >>> [ 3189.267324] do_renameat2+0x49b/0x550 >>> [ 3189.269316] __x64_sys_renameat2+0x20/0x30 >>> [ 3189.271441] do_syscall_64+0x5a/0x230 >>> [ 3189.273410] entry_SYSCALL_64_after_hwframe+0x49/0xbe >>> [ 3189.275848] RIP: 0033:0x7f270b4d9a49 >>> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >>> --- >>> fs/f2fs/namei.c | 27 +++++++++++++-------------- >>> 1 file changed, 13 insertions(+), 14 deletions(-) >>> >>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >>> index a1c507b0b4ac..5d9584281935 100644 >>> --- a/fs/f2fs/namei.c >>> +++ b/fs/f2fs/namei.c >>> @@ -797,6 +797,7 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry, >>> >>> if (whiteout) { >>> f2fs_i_links_write(inode, false); >>> + inode->i_state |= I_LINKABLE; >>> *whiteout = inode; >>> } else { >>> d_tmpfile(dentry, inode); >>> @@ -867,6 +868,12 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> F2FS_I(old_dentry->d_inode)->i_projid))) >>> return -EXDEV; >>> >>> + if (flags & RENAME_WHITEOUT) { >>> + err = f2fs_create_whiteout(old_dir, &whiteout); >>> + if (err) >>> + return err; >>> + } >> >> To record quota info correctly, we need to create whiteout inode after >> dquot_initialize(old_dir)? > > __f2fs_tmpfile() will do it. Okay. Any comments on below question? > >> >>> + >>> err = dquot_initialize(old_dir); >>> if (err) >>> goto out; >>> @@ -898,17 +905,11 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> } >>> } >>> >>> - if (flags & RENAME_WHITEOUT) { >>> - err = f2fs_create_whiteout(old_dir, &whiteout); >>> - if (err) >>> - goto out_dir; >>> - } >>> - >>> if (new_inode) { >>> >>> err = -ENOTEMPTY; >>> if (old_dir_entry && !f2fs_empty_dir(new_inode)) >>> - goto out_whiteout; >>> + goto out_dir; >>> >>> err = -ENOENT; >>> new_entry = f2fs_find_entry(new_dir, &new_dentry->d_name, >>> @@ -916,7 +917,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> if (!new_entry) { >>> if (IS_ERR(new_page)) >>> err = PTR_ERR(new_page); >>> - goto out_whiteout; >>> + goto out_dir; >>> } >>> >>> f2fs_balance_fs(sbi, true); >>> @@ -948,7 +949,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> err = f2fs_add_link(new_dentry, old_inode); >>> if (err) { >>> f2fs_unlock_op(sbi); >>> - goto out_whiteout; >>> + goto out_dir; >>> } >>> >>> if (old_dir_entry) >>> @@ -972,7 +973,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> if (IS_ERR(old_page)) >>> err = PTR_ERR(old_page); >>> f2fs_unlock_op(sbi); >>> - goto out_whiteout; >>> + goto out_dir; >>> } >>> } >>> } >>> @@ -991,7 +992,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> f2fs_delete_entry(old_entry, old_page, old_dir, NULL); >>> >>> if (whiteout) { >>> - whiteout->i_state |= I_LINKABLE; >>> set_inode_flag(whiteout, FI_INC_LINK); >>> err = f2fs_add_link(old_dentry, whiteout); >> >> [ 3189.256353] f2fs_do_add_link+0xc5/0x100 [f2fs] >> [ 3189.258774] f2fs_rename2+0xabf/0x1010 [f2fs] >> >> Does the call stack point here? if so, we have set I_LINKABLE before >> f2fs_add_link(), why the warning still be triggered? Am I missing something? Thanks, >> >> Thanks, >> >>> if (err) >>> @@ -1027,15 +1027,14 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> f2fs_unlock_op(sbi); >>> if (new_page) >>> f2fs_put_page(new_page, 0); >>> -out_whiteout: >>> - if (whiteout) >>> - iput(whiteout); >>> out_dir: >>> if (old_dir_entry) >>> f2fs_put_page(old_dir_page, 0); >>> out_old: >>> f2fs_put_page(old_page, 0); >>> out: >>> + if (whiteout) >>> + iput(whiteout); >>> return err; >>> } >>> >>> > . >
WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <yuchao0@huawei.com> To: Jaegeuk Kim <jaegeuk@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH 6/6] f2fs: set I_LINKABLE early to avoid wrong access by vfs Date: Wed, 11 Dec 2019 09:23:34 +0800 [thread overview] Message-ID: <00ced682-9522-236d-4078-4c8f2e348d39@huawei.com> (raw) In-Reply-To: <20191211012121.GA52962@jaegeuk-macbookpro.roam.corp.google.com> On 2019/12/11 9:21, Jaegeuk Kim wrote: > On 12/10, Chao Yu wrote: >> On 2019/12/10 6:23, Jaegeuk Kim wrote: >>> This patch moves setting I_LINKABLE early in rename2(whiteout) to avoid the >>> below warning. >>> >>> [ 3189.163385] WARNING: CPU: 3 PID: 59523 at fs/inode.c:358 inc_nlink+0x32/0x40 >>> [ 3189.246979] Call Trace: >>> [ 3189.248707] f2fs_init_inode_metadata+0x2d6/0x440 [f2fs] >>> [ 3189.251399] f2fs_add_inline_entry+0x162/0x8c0 [f2fs] >>> [ 3189.254010] f2fs_add_dentry+0x69/0xe0 [f2fs] >>> [ 3189.256353] f2fs_do_add_link+0xc5/0x100 [f2fs] >>> [ 3189.258774] f2fs_rename2+0xabf/0x1010 [f2fs] >>> [ 3189.261079] vfs_rename+0x3f8/0xaa0 >>> [ 3189.263056] ? tomoyo_path_rename+0x44/0x60 >>> [ 3189.265283] ? do_renameat2+0x49b/0x550 >>> [ 3189.267324] do_renameat2+0x49b/0x550 >>> [ 3189.269316] __x64_sys_renameat2+0x20/0x30 >>> [ 3189.271441] do_syscall_64+0x5a/0x230 >>> [ 3189.273410] entry_SYSCALL_64_after_hwframe+0x49/0xbe >>> [ 3189.275848] RIP: 0033:0x7f270b4d9a49 >>> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >>> --- >>> fs/f2fs/namei.c | 27 +++++++++++++-------------- >>> 1 file changed, 13 insertions(+), 14 deletions(-) >>> >>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >>> index a1c507b0b4ac..5d9584281935 100644 >>> --- a/fs/f2fs/namei.c >>> +++ b/fs/f2fs/namei.c >>> @@ -797,6 +797,7 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry, >>> >>> if (whiteout) { >>> f2fs_i_links_write(inode, false); >>> + inode->i_state |= I_LINKABLE; >>> *whiteout = inode; >>> } else { >>> d_tmpfile(dentry, inode); >>> @@ -867,6 +868,12 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> F2FS_I(old_dentry->d_inode)->i_projid))) >>> return -EXDEV; >>> >>> + if (flags & RENAME_WHITEOUT) { >>> + err = f2fs_create_whiteout(old_dir, &whiteout); >>> + if (err) >>> + return err; >>> + } >> >> To record quota info correctly, we need to create whiteout inode after >> dquot_initialize(old_dir)? > > __f2fs_tmpfile() will do it. Okay. Any comments on below question? > >> >>> + >>> err = dquot_initialize(old_dir); >>> if (err) >>> goto out; >>> @@ -898,17 +905,11 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> } >>> } >>> >>> - if (flags & RENAME_WHITEOUT) { >>> - err = f2fs_create_whiteout(old_dir, &whiteout); >>> - if (err) >>> - goto out_dir; >>> - } >>> - >>> if (new_inode) { >>> >>> err = -ENOTEMPTY; >>> if (old_dir_entry && !f2fs_empty_dir(new_inode)) >>> - goto out_whiteout; >>> + goto out_dir; >>> >>> err = -ENOENT; >>> new_entry = f2fs_find_entry(new_dir, &new_dentry->d_name, >>> @@ -916,7 +917,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> if (!new_entry) { >>> if (IS_ERR(new_page)) >>> err = PTR_ERR(new_page); >>> - goto out_whiteout; >>> + goto out_dir; >>> } >>> >>> f2fs_balance_fs(sbi, true); >>> @@ -948,7 +949,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> err = f2fs_add_link(new_dentry, old_inode); >>> if (err) { >>> f2fs_unlock_op(sbi); >>> - goto out_whiteout; >>> + goto out_dir; >>> } >>> >>> if (old_dir_entry) >>> @@ -972,7 +973,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> if (IS_ERR(old_page)) >>> err = PTR_ERR(old_page); >>> f2fs_unlock_op(sbi); >>> - goto out_whiteout; >>> + goto out_dir; >>> } >>> } >>> } >>> @@ -991,7 +992,6 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> f2fs_delete_entry(old_entry, old_page, old_dir, NULL); >>> >>> if (whiteout) { >>> - whiteout->i_state |= I_LINKABLE; >>> set_inode_flag(whiteout, FI_INC_LINK); >>> err = f2fs_add_link(old_dentry, whiteout); >> >> [ 3189.256353] f2fs_do_add_link+0xc5/0x100 [f2fs] >> [ 3189.258774] f2fs_rename2+0xabf/0x1010 [f2fs] >> >> Does the call stack point here? if so, we have set I_LINKABLE before >> f2fs_add_link(), why the warning still be triggered? Am I missing something? Thanks, >> >> Thanks, >> >>> if (err) >>> @@ -1027,15 +1027,14 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry, >>> f2fs_unlock_op(sbi); >>> if (new_page) >>> f2fs_put_page(new_page, 0); >>> -out_whiteout: >>> - if (whiteout) >>> - iput(whiteout); >>> out_dir: >>> if (old_dir_entry) >>> f2fs_put_page(old_dir_page, 0); >>> out_old: >>> f2fs_put_page(old_page, 0); >>> out: >>> + if (whiteout) >>> + iput(whiteout); >>> return err; >>> } >>> >>> > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2019-12-11 1:23 UTC|newest] Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-09 22:23 [PATCH 1/6] f2fs: call f2fs_balance_fs outside of locked page Jaegeuk Kim 2019-12-09 22:23 ` [f2fs-dev] " Jaegeuk Kim 2019-12-09 22:23 ` [PATCH 2/6] f2fs: declare nested quota_sem and remove unnecessary sems Jaegeuk Kim 2019-12-09 22:23 ` [f2fs-dev] " Jaegeuk Kim 2019-12-10 6:20 ` Chao Yu 2019-12-10 6:20 ` Chao Yu 2019-12-09 22:23 ` [PATCH 3/6] f2fs: keep quota data on write_begin failure Jaegeuk Kim 2019-12-09 22:23 ` [f2fs-dev] " Jaegeuk Kim 2019-12-10 6:22 ` Chao Yu 2019-12-10 6:22 ` Chao Yu 2019-12-09 22:23 ` [PATCH 4/6] f2fs: should avoid recursive filesystem ops Jaegeuk Kim 2019-12-09 22:23 ` [f2fs-dev] " Jaegeuk Kim 2019-12-10 6:22 ` Chao Yu 2019-12-10 6:22 ` Chao Yu 2019-12-09 22:23 ` [PATCH 5/6] f2fs: set GFP_NOFS when moving inline dentries Jaegeuk Kim 2019-12-09 22:23 ` [f2fs-dev] " Jaegeuk Kim 2019-12-10 6:22 ` Chao Yu 2019-12-10 6:22 ` Chao Yu 2019-12-09 22:23 ` [PATCH 6/6] f2fs: set I_LINKABLE early to avoid wrong access by vfs Jaegeuk Kim 2019-12-09 22:23 ` [f2fs-dev] " Jaegeuk Kim 2019-12-10 6:37 ` Chao Yu 2019-12-10 6:37 ` Chao Yu 2019-12-11 1:21 ` Jaegeuk Kim 2019-12-11 1:21 ` Jaegeuk Kim 2019-12-11 1:23 ` Chao Yu [this message] 2019-12-11 1:23 ` Chao Yu 2019-12-11 1:31 ` Jaegeuk Kim 2019-12-11 1:31 ` Jaegeuk Kim 2019-12-11 1:42 ` Chao Yu 2019-12-11 1:42 ` Chao Yu 2019-12-12 16:55 ` Jaegeuk Kim 2019-12-12 16:55 ` Jaegeuk Kim 2019-12-10 2:09 ` [f2fs-dev] [PATCH 1/6] f2fs: call f2fs_balance_fs outside of locked page Chao Yu 2019-12-10 2:09 ` Chao Yu 2020-02-22 4:46 ` Ondřej Jirman 2020-02-22 4:46 ` [f2fs-dev] " Ondřej Jirman 2020-02-22 18:17 ` Writes stoped working on f2fs after the compression support was added Ondřej Jirman 2020-02-22 18:17 ` [f2fs-dev] " Ondřej Jirman 2020-02-24 10:37 ` Chao Yu 2020-02-24 10:37 ` [f2fs-dev] " Chao Yu 2020-02-24 10:41 ` Chao Yu 2020-02-24 10:41 ` Chao Yu 2020-02-24 13:58 ` Ondřej Jirman 2020-02-24 13:58 ` Ondřej Jirman 2020-02-24 14:03 ` Ondřej Jirman 2020-02-24 14:03 ` Ondřej Jirman 2020-02-24 14:31 ` Ondřej Jirman 2020-02-24 14:31 ` Ondřej Jirman 2020-02-25 11:24 ` Chao Yu 2020-02-25 11:24 ` Chao Yu 2020-02-25 11:32 ` Chao Yu 2020-02-25 11:32 ` Chao Yu 2020-02-25 12:08 ` Ondřej Jirman 2020-02-25 12:08 ` Ondřej Jirman 2020-02-25 12:27 ` Ondřej Jirman 2020-02-25 12:27 ` Ondřej Jirman 2020-02-26 1:58 ` Chao Yu 2020-02-26 1:58 ` Chao Yu 2020-02-26 12:11 ` Ondřej Jirman 2020-02-26 12:11 ` Ondřej Jirman 2020-02-26 18:05 ` Ondřej Jirman 2020-02-26 18:05 ` Ondřej Jirman 2020-02-27 2:01 ` Chao Yu 2020-02-27 2:01 ` Chao Yu 2020-03-06 12:02 ` Ondřej Jirman 2020-03-06 12:02 ` Ondřej Jirman 2020-03-06 12:43 ` Ondřej Jirman 2020-03-06 12:43 ` Ondřej Jirman 2020-03-11 9:02 ` Chao Yu 2020-03-11 9:02 ` Chao Yu 2020-03-11 10:33 ` Ondřej Jirman 2020-03-11 10:33 ` Ondřej Jirman 2020-03-11 10:51 ` Chao Yu 2020-03-11 10:51 ` Chao Yu 2020-03-11 11:01 ` Ondřej Jirman 2020-03-11 11:01 ` Ondřej Jirman 2020-03-11 12:13 ` Chao Yu 2020-03-11 17:01 ` Jaegeuk Kim 2020-03-11 17:01 ` Jaegeuk Kim 2020-03-22 10:15 ` Chao Yu 2020-03-22 10:15 ` Chao Yu 2020-02-24 14:20 ` Ondřej Jirman 2020-02-24 14:20 ` Ondřej Jirman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=00ced682-9522-236d-4078-4c8f2e348d39@huawei.com \ --to=yuchao0@huawei.com \ --cc=jaegeuk@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.