Linux-XFS Archive on lore.kernel.org
 help / color / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	linux-xfs@vger.kernel.org, Dave Chinner <david@fromorbit.com>,
	Matthew Bobrowski <mbobrowski@mbobrowski.org>
Subject: Re: [PATCH 1/2] iomap: Allow forcing of waiting for running DIO in iomap_dio_rw()
Date: Fri, 11 Oct 2019 08:28:21 -0700
Message-ID: <20191011152821.GJ13108@magnolia> (raw)
In-Reply-To: <20191011141433.18354-1-jack@suse.cz>

On Fri, Oct 11, 2019 at 04:14:31PM +0200, Jan Kara wrote:
> Filesystems do not support doing IO as asynchronous in some cases. For
> example in case of unaligned writes or in case file size needs to be
> extended (e.g. for ext4). Instead of forcing filesystem to wait for AIO
> in such cases, add argument to iomap_dio_rw() which makes the function
> wait for IO completion. This also results in executing
> iomap_dio_complete() inline in iomap_dio_rw() providing its return value
> to the caller as for ordinary sync IO.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/gfs2/file.c        | 6 ++++--
>  fs/iomap/direct-io.c  | 7 +++++--
>  fs/xfs/xfs_file.c     | 5 +++--
>  include/linux/iomap.h | 3 ++-
>  4 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 997b326247e2..f0caee2b7c00 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -732,7 +732,8 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to)
>  	if (ret)
>  		goto out_uninit;
>  
> -	ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL);
> +	ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL,
> +			   is_sync_kiocb(iocb));
>  
>  	gfs2_glock_dq(&gh);
>  out_uninit:
> @@ -767,7 +768,8 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from)
>  	if (offset + len > i_size_read(&ip->i_inode))
>  		goto out;
>  
> -	ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL);
> +	ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL,
> +			   is_sync_kiocb(iocb));
>  
>  out:
>  	gfs2_glock_dq(&gh);
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 1fc28c2da279..1bfa0993f705 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -392,7 +392,8 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
>   */
>  ssize_t
>  iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> -		const struct iomap_ops *ops, const struct iomap_dio_ops *dops)
> +		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
> +		bool wait_for_completion)
>  {
>  	struct address_space *mapping = iocb->ki_filp->f_mapping;
>  	struct inode *inode = file_inode(iocb->ki_filp);
> @@ -400,7 +401,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  	loff_t pos = iocb->ki_pos, start = pos;
>  	loff_t end = iocb->ki_pos + count - 1, ret = 0;
>  	unsigned int flags = IOMAP_DIRECT;
> -	bool wait_for_completion = is_sync_kiocb(iocb);
>  	struct blk_plug plug;
>  	struct iomap_dio *dio;
>  
> @@ -409,6 +409,9 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
>  	if (!count)
>  		return 0;
>  
> +	if (WARN_ON(is_sync_kiocb(iocb) && !wait_for_completion))
> +		return -EINVAL;

So far in iomap we've been returning EIO when someone internal screws
up, which (AFAICT) is the case here.

Other than that, the rest of the changes look ok.

--D

> +
>  	dio = kmalloc(sizeof(*dio), GFP_KERNEL);
>  	if (!dio)
>  		return -ENOMEM;
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 1ffb179f35d2..0739ba72a82e 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -188,7 +188,7 @@ xfs_file_dio_aio_read(
>  	file_accessed(iocb->ki_filp);
>  
>  	xfs_ilock(ip, XFS_IOLOCK_SHARED);
> -	ret = iomap_dio_rw(iocb, to, &xfs_iomap_ops, NULL);
> +	ret = iomap_dio_rw(iocb, to, &xfs_iomap_ops, NULL, is_sync_kiocb(iocb));
>  	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
>  
>  	return ret;
> @@ -547,7 +547,8 @@ xfs_file_dio_aio_write(
>  	}
>  
>  	trace_xfs_file_direct_write(ip, count, iocb->ki_pos);
> -	ret = iomap_dio_rw(iocb, from, &xfs_iomap_ops, &xfs_dio_write_ops);
> +	ret = iomap_dio_rw(iocb, from, &xfs_iomap_ops, &xfs_dio_write_ops,
> +			   is_sync_kiocb(iocb));
>  
>  	/*
>  	 * If unaligned, this is the only IO in-flight. If it has not yet
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 7aa5d6117936..76b14cb729dc 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -195,7 +195,8 @@ struct iomap_dio_ops {
>  };
>  
>  ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> -		const struct iomap_ops *ops, const struct iomap_dio_ops *dops);
> +		const struct iomap_ops *ops, const struct iomap_dio_ops *dops,
> +		bool wait_for_completion);
>  int iomap_dio_iopoll(struct kiocb *kiocb, bool spin);
>  
>  #ifdef CONFIG_SWAP
> -- 
> 2.16.4
> 

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 14:14 [PATCH 0/2 v2] iomap: Waiting for IO " Jan Kara
2019-10-11 14:14 ` [PATCH 1/2] iomap: Allow forcing of waiting for running DIO " Jan Kara
2019-10-11 15:28   ` Darrick J. Wong [this message]
2019-10-11 16:31     ` Jan Kara
2019-10-11 18:04       ` Darrick J. Wong
2019-10-11 21:13         ` Darrick J. Wong
2019-10-11 14:14 ` [PATCH 2/2] xfs: Use iomap_dio_rw_wait() Jan Kara
2019-10-11 15:28   ` Darrick J. Wong
  -- strict thread matches above, loose matches on Subject: below --
2019-10-14  8:26 [PATCH 0/2 v3] iomap: Waiting for IO in iomap_dio_rw() Jan Kara
2019-10-14  8:26 ` [PATCH 1/2] iomap: Allow forcing of waiting for running DIO " Jan Kara
2019-10-14  8:40   ` Christoph Hellwig
2019-10-09 20:41 [PATCH 0/2] iomap: Waiting for IO " Jan Kara
2019-10-09 20:41 ` [PATCH 1/2] iomap: Allow forcing of waiting for running DIO " Jan Kara

Reply instructions:

You may reply publically 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=20191011152821.GJ13108@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mbobrowski@mbobrowski.org \
    /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

Linux-XFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-xfs linux-xfs/ https://lore.kernel.org/linux-xfs \
		linux-xfs@vger.kernel.org
	public-inbox-index linux-xfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-xfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git