xfs
[Top] [All Lists]

Re: [PATCH 7/8] xfs: add DAX IO path support

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 7/8] xfs: add DAX IO path support
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 6 Apr 2015 13:49:14 -0400
Cc: xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, willy@xxxxxxxxxxxxxxx, jack@xxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1427194266-2885-8-git-send-email-david@xxxxxxxxxxxxx>
References: <1427194266-2885-1-git-send-email-david@xxxxxxxxxxxxx> <1427194266-2885-8-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Tue, Mar 24, 2015 at 09:51:05PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> DAX does not do buffered IO (can't buffer direct access!) and hence
> all read/write IO is vectored through the direct IO path.  Hence we
> need to add the DAX IO path callouts to the direct IO
> infrastructure.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_aops.c | 35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 3fc5052..97979e9 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1559,6 +1559,30 @@ xfs_end_io_direct_write(
>       }
>  }
>  
> +static inline ssize_t
> +xfs_vm_do_dio(
> +     struct inode            *inode,
> +     int                     rw,
> +     struct kiocb            *iocb,
> +     struct iov_iter         *iter,
> +     loff_t                  offset,
> +     void                    (*endio)(struct kiocb   *iocb,
> +                                      loff_t         offset,
> +                                      ssize_t        size,
> +                                      void           *private),
> +     int                     flags)
> +{
> +     struct block_device     *bdev;
> +
> +     if (IS_DAX(inode))
> +             return dax_do_io(rw, iocb, inode, iter, offset,
> +                              xfs_get_blocks_direct, endio, 0);
> +

I assume this is supposed to be get_blocks_direct and not
get_blocks_dax, based on the I/O codepath. The naming is starting to get
a little confusing though. xfs_get_blocks_dax() implies to me that it's
for any DAX I/O, but we only appear to use it internally for
truncate/zeroing/mmap and such. Alas, I can't think of a better name atm
and the code seems Ok to me:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

... but a comment somewhere around here and/or at the
xfs_get_blocks_dax() function would be helpful.

Brian

> +     bdev = xfs_find_bdev_for_inode(inode);
> +     return  __blockdev_direct_IO(rw, iocb, inode, bdev, iter, offset,
> +                                  xfs_get_blocks_direct, endio, NULL, flags);
> +}
> +
>  STATIC ssize_t
>  xfs_vm_direct_IO(
>       int                     rw,
> @@ -1567,17 +1591,12 @@ xfs_vm_direct_IO(
>       loff_t                  offset)
>  {
>       struct inode            *inode = iocb->ki_filp->f_mapping->host;
> -     struct block_device     *bdev = xfs_find_bdev_for_inode(inode);
>  
>       if (rw & WRITE) {
> -             return __blockdev_direct_IO(rw, iocb, inode, bdev, iter,
> -                                         offset, xfs_get_blocks_direct,
> -                                         xfs_end_io_direct_write, NULL,
> -                                         DIO_ASYNC_EXTEND);
> +             return xfs_vm_do_dio(inode, rw, iocb, iter, offset,
> +                                  xfs_end_io_direct_write, DIO_ASYNC_EXTEND);
>       }
> -     return __blockdev_direct_IO(rw, iocb, inode, bdev, iter,
> -                                 offset, xfs_get_blocks_direct,
> -                                 NULL, NULL, 0);
> +     return xfs_vm_do_dio(inode, rw, iocb, iter, offset, NULL, 0);
>  }
>  
>  /*
> -- 
> 2.0.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>