[PATCH] f2fs: fix to return EIO when reading after device removal

[PATCH] f2fs: fix to return EIO when reading after device removal

[Chao Yu]

generic/730 2s ... - output mismatch (see /media/fstests/results//generic/730.out.bad)
    --- tests/generic/730.out	2023-08-07 01:39:51.055568499 +0000
    +++ /media/fstests/results//generic/730.out.bad	2024-02-06 02:26:43.000000000 +0000
    @@ -1,2 +1 @@
     QA output created by 730
    -cat: -: Input/output error
    ...
    (Run 'diff -u /media/fstests/tests/generic/730.out /media/fstests/results//generic/730.out.bad'  to see the entire diff)
Ran: generic/730
Failures: generic/730
Failed 1 of 1 tests

This patch adds a check condition in f2fs_file_read_iter() to
detect cp_error status after device removal, and retrurn -EIO
for such case.

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/file.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 45b7e3610b0f..9e4386d4144c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4462,6 +4462,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	const loff_t pos = iocb->ki_pos;
 	ssize_t ret;
 
+	if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
+		return -EIO;
+
 	if (!f2fs_is_compress_backend_ready(inode))
 		return -EOPNOTSUPP;
 
-- 
2.40.1

Re: [PATCH] f2fs: fix to return EIO when reading after device removal

[Jaegeuk Kim]

On 02/06, Chao Yu wrote:
> generic/730 2s ... - output mismatch (see /media/fstests/results//generic/730.out.bad)
>     --- tests/generic/730.out	2023-08-07 01:39:51.055568499 +0000
>     +++ /media/fstests/results//generic/730.out.bad	2024-02-06 02:26:43.000000000 +0000
>     @@ -1,2 +1 @@
>      QA output created by 730
>     -cat: -: Input/output error
>     ...
>     (Run 'diff -u /media/fstests/tests/generic/730.out /media/fstests/results//generic/730.out.bad'  to see the entire diff)
> Ran: generic/730
> Failures: generic/730
> Failed 1 of 1 tests
> 
> This patch adds a check condition in f2fs_file_read_iter() to
> detect cp_error status after device removal, and retrurn -EIO
> for such case.

Can we check device removal?

> 
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/file.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 45b7e3610b0f..9e4386d4144c 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -4462,6 +4462,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>  	const loff_t pos = iocb->ki_pos;
>  	ssize_t ret;
>  
> +	if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
> +		return -EIO;
> +
>  	if (!f2fs_is_compress_backend_ready(inode))
>  		return -EOPNOTSUPP;
>  
> -- 
> 2.40.1

Re: [PATCH] f2fs: fix to return EIO when reading after device removal

[Chao Yu]

On 2024/2/8 8:18, Jaegeuk Kim wrote:
> On 02/06, Chao Yu wrote:
>> generic/730 2s ... - output mismatch (see /media/fstests/results//generic/730.out.bad)
>>      --- tests/generic/730.out	2023-08-07 01:39:51.055568499 +0000
>>      +++ /media/fstests/results//generic/730.out.bad	2024-02-06 02:26:43.000000000 +0000
>>      @@ -1,2 +1 @@
>>       QA output created by 730
>>      -cat: -: Input/output error
>>      ...
>>      (Run 'diff -u /media/fstests/tests/generic/730.out /media/fstests/results//generic/730.out.bad'  to see the entire diff)
>> Ran: generic/730
>> Failures: generic/730
>> Failed 1 of 1 tests
>>
>> This patch adds a check condition in f2fs_file_read_iter() to
>> detect cp_error status after device removal, and retrurn -EIO
>> for such case.
> 
> Can we check device removal?

We can use blk_queue_dying() to detect device removal, but, IMO, device
removal can almost not happen in Android, not sure for distros or server,
do you want to add this check condition into f2fs_cp_error()?

Thanks,

> 
>>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
>>   fs/f2fs/file.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 45b7e3610b0f..9e4386d4144c 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -4462,6 +4462,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>>   	const loff_t pos = iocb->ki_pos;
>>   	ssize_t ret;
>>   
>> +	if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
>> +		return -EIO;
>> +
>>   	if (!f2fs_is_compress_backend_ready(inode))
>>   		return -EOPNOTSUPP;
>>   
>> -- 
>> 2.40.1

Re: [f2fs-dev] [PATCH] f2fs: fix to return EIO when reading after device removal

[Chao Yu]

Any comments?

Thanks,

On 2024/2/19 11:13, Chao Yu wrote:
> On 2024/2/8 8:18, Jaegeuk Kim wrote:
>> On 02/06, Chao Yu wrote:
>>> generic/730 2s ... - output mismatch (see /media/fstests/results//generic/730.out.bad)
>>>      --- tests/generic/730.out    2023-08-07 01:39:51.055568499 +0000
>>>      +++ /media/fstests/results//generic/730.out.bad    2024-02-06 02:26:43.000000000 +0000
>>>      @@ -1,2 +1 @@
>>>       QA output created by 730
>>>      -cat: -: Input/output error
>>>      ...
>>>      (Run 'diff -u /media/fstests/tests/generic/730.out /media/fstests/results//generic/730.out.bad'  to see the entire diff)
>>> Ran: generic/730
>>> Failures: generic/730
>>> Failed 1 of 1 tests
>>>
>>> This patch adds a check condition in f2fs_file_read_iter() to
>>> detect cp_error status after device removal, and retrurn -EIO
>>> for such case.
>>
>> Can we check device removal?
> 
> We can use blk_queue_dying() to detect device removal, but, IMO, device
> removal can almost not happen in Android, not sure for distros or server,
> do you want to add this check condition into f2fs_cp_error()?
> 
> Thanks,
> 
>>
>>>
>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>> ---
>>>   fs/f2fs/file.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 45b7e3610b0f..9e4386d4144c 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -4462,6 +4462,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>>>       const loff_t pos = iocb->ki_pos;
>>>       ssize_t ret;
>>> +    if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
>>> +        return -EIO;
>>> +
>>>       if (!f2fs_is_compress_backend_ready(inode))
>>>           return -EOPNOTSUPP;
>>> -- 
>>> 2.40.1
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix to return EIO when reading after device removal

[Chao Yu]

Ping,

Jaegeuk, do you have any comments on this patch?

Thanks,

On 2024/2/26 16:00, Chao Yu wrote:
> Any comments?
> 
> Thanks,
> 
> On 2024/2/19 11:13, Chao Yu wrote:
>> On 2024/2/8 8:18, Jaegeuk Kim wrote:
>>> On 02/06, Chao Yu wrote:
>>>> generic/730 2s ... - output mismatch (see /media/fstests/results//generic/730.out.bad)
>>>>      --- tests/generic/730.out    2023-08-07 01:39:51.055568499 +0000
>>>>      +++ /media/fstests/results//generic/730.out.bad    2024-02-06 02:26:43.000000000 +0000
>>>>      @@ -1,2 +1 @@
>>>>       QA output created by 730
>>>>      -cat: -: Input/output error
>>>>      ...
>>>>      (Run 'diff -u /media/fstests/tests/generic/730.out /media/fstests/results//generic/730.out.bad'  to see the entire diff)
>>>> Ran: generic/730
>>>> Failures: generic/730
>>>> Failed 1 of 1 tests
>>>>
>>>> This patch adds a check condition in f2fs_file_read_iter() to
>>>> detect cp_error status after device removal, and retrurn -EIO
>>>> for such case.
>>>
>>> Can we check device removal?
>>
>> We can use blk_queue_dying() to detect device removal, but, IMO, device
>> removal can almost not happen in Android, not sure for distros or server,
>> do you want to add this check condition into f2fs_cp_error()?
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>>   fs/f2fs/file.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 45b7e3610b0f..9e4386d4144c 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -4462,6 +4462,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>>>>       const loff_t pos = iocb->ki_pos;
>>>>       ssize_t ret;
>>>> +    if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
>>>> +        return -EIO;
>>>> +
>>>>       if (!f2fs_is_compress_backend_ready(inode))
>>>>           return -EOPNOTSUPP;
>>>> -- 
>>>> 2.40.1
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix to return EIO when reading after device removal

[Jaegeuk Kim]

So, I was wondering we can give data even in cp_error case.

On 03/12, Chao Yu wrote:
> Ping,
> 
> Jaegeuk, do you have any comments on this patch?
> 
> Thanks,
> 
> On 2024/2/26 16:00, Chao Yu wrote:
> > Any comments?
> > 
> > Thanks,
> > 
> > On 2024/2/19 11:13, Chao Yu wrote:
> > > On 2024/2/8 8:18, Jaegeuk Kim wrote:
> > > > On 02/06, Chao Yu wrote:
> > > > > generic/730 2s ... - output mismatch (see /media/fstests/results//generic/730.out.bad)
> > > > >      --- tests/generic/730.out    2023-08-07 01:39:51.055568499 +0000
> > > > >      +++ /media/fstests/results//generic/730.out.bad    2024-02-06 02:26:43.000000000 +0000
> > > > >      @@ -1,2 +1 @@
> > > > >       QA output created by 730
> > > > >      -cat: -: Input/output error
> > > > >      ...
> > > > >      (Run 'diff -u /media/fstests/tests/generic/730.out /media/fstests/results//generic/730.out.bad'  to see the entire diff)
> > > > > Ran: generic/730
> > > > > Failures: generic/730
> > > > > Failed 1 of 1 tests
> > > > > 
> > > > > This patch adds a check condition in f2fs_file_read_iter() to
> > > > > detect cp_error status after device removal, and retrurn -EIO
> > > > > for such case.
> > > > 
> > > > Can we check device removal?
> > > 
> > > We can use blk_queue_dying() to detect device removal, but, IMO, device
> > > removal can almost not happen in Android, not sure for distros or server,
> > > do you want to add this check condition into f2fs_cp_error()?
> > > 
> > > Thanks,
> > > 
> > > > 
> > > > > 
> > > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > > ---
> > > > >   fs/f2fs/file.c | 3 +++
> > > > >   1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > > > index 45b7e3610b0f..9e4386d4144c 100644
> > > > > --- a/fs/f2fs/file.c
> > > > > +++ b/fs/f2fs/file.c
> > > > > @@ -4462,6 +4462,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> > > > >       const loff_t pos = iocb->ki_pos;
> > > > >       ssize_t ret;
> > > > > +    if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
> > > > > +        return -EIO;
> > > > > +
> > > > >       if (!f2fs_is_compress_backend_ready(inode))
> > > > >           return -EOPNOTSUPP;
> > > > > -- 
> > > > > 2.40.1
> > > 
> > > 
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel