2024-02-06 03:26:47

by Chao Yu

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

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 <[email protected]>
---
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

2024-02-08 00:21:08

by Jaegeuk Kim

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

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 <[email protected]>
> ---
> 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

2024-02-19 03:13:32

by Chao Yu

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

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 <[email protected]>
>> ---
>> 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

2024-02-26 08:11:09

by Chao Yu

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

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 <[email protected]>
>>> ---
>>>   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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-03-12 01:52:45

by Chao Yu

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

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 <[email protected]>
>>>> ---
>>>>   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
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

2024-03-13 01:42:51

by Jaegeuk Kim

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

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 <[email protected]>
> > > > > ---
> > > > > ? 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
> > > [email protected]
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel