All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-fsdevel@vger.kernel.org, willy@linux.intel.com,
	jack@suse.cz, xfs@oss.sgi.com
Subject: Re: [PATCH 7/8] xfs: add DAX IO path support
Date: Mon, 6 Apr 2015 13:49:14 -0400	[thread overview]
Message-ID: <20150406174913.GE58965@bfoster.bfoster> (raw)
In-Reply-To: <1427194266-2885-8-git-send-email-david@fromorbit.com>

On Tue, Mar 24, 2015 at 09:51:05PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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@redhat.com>
> ---
>  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@redhat.com>

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

WARNING: multiple messages have this Message-ID (diff)
From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com, linux-fsdevel@vger.kernel.org,
	willy@linux.intel.com, jack@suse.cz
Subject: Re: [PATCH 7/8] xfs: add DAX IO path support
Date: Mon, 6 Apr 2015 13:49:14 -0400	[thread overview]
Message-ID: <20150406174913.GE58965@bfoster.bfoster> (raw)
In-Reply-To: <1427194266-2885-8-git-send-email-david@fromorbit.com>

On Tue, Mar 24, 2015 at 09:51:05PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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@redhat.com>
> ---
>  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@redhat.com>

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

  reply	other threads:[~2015-04-06 17:49 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-24 10:50 [PATCH 0/8 v2] xfs: DAX support Dave Chinner
2015-03-24 10:50 ` Dave Chinner
2015-03-24 10:50 ` [PATCH 1/8] xfs: mmap lock needs to be inside freeze protection Dave Chinner
2015-04-01 14:34   ` Jan Kara
2015-04-01 14:34     ` Jan Kara
2015-04-06 17:48   ` Brian Foster
2015-04-06 17:48     ` Brian Foster
2015-03-24 10:51 ` [PATCH 2/8] dax: don't abuse get_block mapping for endio callbacks Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-04-01 14:53   ` Jan Kara
2015-04-01 14:53     ` Jan Kara
2015-03-24 10:51 ` [PATCH 3/8] dax: expose __dax_fault for filesystems with locking constraints Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-04-01 15:07   ` Jan Kara
2015-04-01 15:07     ` Jan Kara
2015-03-24 10:51 ` [PATCH 4/8] xfs: add DAX block zeroing support Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-04-06 17:48   ` Brian Foster
2015-04-06 17:48     ` Brian Foster
2015-03-24 10:51 ` [PATCH 5/8] xfs: add DAX file operations support Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-03-24 12:08   ` Boaz Harrosh
2015-03-24 12:08     ` Boaz Harrosh
2015-03-24 12:24     ` Boaz Harrosh
2015-03-24 12:24       ` Boaz Harrosh
2015-03-24 21:17     ` Dave Chinner
2015-03-24 21:17       ` Dave Chinner
2015-03-25  8:47       ` Boaz Harrosh
2015-03-25  8:47         ` Boaz Harrosh
2015-04-06 17:49   ` Brian Foster
2015-04-06 17:49     ` Brian Foster
2015-04-16  8:29     ` Dave Chinner
2015-04-16  8:29       ` Dave Chinner
2015-04-16  9:33   ` Boaz Harrosh
2015-04-16  9:33     ` Boaz Harrosh
2015-04-16 11:47     ` Dave Chinner
2015-04-16 11:47       ` Dave Chinner
2015-03-24 10:51 ` [PATCH 6/8] xfs: add DAX truncate support Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-04-06 17:49   ` Brian Foster
2015-04-06 17:49     ` Brian Foster
2015-03-24 10:51 ` [PATCH 7/8] xfs: add DAX IO path support Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-04-06 17:49   ` Brian Foster [this message]
2015-04-06 17:49     ` Brian Foster
2015-04-16  8:54     ` Dave Chinner
2015-04-16  8:54       ` Dave Chinner
2015-03-24 10:51 ` [PATCH 8/8] xfs: add initial DAX support Dave Chinner
2015-03-24 10:51   ` Dave Chinner
2015-03-24 12:52   ` Boaz Harrosh
2015-03-24 12:52     ` Boaz Harrosh
2015-03-24 21:25     ` Dave Chinner
2015-03-24 21:25       ` Dave Chinner
2015-03-25  9:14       ` Boaz Harrosh
2015-03-25  9:14         ` Boaz Harrosh
2015-04-06 19:00   ` Brian Foster
2015-04-06 19:00     ` Brian Foster
  -- strict thread matches above, loose matches on Subject: below --
2015-05-28 23:45 [PATCH 0/8 v3] xfs: " Dave Chinner
2015-05-28 23:45 ` [PATCH 7/8] xfs: add DAX IO path support Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150406174913.GE58965@bfoster.bfoster \
    --to=bfoster@redhat.com \
    --cc=david@fromorbit.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=willy@linux.intel.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.