xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 03/19] xfs: uncached buffer reads need to return an error
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 12 Oct 2012 09:11:37 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20121011213802.GC6346@xxxxxxxxxxxxx>
References: <1349754670-32009-1-git-send-email-david@xxxxxxxxxxxxx> <1349754670-32009-4-git-send-email-david@xxxxxxxxxxxxx> <20121011213802.GC6346@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>