xfs
[Top] [All Lists]

Re: [PATCH 6/9] xfs: kill xfs_bioerror_relse

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/9] xfs: kill xfs_bioerror_relse
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 29 Aug 2014 11:12:36 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140829003257.GF17502@xxxxxxxxxxxxx>
References: <1408084747-4540-1-git-send-email-david@xxxxxxxxxxxxx> <1408084747-4540-7-git-send-email-david@xxxxxxxxxxxxx> <20140829003257.GF17502@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Aug 28, 2014 at 05:32:57PM -0700, Christoph Hellwig wrote:
> > index 96c898e..758c07d 100644
> > --- a/fs/xfs/xfs_trans_buf.c
> > +++ b/fs/xfs/xfs_trans_buf.c
> > @@ -324,11 +324,13 @@ xfs_trans_read_buf_map(
> >                      */
> >                     if (XFS_FORCED_SHUTDOWN(mp)) {
> >                             trace_xfs_bdstrat_shut(bp, _RET_IP_);
> > -                           xfs_bioerror_relse(bp);
> > -                   } else {
> > -                           xfs_buf_iorequest(bp);
> > +                           bp->b_flags &= ~(XBF_READ | XBF_DONE);
> > +                           xfs_buf_ioerror(bp, -EIO);
> > +                           xfs_buf_stale(bp);
> > +                           return -EIO;
> >                     }
> 
> This is a large change of behavior as it doesn't hit the error
> path after the xfs_buf_iowait anymore.  While I don't think that
> that path was entirely correct this version seems to be even less so
> by not releasing the buffer reference or forcing the shutdown.

The IO is synchronous, so the previous behaviour did not release
the buffer here. But, yes, it needs to because we're not running the
io wait on it anymore. And this happens only during a shutdown, so i
don't see any need to trigger a shutdown ;)

As it is, I think this gets properly fixed by the next patch....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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