Re: [PATCH] xfs: do not propagate ENODATA disk errors into xattr code

["Darrick J. Wong" <djwong@kernel.org>]

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);
>