[PATCH] erofs: mark fileio folios uptodate based on the number of bytes read

[PATCH] erofs: mark fileio folios uptodate based on the number of bytes read

[Sheng Yong]

From: Sheng Yong <shengyong1@xiaomi.com>

For file-backed mount, IO requests are handled by vfs_iocb_iter_read().
However, it can be interrupted by SIGKILL, returning the number of
bytes actually copied. Although unused folios are zero filled, they
are unexpectedly marked as uptodate.
This patch addresses this by setting folios uptodate based on the actual
number of bytes read for the plain backing file. And for the compressed
backing file, there may not have sufficient data for decompression,
in such case, the bio is marked with an error directly.

Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
Reported-by: chenguanyou <chenguanyou@xiaomi.com>
Signed-off-by: Yunlei He <heyunlei@xiaomi.com>
Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>
---
 fs/erofs/fileio.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
index abe873f01297..172444ae4ede 100644
--- a/fs/erofs/fileio.c
+++ b/fs/erofs/fileio.c
@@ -24,18 +24,30 @@ static void erofs_fileio_ki_complete(struct kiocb *iocb, long ret)
 	struct erofs_fileio_rq *rq =
 			container_of(iocb, struct erofs_fileio_rq, iocb);
 	struct folio_iter fi;
+	bool bio_advanced = false;
 
 	if (ret >= 0 && ret != rq->bio.bi_iter.bi_size) {
 		bio_advance(&rq->bio, ret);
 		zero_fill_bio(&rq->bio);
+		bio_advanced = true;
 	}
 	if (!rq->bio.bi_end_io) {
 		bio_for_each_folio_all(fi, &rq->bio) {
 			DBG_BUGON(folio_test_uptodate(fi.folio));
-			erofs_onlinefolio_end(fi.folio, ret < 0, false);
+			if (likely(!bio_advanced ||
+				   ret >= (long)folio_size(fi.folio))) {
+				erofs_onlinefolio_end(fi.folio, 0, false);
+				ret -= folio_size(fi.folio);
+			} else {
+				erofs_onlinefolio_end(fi.folio, -EIO, false);
+			}
 		}
 	} else if (ret < 0 && !rq->bio.bi_status) {
 		rq->bio.bi_status = errno_to_blk_status(ret);
+	} else if (bio_advanced &&
+		   ret + iocb->ki_pos < i_size_read(file_inode(iocb->ki_filp))) {
+		/* may not have sufficient data for decompression */
+		rq->bio.bi_status = -EIO;
 	}
 	bio_endio(&rq->bio);
 	bio_uninit(&rq->bio);
-- 
2.43.0

Re: [PATCH] erofs: mark fileio folios uptodate based on the number of bytes read

[Gao Xiang]

Hi Yong,

On 2026/2/26 17:09, Sheng Yong wrote:
> From: Sheng Yong <shengyong1@xiaomi.com>
> 
> For file-backed mount, IO requests are handled by vfs_iocb_iter_read().
> However, it can be interrupted by SIGKILL, returning the number of
> bytes actually copied. Although unused folios are zero filled, they
> are unexpectedly marked as uptodate.
> This patch addresses this by setting folios uptodate based on the actual
> number of bytes read for the plain backing file. And for the compressed
> backing file, there may not have sufficient data for decompression,
> in such case, the bio is marked with an error directly.
> 
> Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
> Reported-by: chenguanyou <chenguanyou@xiaomi.com>
> Signed-off-by: Yunlei He <heyunlei@xiaomi.com>
> Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>

Yes, it sounds possible. But can we just fail the
whole I/O for both cases?

In principle, we should retry the remaining I/O once more
for short read, but failing the whole I/O could be one
short-term solution.

Thanks,
Gao Xiang

Re: [PATCH] erofs: mark fileio folios uptodate based on the number of bytes read

[Gao Xiang]

