[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b0a8a71e-f232-4555-9e5b-e62e21b93b5d@linux.alibaba.com>
Date: Mon, 22 Dec 2025 00:00:00 +0800
From: Gao Xiang <hsiangkao@...ux.alibaba.com>
To: Junbeom Yeom <junbeom.yeom@...sung.com>, xiang@...nel.org, chao@...nel.org
Cc: linux-erofs@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org, Jaewook Kim <jw5454.kim@...sung.com>,
Sungjong Seo <sj1557.seo@...sung.com>
Subject: Re: [PATCH v2] erofs: fix unexpected EIO under memory pressure
On 2025/12/19 20:40, Junbeom Yeom wrote:
> erofs readahead could fail with ENOMEM under the memory pressure because
> it tries to alloc_page with GFP_NOWAIT | GFP_NORETRY, while GFP_KERNEL
> for a regular read. And if readahead fails (with non-uptodate folios),
> the original request will then fall back to synchronous read, and
> `.read_folio()` should return appropriate errnos.
>
> However, in scenarios where readahead and read operations compete,
> read operation could return an unintended EIO because of an incorrect
> error propagation.
>
> To resolve this, this patch modifies the behavior so that, when the
> PCL is for read(which means pcl.besteffort is true), it attempts actual
> decompression instead of propagating the privios error except initial EIO.
>
> - Page size: 4K
> - The original size of FileA: 16K
> - Compress-ratio per PCL: 50% (Uncompressed 8K -> Compressed 4K)
> [page0, page1] [page2, page3]
> [PCL0]---------[PCL1]
>
> - functions declaration:
> . pread(fd, buf, count, offset)
> . readahead(fd, offset, count)
> - Thread A tries to read the last 4K
> - Thread B tries to do readahead 8K from 4K
> - RA, besteffort == false
> - R, besteffort == true
>
> <process A> <process B>
>
> pread(FileA, buf, 4K, 12K)
> do readahead(page3) // failed with ENOMEM
> wait_lock(page3)
> if (!uptodate(page3))
> goto do_read
> readahead(FileA, 4K, 8K)
> // Here create PCL-chain like below:
> // [null, page1] [page2, null]
> // [PCL0:RA]-----[PCL1:RA]
> ...
> do read(page3) // found [PCL1:RA] and add page3 into it,
> // and then, change PCL1 from RA to R
> ...
> // Now, PCL-chain is as below:
> // [null, page1] [page2, page3]
> // [PCL0:RA]-----[PCL1:R]
>
> // try to decompress PCL-chain...
> z_erofs_decompress_queue
> err = 0;
>
> // failed with ENOMEM, so page 1
> // only for RA will not be uptodated.
> // it's okay.
> err = decompress([PCL0:RA], err)
>
> // However, ENOMEM propagated to next
> // PCL, even though PCL is not only
> // for RA but also for R. As a result,
> // it just failed with ENOMEM without
> // trying any decompression, so page2
> // and page3 will not be uptodated.
> ** BUG HERE ** --> err = decompress([PCL1:R], err)
>
> return err as ENOMEM
> ...
> wait_lock(page3)
> if (!uptodate(page3))
> return EIO <-- Return an unexpected EIO!
> ...
>
> Fixes: 2349d2fa02db ("erofs: sunset unneeded NOFAILs")
> Cc: stable@...r.kernel.org
> Reviewed-by: Jaewook Kim <jw5454.kim@...sung.com>
> Reviewed-by: Sungjong Seo <sj1557.seo@...sung.com>
> Signed-off-by: Junbeom Yeom <junbeom.yeom@...sung.com>
Reviewed-by: Gao Xiang <hsiangkao@...ux.alibaba.com>
Thanks,
Gao Xiang
Powered by blists - more mailing lists