Re: [PATCH] fuse: fix alignment in short read optimization for async_dio

[Brian Foster]

On 05/30/2013 08:41 AM, Maxim Patlasov wrote:
> The bug was introduced with async_dio feature: trying to optimize short reads,
> we cut number-of-bytes-to-read to i_size boundary. Hence the following example:
> 
> 	truncate --size=300 /mnt/file
> 	dd if=/mnt/file of=/dev/null iflag=direct
> 
> led to FUSE_READ request of 300 bytes size. This turned out to be problem
> for userspace fuse implementations who rely on assumption that kernel fuse
> does not change alignment of request from client FS.
> 
> The patch turns off the optimization if async_dio is disabled. And, if it's
> enabled, the patch fixes adjustment of number-of-bytes-to-read to preserve
> alignment.
> 
> Note, that we cannot throw out short read optimization entirely because
> otherwise a direct read of a huge size issued on a tiny file would generate
> a huge amount of fuse requests and most of them would be ACKed by userspace
> with zero bytes read.
> 
> Signed-off-by: Maxim Patlasov <MPatlasov@xxxxxxxxxxxxx>
> ---

Looks good and passes my tests. Thanks for cooking this up Maxim!

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/fuse/file.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index d1c9b85..9026572 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2365,6 +2365,11 @@ static void fuse_do_truncate(struct file *file)
>  	fuse_do_setattr(inode, &attr, file);
>  }
>  
> +static inline loff_t fuse_round_up(loff_t off)
> +{
> +	return round_up(off, FUSE_MAX_PAGES_PER_REQ << PAGE_SHIFT);
> +}
> +
>  static ssize_t
>  fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
>  			loff_t offset, unsigned long nr_segs)
> @@ -2372,6 +2377,7 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
>  	ssize_t ret = 0;
>  	struct file *file = iocb->ki_filp;
>  	struct fuse_file *ff = file->private_data;
> +	bool async_dio = ff->fc->async_dio;
>  	loff_t pos = 0;
>  	struct inode *inode;
>  	loff_t i_size;
> @@ -2383,10 +2389,10 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
>  	i_size = i_size_read(inode);
>  
>  	/* optimization for short read */
> -	if (rw != WRITE && offset + count > i_size) {
> +	if (async_dio && rw != WRITE && offset + count > i_size) {
>  		if (offset >= i_size)
>  			return 0;
> -		count = i_size - offset;
> +		count = min_t(loff_t, count, fuse_round_up(i_size - offset));
>  	}
>  
>  	io = kmalloc(sizeof(struct fuse_io_priv), GFP_KERNEL);
> @@ -2404,7 +2410,7 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
>  	 * By default, we want to optimize all I/Os with async request
>  	 * submission to the client filesystem if supported.
>  	 */
> -	io->async = ff->fc->async_dio;
> +	io->async = async_dio;
>  	io->iocb = iocb;
>  
>  	/*
> @@ -2412,7 +2418,7 @@ fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
>  	 * to wait on real async I/O requests, so we must submit this request
>  	 * synchronously.
>  	 */
> -	if (!is_sync_kiocb(iocb) && (offset + count > i_size))
> +	if (!is_sync_kiocb(iocb) && (offset + count > i_size) && rw == WRITE)
>  		io->async = false;
>  
>  	if (rw == WRITE)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/