Re: [PATCH 03/19] xfs: uncached buffer reads need to return an error

Dave Chinner
2012-10-11T22:10:06Z

On Thu, Oct 11, 2012 at 05:38:02PM -0400, Christoph Hellwig wrote:
> > index 917e121..dee14eb 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -149,6 +149,11 @@ xfs_growfs_data_private(
> > XFS_FSS_TO_BB(mp, 1), 0, NULL);
> > if (!bp)
> > return EIO;
> > + if (bp->b_error) {
> > + int error = bp->b_error;
> > + xfs_buf_relse(bp);
> > + return error;
> > + }
> > xfs_buf_relse(bp);
>
> > + if (bp->b_error) {
> > + error = bp->b_error;
> > + if (loud)
> > + xfs_warn(mp, "SB validate failed");
> > + goto release_buf;
> > + }
>
> > + if (bp->b_error) {
> > + error = bp->b_error;
> > + xfs_buf_relse(bp);
> > + return error;
> > + }
>
> > + if (!bp || bp->b_error) {
> > xfs_warn(mp, "realtime device size check failed");
> > + if (bp)
> > + xfs_buf_relse(bp);
> > return EIO;
> > }
> > xfs_buf_relse(bp);
>
> It seems like all these callers would be a lot cleaner if we'd just
> return the error as the return value, and a buffer as an indirect
> pointer if and only if the read succeeded.

The number of callers is relatively small, and the knock-on effect
through the subsequent patches isn't that bad, so changing the
interface is probably the right thing to do here. I'll rework this
patch appropriately.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com