All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: forbid to do fstrim if fs has some error
@ 2016-09-01  2:14 Yunlei He
  2016-09-07 13:24 ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Yunlei He @ 2016-09-01  2:14 UTC (permalink / raw)
  To: linux-f2fs-devel, jaegeuk, yuchao0; +Cc: heyunlei

This patch skip fstrim if sbi set SBI_NEED_FSCK flag

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/segment.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 93c5e26..1b62213 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1353,6 +1353,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
 	if (end <= MAIN_BLKADDR(sbi))
 		goto out;
 
+	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
+		goto out;
+
 	/* start/end segment number in main_area */
 	start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start);
 	end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
-- 
1.9.1


------------------------------------------------------------------------------

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: forbid to do fstrim if fs has some error
  2016-09-01  2:14 [PATCH] f2fs: forbid to do fstrim if fs has some error Yunlei He
@ 2016-09-07 13:24 ` Chao Yu
  2016-09-08  0:02   ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2016-09-07 13:24 UTC (permalink / raw)
  To: Yunlei He, linux-f2fs-devel, jaegeuk, yuchao0; +Cc: heyunlei

Hi Yunlei,

On 2016/9/1 10:14, Yunlei He wrote:
> This patch skip fstrim if sbi set SBI_NEED_FSCK flag
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/segment.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 93c5e26..1b62213 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1353,6 +1353,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
>  	if (end <= MAIN_BLKADDR(sbi))
>  		goto out;
>  
> +	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> +		goto out;

IMO, it's better to check this condition in f2fs_ioc_fstrim, and return error
number to user, such as EIO?

Thanks,

> +
>  	/* start/end segment number in main_area */
>  	start_segno = (start <= MAIN_BLKADDR(sbi)) ? 0 : GET_SEGNO(sbi, start);
>  	end_segno = (end >= MAX_BLKADDR(sbi)) ? MAIN_SEGS(sbi) - 1 :
> 

------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: forbid to do fstrim if fs has some error
  2016-09-07 13:24 ` Chao Yu
@ 2016-09-08  0:02   ` Jaegeuk Kim
  2016-09-08 16:02     ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Jaegeuk Kim @ 2016-09-08  0:02 UTC (permalink / raw)
  To: Chao Yu; +Cc: heyunlei, linux-f2fs-devel

On Wed, Sep 07, 2016 at 09:24:31PM +0800, Chao Yu wrote:
> Hi Yunlei,
> 
> On 2016/9/1 10:14, Yunlei He wrote:
> > This patch skip fstrim if sbi set SBI_NEED_FSCK flag
> > 
> > Signed-off-by: Yunlei He <heyunlei@huawei.com>
> > ---
> >  fs/f2fs/segment.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 93c5e26..1b62213 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1353,6 +1353,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
> >  	if (end <= MAIN_BLKADDR(sbi))
> >  		goto out;
> >  
> > +	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> > +		goto out;
> 
> IMO, it's better to check this condition in f2fs_ioc_fstrim, and return error
> number to user, such as EIO?

I don't think it's an error case such as EIO, ENOMEM, EINVAL, and so on.
Instead, It'd be okay to return trimmed size with zero.
Oh, we'd better give a message to notify like this?

	f2fs_msg(sbi->sb, KERN_WARNING,
		"Found FS corruption, run fsck to fix.");

Thanks,

------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: forbid to do fstrim if fs has some error
  2016-09-08  0:02   ` Jaegeuk Kim
@ 2016-09-08 16:02     ` Chao Yu
  2016-09-09  0:45       ` heyunlei
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2016-09-08 16:02 UTC (permalink / raw)
  To: Jaegeuk Kim, Yunlei He; +Cc: heyunlei, linux-f2fs-devel

On 2016/9/8 8:02, Jaegeuk Kim wrote:
> On Wed, Sep 07, 2016 at 09:24:31PM +0800, Chao Yu wrote:
>> Hi Yunlei,
>>
>> On 2016/9/1 10:14, Yunlei He wrote:
>>> This patch skip fstrim if sbi set SBI_NEED_FSCK flag
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/segment.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 93c5e26..1b62213 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1353,6 +1353,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
>>>  	if (end <= MAIN_BLKADDR(sbi))
>>>  		goto out;
>>>  
>>> +	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
>>> +		goto out;
>>
>> IMO, it's better to check this condition in f2fs_ioc_fstrim, and return error
>> number to user, such as EIO?
> 
> I don't think it's an error case such as EIO, ENOMEM, EINVAL, and so on.
> Instead, It'd be okay to return trimmed size with zero.
> Oh, we'd better give a message to notify like this?
> 
> 	f2fs_msg(sbi->sb, KERN_WARNING,
> 		"Found FS corruption, run fsck to fix.");

Hmm.. leave some dmesg looks okay to me.

So, Yunlei, how do you think of it?

Thanks,

> 
> Thanks,
> 

------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: forbid to do fstrim if fs has some error
  2016-09-08 16:02     ` Chao Yu
@ 2016-09-09  0:45       ` heyunlei
  0 siblings, 0 replies; 5+ messages in thread
From: heyunlei @ 2016-09-09  0:45 UTC (permalink / raw)
  To: Chao Yu, Jaegeuk Kim; +Cc: heyunlei, linux-f2fs-devel



On 2016/9/9 0:02, Chao Yu wrote:
> On 2016/9/8 8:02, Jaegeuk Kim wrote:
>> On Wed, Sep 07, 2016 at 09:24:31PM +0800, Chao Yu wrote:
>>> Hi Yunlei,
Hi all,
>>>
>>> On 2016/9/1 10:14, Yunlei He wrote:
>>>> This patch skip fstrim if sbi set SBI_NEED_FSCK flag
>>>>
>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>> ---
>>>>  fs/f2fs/segment.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 93c5e26..1b62213 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -1353,6 +1353,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range)
>>>>  	if (end <= MAIN_BLKADDR(sbi))
>>>>  		goto out;
>>>>
>>>> +	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
>>>> +		goto out;
>>>
>>> IMO, it's better to check this condition in f2fs_ioc_fstrim, and return error
>>> number to user, such as EIO?
>>
>> I don't think it's an error case such as EIO, ENOMEM, EINVAL, and so on.
>> Instead, It'd be okay to return trimmed size with zero.
>> Oh, we'd better give a message to notify like this?
>>
>> 	f2fs_msg(sbi->sb, KERN_WARNING,
>> 		"Found FS corruption, run fsck to fix.");
>
> Hmm.. leave some dmesg looks okay to me.
>
> So, Yunlei, how do you think of it?

Yes, it looks good to me, which make debug is easy when we do fstrim to discard
some invalid blocks, but zero is return.

Thanks,
>
> Thanks,
>
>>
>> Thanks,
>>
>
> .
>


------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-09  0:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-01  2:14 [PATCH] f2fs: forbid to do fstrim if fs has some error Yunlei He
2016-09-07 13:24 ` Chao Yu
2016-09-08  0:02   ` Jaegeuk Kim
2016-09-08 16:02     ` Chao Yu
2016-09-09  0:45       ` heyunlei

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.