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
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
