On Tue, 2011-01-04 at 15:48 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> The current xfs_file_aio_write code is a mess of locking shenanigans
> to handle the different locking requirements of buffered and direct
> IO. Start to clean this up by disentangling the direct IO path from
> the mess.
All good, very good. But I'm not sure why you cut
out the code that backed off to buffered I/O if
generic_file_direct_write() returns an error.
(You gave no explanation.)
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> fs/xfs/linux-2.6/xfs_file.c | 168
> 1 files changed, 110 insertions(+), 58 deletions(-)
> diff --git a/fs/xfs/linux-2.6/xfs_file.c b/fs/xfs/linux-2.6/xfs_file.c
> index 0d6111e..d546953 100644
> --- a/fs/xfs/linux-2.6/xfs_file.c
> +++ b/fs/xfs/linux-2.6/xfs_file.c
> @@ -619,6 +619,110 @@ out_lock:
> return error;
> + * xfs_file_dio_aio_write - handle direct IO writes
> + *
> + * Lock the inode appropriately to prepare for and issue a direct IO write.
> + * By spearating it from the buffered write path we remove all the tricky to
> + * follow locking changes and looping. This also clearly indicates that XFS
> + * does not fall back to buffered IO in the direct IO write path.
> + *
> + * Returns with locks held indicated by @iolock and errors indicated by
> + * negative return values.
> + */
> +STATIC ssize_t
> + struct kiocb *iocb,
> + const struct iovec *iovp,
. . .
> + trace_xfs_file_direct_write(ip, count, iocb->ki_pos, 0);
> + ret = generic_file_direct_write(iocb, iovp,
> + &nr_segs, pos, &iocb->ki_pos, count, ocount);
> + /* No fallback to buffered IO on errors for XFS. */
Why is this? The previous code did fall back (so this change
is doing more than just splitting out the direct I/O path).
> + return ret;
> STATIC ssize_t
> struct kiocb *iocb,
. . .
> @@ -788,6 +839,7 @@ write_retry:
> current->backing_dev_info = NULL;
> xfs_aio_write_isize_update(inode, &iocb->ki_pos, ret);
> if (ret <= 0)