linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
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 3/3] f2fs: fix to cover meta flush with cp_lock
Date: Fri, 29 May 2020 14:37:02 +0800	[thread overview]
Message-ID: <9fa465a0-70b5-7d17-baf4-21224196e2bc@huawei.com> (raw)
In-Reply-To: <20200528190010.GA162605@google.com>

On 2020/5/29 3:00, Jaegeuk Kim wrote:
> On 05/28, Chao Yu wrote:
>> On 2020/5/28 9:26, Jaegeuk Kim wrote:
>>> On 05/28, Chao Yu wrote:
>>>> On 2020/5/28 5:02, Jaegeuk Kim wrote:
>>>>> On 05/27, Chao Yu wrote:
>>>>>> meta inode page should be flushed under cp_lock, fix it.
>>>>>
>>>>> It doesn't matter for this case, yes?
>>>>
>>>> It's not related to discard issue.
>>>
>>> I meant we really need this or not. :P
>>
>> Yes, let's keep that rule: flush meta pages under cp_lock, otherwise
>> checkpoint flush order may be broken due to race, right? as checkpoint
>> should write 2rd cp park page after flushing all meta pages.
> 
> Well, this is for shutdown test, and thus we don't need to sync up here.

I'm a little worried about race condition:

f2fs_write_checkpoint
 do_checkpoint
  ...
					shutdown
					f2fs_sync_meta_pages
					stop_checkpoint
  ...

Though, I haven't figure out potential damage of their racing.

> 
>>
>>>
>>>>
>>>> Now, I got some progress, I can reproduce that bug occasionally.
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>>> ---
>>>>>>  fs/f2fs/file.c | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index f7de2a1da528..0fcae4d90074 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
>>>>>>  		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
>>>>>>  		break;
>>>>>>  	case F2FS_GOING_DOWN_METAFLUSH:
>>>>>> +		mutex_lock(&sbi->cp_mutex);
>>>>>>  		f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO);
>>>>>> +		mutex_unlock(&sbi->cp_mutex);
>>>>>>  		f2fs_stop_checkpoint(sbi, false);
>>>>>>  		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
>>>>>>  		break;
>>>>>> -- 
>>>>>> 2.18.0.rc1
>>>>> .
>>>>>
>>> .
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

      reply	other threads:[~2020-05-29  6:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27 10:27 [f2fs-dev] [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock Chao Yu
2020-05-27 21:02   ` Jaegeuk Kim
2020-05-28  1:23     ` Chao Yu
2020-05-28  1:26       ` Jaegeuk Kim
2020-05-28  1:47         ` Chao Yu
2020-05-28 19:00           ` Jaegeuk Kim
2020-05-29  6:37             ` Chao Yu [this message]

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=9fa465a0-70b5-7d17-baf4-21224196e2bc@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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).