xfs
[Top] [All Lists]

Re: [PATCH 5/9] xfs: xfs_bioerror can die.

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 5/9] xfs: xfs_bioerror can die.
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 16 Aug 2014 09:27:41 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140815143540.GD4096@xxxxxxxxxxxxxx>
References: <1408084747-4540-1-git-send-email-david@xxxxxxxxxxxxx> <1408084747-4540-6-git-send-email-david@xxxxxxxxxxxxx> <20140815143540.GD4096@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Aug 15, 2014 at 10:35:41AM -0400, Brian Foster wrote:
> On Fri, Aug 15, 2014 at 04:39:03PM +1000, Dave Chinner wrote:
> > @@ -1149,19 +1119,19 @@ xfs_bwrite(
> >     ASSERT(xfs_buf_islocked(bp));
> >  
> >     bp->b_flags |= XBF_WRITE;
> > -   bp->b_flags &= ~(XBF_ASYNC | XBF_READ | _XBF_DELWRI_Q | XBF_WRITE_FAIL);
> > +   bp->b_flags &= ~(XBF_ASYNC | XBF_READ | _XBF_DELWRI_Q |
> > +                    XBF_WRITE_FAIL | XBF_DONE);
> >  
> >     if (XFS_FORCED_SHUTDOWN(bp->b_target->bt_mount)) {
> >             trace_xfs_bdstrat_shut(bp, _RET_IP_);
> >  
> > -           /*
> > -            * Metadata write that didn't get logged but written anyway.
> > -            * These aren't associated with a transaction, and can be
> > -            * ignored.
> > -            */
> > -           if (!bp->b_iodone)
> > -                   return xfs_bioerror_relse(bp);
> > -           return xfs_bioerror(bp);
> > +           xfs_buf_ioerror(bp, -EIO);
> > +           xfs_buf_stale(bp);
> > +
> > +           /* sync IO, xfs_buf_ioend is going to remove a ref here */
> > +           xfs_buf_hold(bp);
> 
> Looks correct, but that's kind of ugly. The reference serves no purpose
> except to appease the error sequence. It gives the impression that the
> reference management should be handled at a higher level (and with truly
> synchronous I/O primitives/mechanisms, it is ;).

Oh, yeah, it's nasty ugly, but that goes away with patch 8. This is
just a temporary state that then gets factored neatly when the new
interfaces are introduced.

> At the very least, can we reorganize the ioend side of things to handle
> this? We already have the duplicate execution issue previously mentioned
> that has to get resolved. Some duplicate code is fine if it improves
> clarity, imo.

patch 8 does that - this is just preparing the code for being easy
to factor.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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