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

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs