Looks good, although I still think this would benefit from a helper like the one below (patch lightly tested). Reviewed-by: Christoph Hellwig <hch@xxxxxx> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 0ac54a0..081ccf3 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -1287,28 +1287,18 @@ _xfs_buf_ioapply( blk_finish_plug(&plug); } -/* - * Asynchronous IO submission path. This transfers the buffer lock ownership and - * the current reference to the IO. It is not safe to reference the buffer after - * a call to this function unless the caller holds an additional reference - * itself. - */ -void -xfs_buf_submit( +static int +__xfs_buf_submit( struct xfs_buf *bp) { - trace_xfs_buf_submit(bp, _RET_IP_); - ASSERT(!(bp->b_flags & _XBF_DELWRI_Q)); - ASSERT(bp->b_flags & XBF_ASYNC); /* on shutdown we stale and complete the buffer immediately */ if (XFS_FORCED_SHUTDOWN(bp->b_target->bt_mount)) { xfs_buf_ioerror(bp, -EIO); bp->b_flags &= ~XBF_DONE; xfs_buf_stale(bp); - xfs_buf_ioend(bp); - return; + return -EIO; } if (bp->b_flags & XBF_WRITE) @@ -1318,12 +1308,8 @@ xfs_buf_submit( bp->b_io_error = 0; /* - * The caller's reference is released during I/O completion. - * This occurs some time after the last b_io_remaining reference is - * released, so after we drop our Io reference we have to have some - * other reference to ensure the buffer doesn't go away from underneath - * us. Take a direct reference to ensure we have safe access to the - * buffer until we are finished with it. + * Take a reference to ensure we have safe access to the buffer until + * we are finished with it. */ xfs_buf_hold(bp); @@ -1341,64 +1327,56 @@ xfs_buf_submit( * that we don't return to the caller with completion still pending. */ if (atomic_dec_and_test(&bp->b_io_remaining) == 1) { - if (bp->b_error) - xfs_buf_ioend(bp); - else + if ((bp->b_flags & XBF_ASYNC) && !bp->b_error) xfs_buf_ioend_async(bp); + else + xfs_buf_ioend(bp); } - xfs_buf_rele(bp); - /* Note: it is not safe to reference bp now we've dropped our ref */ + return 0; } /* - * Synchronous buffer IO submission path, read or write. + * Asynchronous IO submission path. This transfers the buffer lock ownership and + * the current reference to the IO. It is not safe to reference the buffer after + * a call to this function unless the caller holds an additional reference + * itself. */ -int -xfs_buf_submit_wait( +void +xfs_buf_submit( struct xfs_buf *bp) { int error; - trace_xfs_buf_submit_wait(bp, _RET_IP_); + trace_xfs_buf_submit(bp, _RET_IP_); - ASSERT(!(bp->b_flags & (_XBF_DELWRI_Q | XBF_ASYNC))); + ASSERT(bp->b_flags & XBF_ASYNC); - if (XFS_FORCED_SHUTDOWN(bp->b_target->bt_mount)) { - xfs_buf_ioerror(bp, -EIO); - xfs_buf_stale(bp); - bp->b_flags &= ~XBF_DONE; - return -EIO; - } + error = __xfs_buf_submit(bp); + if (error) + xfs_buf_ioend(bp); + else + xfs_buf_rele(bp); - if (bp->b_flags & XBF_WRITE) - xfs_buf_wait_unpin(bp); + /* Note: it is not safe to reference bp now we've dropped our ref */ +} - /* clear the internal error state to avoid spurious errors */ - bp->b_io_error = 0; +/* + * Synchronous buffer IO submission path, read or write. + */ +int +xfs_buf_submit_wait( + struct xfs_buf *bp) +{ + int error; - /* - * For synchronous IO, the IO does not inherit the submitters reference - * count, nor the buffer lock. Hence we cannot release the reference we - * are about to take until we've waited for all IO completion to occur, - * including any xfs_buf_ioend_async() work that may be pending. - */ - xfs_buf_hold(bp); + trace_xfs_buf_submit_wait(bp, _RET_IP_); - /* - * Set the count to 1 initially, this will stop an I/O completion - * callout which happens before we have started all the I/O from calling - * xfs_buf_ioend too early. - */ - atomic_set(&bp->b_io_remaining, 1); - _xfs_buf_ioapply(bp); + ASSERT(!(bp->b_flags & XBF_ASYNC)); - /* - * make sure we run completion synchronously if it raced with us and is - * already complete. - */ - if (atomic_dec_and_test(&bp->b_io_remaining) == 1) - xfs_buf_ioend(bp); + error = __xfs_buf_submit(bp); + if (error) + return error; /* wait for completion before gathering the error from the buffer */ trace_xfs_buf_iowait(bp, _RET_IP_); _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs