linux-kernel - Re: [PATCH 1/3] mpage: terminate read-ahead on read error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250817194125.921dd351332677e516cc3b53@linux-foundation.org>
Date: Sun, 17 Aug 2025 19:41:25 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Chi Zhiling <chizhiling@....com>
Cc: linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Matthew
 Wilcox <willy@...radead.org>, Namjae Jeon <linkinjeon@...nel.org>, Sungjong
 Seo <sj1557.seo@...sung.com>, Yuezhang Mo <yuezhang.mo@...y.com>, Chi
 Zhiling <chizhiling@...inos.cn>
Subject: Re: [PATCH 1/3] mpage: terminate read-ahead on read error

On Tue, 12 Aug 2025 15:22:23 +0800 Chi Zhiling <chizhiling@....com> wrote:

> From: Chi Zhiling <chizhiling@...inos.cn>
> 
> For exFAT filesystems with 4MB read_ahead_size, removing the storage device
> during read operations can delay EIO error reporting by several minutes.
> This occurs because the read-ahead implementation in mpage doesn't handle
> errors.
> 
> Another reason for the delay is that the filesystem requires metadata to
> issue file read request. When the storage device is removed, the metadata
> buffers are invalidated, causing mpage to repeatedly attempt to fetch
> metadata during each get_block call.
> 
> The original purpose of this patch is terminate read ahead when we fail
> to get metadata, to make the patch more generic, implement it by checking
> folio status, instead of checking the return of get_block().
> 
> ...
>
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -369,6 +369,9 @@ void mpage_readahead(struct readahead_control *rac, get_block_t get_block)
>  		args.folio = folio;
>  		args.nr_pages = readahead_count(rac);
>  		args.bio = do_mpage_readpage(&args);
> +		if (!folio_test_locked(folio) &&
> +		    !folio_test_uptodate(folio))
> +			break;
>  	}
>  	if (args.bio)
>  		mpage_bio_submit_read(args.bio);

So...  this is what the fs does when the device is unplugged? 
Synchronously return an unlocked !uptodate folio?  Or is this specific
to FAT?

I think a comment here telling readers why we're doing this would be
helpful.  It isn't obvious that we're dealing with e removed device!

Also, boy this is old code.  Basically akpm code from pre-git times. 
It was quite innovative back then, but everybody who understood it has
since moved on,  got senile or probably died.  Oh well.

Also, that if statement didn't need a newline ;)