[PATCH 8/9] xfs: introduce xfs_buf_submit[_wait]

[Christoph Hellwig]

> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 2f1e30d..c53cc03 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1157,12 +1157,7 @@ xfs_zero_remaining_bytes(

xfs_zero_remaining_bytes really should be switched to
xfs_buf_read_uncached + xfs_bwrite instead of all this mess just to
micro-optimize a few memory allocation away that don't even happen for
the common case of blocksize == PAGE_SIZE.  I'm not even sure we
should be using the buffer cache at all for something inside a regular
file, but that's a discussion for another time.

>  void
> +xfs_buf_submit(
> +	struct xfs_buf	*bp)
>  {
>  	trace_xfs_buf_iorequest(bp, _RET_IP_);

I suspect these two should have properly name and separate trace
points now?

It also seems like a lot of the guts of the two functions are
still the same, so factoring that into a __xfs_buf_submit helper
would be useful.

> +	 * If _xfs_buf_ioapply failed, 
> +	 * we can get back here with only the IO
>  	 * reference we took above.  _xfs_buf_ioend will drop it to zero, so
>  	 * we'd better run completion processing synchronously so that the we
> +	 * don't return to the caller with completion still pending.
> +	 * this allows the caller to check b_error safely without
> +	 * waiting
>  	 */
>  	if (atomic_dec_and_test(&bp->b_io_remaining) == 1) {
>  		if (bp->b_error || !(bp->b_flags & XBF_ASYNC))

I don't think the !ASYNC case can happen here, can it?

> +		if (!wait)
>  			list_del_init(&bp->b_list);
> +		else
> +			xfs_buf_hold(bp);

Maybe switch this around to avoid the negated condition in the if else?

Also might this be worth a change of it's own?

> +++ b/fs/xfs/xfs_trans_buf.c
> @@ -286,18 +286,13 @@ xfs_trans_read_buf_map(
>  			bp->b_flags |= XBF_READ;
>  			bp->b_ops = ops;
>  
> +			error = xfs_buf_submit_wait(bp);
>  			if (error) {
> +				if (!XFS_FORCED_SHUTDOWN(mp)) {
> +					xfs_buf_ioerror_alert(bp, __func__);
> +					goto out_do_shutdown;
> +				}
> +				goto out_stale;

Again maybe reverse the conditions here for a cleaner flow?