On Wed, Jan 26, 2022 at 10:05:07AM -0800, Darrick J. Wong wrote:
> 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.
Makes sense. Fwiw,
Acked-by: Christian Brauner <brauner@xxxxxxxxxx>