On 2026/2/26 17:28, Gao Xiang wrote:
> Hi Yong,
> 
> On 2026/2/26 17:09, Sheng Yong wrote:
>> From: Sheng Yong <shengyong1@xiaomi.com>
>>
>> For file-backed mount, IO requests are handled by vfs_iocb_iter_read().
>> However, it can be interrupted by SIGKILL, returning the number of
>> bytes actually copied. Although unused folios are zero filled, they
>> are unexpectedly marked as uptodate.
>> This patch addresses this by setting folios uptodate based on the actual
>> number of bytes read for the plain backing file. And for the compressed
>> backing file, there may not have sufficient data for decompression,
>> in such case, the bio is marked with an error directly.
>>
>> Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
>> Reported-by: chenguanyou <chenguanyou@xiaomi.com>
>> Signed-off-by: Yunlei He <heyunlei@xiaomi.com>
>> Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>
> 
> Yes, it sounds possible. But can we just fail the
> whole I/O for both cases?
> 
> In principle, we should retry the remaining I/O once more
> for short read, but failing the whole I/O could be one
> short-term solution.

I wonder if we should simply:

diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
index abe873f01297..98cdaa1cd1a7 100644
--- a/fs/erofs/fileio.c
+++ b/fs/erofs/fileio.c
@@ -25,10 +25,8 @@ static void erofs_fileio_ki_complete(struct kiocb *iocb, long ret)
                         container_of(iocb, struct erofs_fileio_rq, iocb);
         struct folio_iter fi;

-       if (ret >= 0 && ret != rq->bio.bi_iter.bi_size) {
-               bio_advance(&rq->bio, ret);
-               zero_fill_bio(&rq->bio);
-       }
+       if (ret >= 0 && ret != rq->bio.bi_iter.bi_size)
+               ret = -EIO;

instead. IOWs, filling zeros means nothing for us.

Thanks,
Gao Xiang

> 
> Thanks,
> Gao Xiang
> 

Re: [PATCH] erofs: mark fileio folios uptodate based on the number of bytes read

[Sheng Yong]

On 2/26/26 17:36, Gao Xiang wrote:
> 
> 
> On 2026/2/26 17:28, Gao Xiang wrote:
>> Hi Yong,
>>
>> On 2026/2/26 17:09, Sheng Yong wrote:
>>> From: Sheng Yong <shengyong1@xiaomi.com>
>>>
>>> For file-backed mount, IO requests are handled by vfs_iocb_iter_read().
>>> However, it can be interrupted by SIGKILL, returning the number of
>>> bytes actually copied. Although unused folios are zero filled, they
>>> are unexpectedly marked as uptodate.
>>> This patch addresses this by setting folios uptodate based on the actual
>>> number of bytes read for the plain backing file. And for the compressed
>>> backing file, there may not have sufficient data for decompression,
>>> in such case, the bio is marked with an error directly.
>>>
>>> Fixes: ce63cb62d794 ("erofs: support unencoded inodes for fileio")
>>> Reported-by: chenguanyou <chenguanyou@xiaomi.com>
>>> Signed-off-by: Yunlei He <heyunlei@xiaomi.com>
>>> Signed-off-by: Sheng Yong <shengyong1@xiaomi.com>
>>
>> Yes, it sounds possible. But can we just fail the
>> whole I/O for both cases?
>>
>> In principle, we should retry the remaining I/O once more
>> for short read, but failing the whole I/O could be one
>> short-term solution.
> 
> I wonder if we should simply:
> 
> diff --git a/fs/erofs/fileio.c b/fs/erofs/fileio.c
> index abe873f01297..98cdaa1cd1a7 100644
> --- a/fs/erofs/fileio.c
> +++ b/fs/erofs/fileio.c
> @@ -25,10 +25,8 @@ static void erofs_fileio_ki_complete(struct kiocb *iocb, long ret)
>                          container_of(iocb, struct erofs_fileio_rq, iocb);
>          struct folio_iter fi;
> 
> -       if (ret >= 0 && ret != rq->bio.bi_iter.bi_size) {
> -               bio_advance(&rq->bio, ret);
> -               zero_fill_bio(&rq->bio);
> -       }
> +       if (ret >= 0 && ret != rq->bio.bi_iter.bi_size)
> +               ret = -EIO;
> 
> instead. IOWs, filling zeros means nothing for us.

That makes sense. I'll send a v2.

thanks,
shengyong
> 
> Thanks,
> Gao Xiang
> 
>>
>> Thanks,
>> Gao Xiang
>>
>