diff mbox series

[3/3] f2fs: fix to f2fs_handle_critical_error when errors=remount-ro

Message ID 20221027120807.6337-3-frank.li@vivo.com
State New
Headers
Series [1/3] f2fs: rename flush_error_work() to f2fs_record_error_work() |

Commit Message

李扬韬 Oct. 27, 2022, 12:08 p.m. UTC 
  When a fatal error occurs in the file system and it is
remounted to ro mode, the flush thread needs to be stopped.

Fixes: d81ab30e85a5 ("f2fs: support errors=remount-ro|continue|panic mountoption")
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/f2fs/super.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Chao Yu Oct. 27, 2022, 1:31 p.m. UTC | #1 
On 2022/10/27 20:08, Yangtao Li wrote:
> When a fatal error occurs in the file system and it is
> remounted to ro mode, the flush thread needs to be stopped.
> 
> Fixes: d81ab30e85a5 ("f2fs: support errors=remount-ro|continue|panic mountoption")

Do you mind letting me merge these two patches into original patch?
since original patch is still in dev branch, rather than mainline.

I guess it needs to stop ckpt thread as well...

Thanks,

> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/f2fs/super.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index fc220b5c5599..3a1238a82dc9 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4017,6 +4017,7 @@ void f2fs_handle_critical_error(struct f2fs_sb_info *sbi, unsigned char reason,
>   
>   	f2fs_stop_gc_thread(sbi);
>   	f2fs_stop_discard_thread(sbi);
> +	f2fs_stop_flush_thread(sbi);
>   }
>   
>   static void f2fs_record_error_work(struct work_struct *work)
李扬韬 Oct. 28, 2022, 2:33 a.m. UTC | #2 
> It won't pollute global namespace since it's a static function just be used in f2fs/super.c...

emm, I think it would be nice to see the f2fs_record_error_work symbol
in the stack, it can be explicitly a function of f2fs. personal opinion for reference only.

> Do you mind letting me merge these two patches into original patch?
> since original patch is still in dev branch, rather than mainline.

Glad to see, if resend.

> I guess it needs to stop ckpt thread as well...

agree, :)

Thanks,
Chao Yu Oct. 28, 2022, 2:43 a.m. UTC | #3 
On 2022/10/28 10:33, Yangtao Li wrote:
>> It won't pollute global namespace since it's a static function just be used in f2fs/super.c...
> 
> emm, I think it would be nice to see the f2fs_record_error_work symbol
> in the stack, it can be explicitly a function of f2fs. personal opinion for reference only.

Oh, yes, let me update in original patch as well.

> 
>> Do you mind letting me merge these two patches into original patch?
>> since original patch is still in dev branch, rather than mainline.
> 
> Glad to see, if resend.

Thanks, :)

> 
>> I guess it needs to stop ckpt thread as well...
> 
> agree, :)
> 
> Thanks,
Jaegeuk Kim Oct. 28, 2022, 3:17 a.m. UTC | #4 
On 10/28, Chao Yu wrote:
> On 2022/10/28 10:33, Yangtao Li wrote:
> > > It won't pollute global namespace since it's a static function just be used in f2fs/super.c...
> > 
> > emm, I think it would be nice to see the f2fs_record_error_work symbol
> > in the stack, it can be explicitly a function of f2fs. personal opinion for reference only.
> 
> Oh, yes, let me update in original patch as well.

Chao,

It seems there're multiple bugs. Please apply this as well.

https://lore.kernel.org/linux-f2fs-devel/20221027180416.3786792-1-jaegeuk@kernel.org/T/#u

> 
> > 
> > > Do you mind letting me merge these two patches into original patch?
> > > since original patch is still in dev branch, rather than mainline.
> > 
> > Glad to see, if resend.
> 
> Thanks, :)
> 
> > 
> > > I guess it needs to stop ckpt thread as well...
> > 
> > agree, :)
> > 
> > Thanks,
Chao Yu Oct. 28, 2022, 3:38 a.m. UTC | #5 
On 2022/10/28 11:17, Jaegeuk Kim wrote:
> On 10/28, Chao Yu wrote:
>> On 2022/10/28 10:33, Yangtao Li wrote:
>>>> It won't pollute global namespace since it's a static function just be used in f2fs/super.c...
>>>
>>> emm, I think it would be nice to see the f2fs_record_error_work symbol
>>> in the stack, it can be explicitly a function of f2fs. personal opinion for reference only.
>>
>> Oh, yes, let me update in original patch as well.
> 
> Chao,
> 
> It seems there're multiple bugs. Please apply this as well.
> 
> https://lore.kernel.org/linux-f2fs-devel/20221027180416.3786792-1-jaegeuk@kernel.org/T/#u

Jaegeuk,

Thanks, let me merge all those patches and do the test.

Thanks,

> 
>>
>>>
>>>> Do you mind letting me merge these two patches into original patch?
>>>> since original patch is still in dev branch, rather than mainline.
>>>
>>> Glad to see, if resend.
>>
>> Thanks, :)
>>
>>>
>>>> I guess it needs to stop ckpt thread as well...
>>>
>>> agree, :)
>>>
>>> Thanks,
diff mbox series

Patch

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index fc220b5c5599..3a1238a82dc9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4017,6 +4017,7 @@  void f2fs_handle_critical_error(struct f2fs_sb_info *sbi, unsigned char reason,
 
 	f2fs_stop_gc_thread(sbi);
 	f2fs_stop_discard_thread(sbi);
+	f2fs_stop_flush_thread(sbi);
 }
 
 static void f2fs_record_error_work(struct work_struct *work)