On Mon, Aug 25, 2025 at 12:55:06AM -0700, Christoph Hellwig wrote:
> On Fri, Aug 22, 2025 at 08:21:37AM -0700, Darrick J. Wong wrote:
> > We only flag corruptions for these two error codes, but ENODATA from the
> > block layer means "critical medium error". I take that to mean the
> > media has permanently lost whatever was persisted there, right?
>
> It can also be a write error. But yes, it's what EIO indidcates in
> general. Which is why I really think we should be doing something like
> the patch below. But as I don't have the time to fully shephed this
> I'm not trying to block this hack, even if I think the issue will
> continue to byte us in the future.
Yes, it's a bit of a problem, whose issues we'll need to nibble on all
over the place to fix all the weird issues before issuing the customary
patchbomb for gluttinous consumption on fsdevel. <cough>
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index f9ef3b2a332a..0252faf038aa 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1290,6 +1290,22 @@ xfs_bwrite(
> return error;
> }
>
> +static int
> +xfs_buf_bio_status(
> + struct bio *bio)
> +{
> + switch (bio->bi_status) {
> + case BLK_STS_OK:
> + return 0;
> + case BLK_STS_NOSPC:
> + return -ENOSPC;
> + case BLK_STS_OFFLINE:
> + return -ENODEV;
> + default:
> + return -EIO;
Well as I pointed out earlier, one interesting "quality" of the current
behavior is that online fsck captures the ENODATA and turns that into a
metadata corruption report. I'd like to keep that behavior.
> + }
> +}
> +
> static void
> xfs_buf_bio_end_io(
> struct bio *bio)
> @@ -1297,7 +1313,7 @@ xfs_buf_bio_end_io(
> struct xfs_buf *bp = bio->bi_private;
>
> if (bio->bi_status)
> - xfs_buf_ioerror(bp, blk_status_to_errno(bio->bi_status));
> + xfs_buf_ioerror(bp, xfs_buf_bio_status(bio));
I think you'd also want to wrap all the submit_bio_wait here too, right?
Hrm, only discard bios, log writes, and zonegc use that function. Maybe
not? I think a failed log write takes down the system no matter what
error code, nobody cares about failing discard, and I think zonegc write
failures just lead to the gc ... aborting?
--D
> else if ((bp->b_flags & XBF_WRITE) && (bp->b_flags & XBF_ASYNC) &&
> XFS_TEST_ERROR(false, bp->b_mount, XFS_ERRTAG_BUF_IOERROR))
> xfs_buf_ioerror(bp, -EIO);
>
