On Wed, Jan 26, 2022 at 09:21:53AM +0100, Christian Brauner wrote:
> On Tue, Jan 25, 2022 at 06:18:09PM -0800, Darrick J. Wong wrote:
> > Hi all,
> >
> > While auditing the VFS code, I noticed that while ->sync_fs is allowed
> > to return error codes to reflect some sort of internal filesystem error,
> > none of the callers actually check the return value. Back when this
> > callout was introduced for sync_filesystem in 2.5 this didn't matter
>
> (Also, it looks like that most(/none?) of the filesystems that
> implemented ->sync_fs around 2.5/2.6 (ext3, jfs, jffs2, reiserfs etc.)
> actually did return an error?
Yes, some of them do -- ext4 will bubble up jbd2 errors and the results
of flushing the bdev write cache.
> In fact, 5.8 seems to be the first kernel to report other errors than
> -EBADF since commit 735e4ae5ba28 ("vfs: track per-sb writeback errors
> and report them to syncfs"?)
Yeah. I think the bdev pagecache flush might occasionally return errors
if there happened to be dirty pages, but (a) that doesn't help XFS which
has its own buffer cache and (b) that doesn't capture the state "fs has
errored out but media is fine".
As it is I think the ext4 syncfs needs to start returning EIO if someone
forced a shutdown, and probably some auditing for dropped error codes
due to the 'traditional' vfs behavior. btrfs probably ought to return
the result of filemap_flush too.
--D
