diff mbox series

f2fs: fix to return EIO when reading after device removal

Message ID 20240206032513.2495025-1-chao@kernel.org
State New
Headers
Series f2fs: fix to return EIO when reading after device removal |

Commit Message

Chao Yu Feb. 6, 2024, 3:25 a.m. UTC 
  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(+)

Comments

Jaegeuk Kim Feb. 8, 2024, 12:18 a.m. UTC | #1 
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
Chao Yu Feb. 19, 2024, 3:13 a.m. UTC | #2 
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
Chao Yu Feb. 26, 2024, 8 a.m. UTC | #3 
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
diff mbox series

Patch

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;