All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Jens Axboe <axboe@kernel.dk>
Cc: stable@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	linux-kernel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	linux-nvdimm@lists.01.org
Subject: Re: [PATCH] loop: Fix lost writes caused by missing flag
Date: Thu, 8 Mar 2018 17:20:48 -0700	[thread overview]
Message-ID: <20180309002048.GA22835@linux.intel.com> (raw)
In-Reply-To: <20180212230558.5546-1-ross.zwisler@linux.intel.com>

This has gotten Reviewed-by tags from Christoph and Ming Lei.

Al, are you the right person to merge this?  Or is the correct person Jens,
whom I accidentally didn't include when I sent this out?

Just wanted to make sure this got merged, and to see whether it was targeting
v4.16 or v4.17.

Thanks,
- Ross

On Mon, Feb 12, 2018 at 04:05:58PM -0700, Ross Zwisler wrote:
> The following commit:
> 
> commit aa4d86163e4e ("block: loop: switch to VFS ITER_BVEC")
> 
> replaced __do_lo_send_write(), which used ITER_KVEC iterators, with
> lo_write_bvec() which uses ITER_BVEC iterators.  In this change, though,
> the WRITE flag was lost:
> 
> -       iov_iter_kvec(&from, ITER_KVEC | WRITE, &kvec, 1, len);
> +       iov_iter_bvec(&i, ITER_BVEC, bvec, 1, bvec->bv_len);
> 
> This flag is necessary for the DAX case because we make decisions based on
> whether or not the iterator is a READ or a WRITE in dax_iomap_actor() and
> in dax_iomap_rw().
> 
> We end up going through this path in configurations where we combine a PMEM
> device with 4k sectors, a loopback device and DAX.  The consequence of this
> missed flag is that what we intend as a write actually turns into a read in
> the DAX code, so no data is ever written.
> 
> The very simplest test case is to create a loopback device and try and
> write a small string to it, then hexdump a few bytes of the device to see
> if the write took.  Without this patch you read back all zeros, with this
> you read back the string you wrote.
> 
> For XFS this causes us to fail or panic during the following xfstests:
> 
> 	xfs/074 xfs/078 xfs/216 xfs/217 xfs/250
> 
> For ext4 we have a similar issue where writes never happen, but we don't
> currently have any xfstests that use loopback and show this issue.
> 
> Fix this by restoring the WRITE flag argument to iov_iter_bvec().  This
> causes the xfstests to all pass.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Fixes: commit aa4d86163e4e ("block: loop: switch to VFS ITER_BVEC")
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: stable@vger.kernel.org
> ---
>  drivers/block/loop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index d5fe720cf149..89d2ee00cced 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -266,7 +266,7 @@ static int lo_write_bvec(struct file *file, struct bio_vec *bvec, loff_t *ppos)
>  	struct iov_iter i;
>  	ssize_t bw;
>  
> -	iov_iter_bvec(&i, ITER_BVEC, bvec, 1, bvec->bv_len);
> +	iov_iter_bvec(&i, ITER_BVEC | WRITE, bvec, 1, bvec->bv_len);
>  
>  	file_start_write(file);
>  	bw = vfs_iter_write(file, &i, ppos, 0);
> -- 
> 2.14.3
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org,
	Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	stable@vger.kernel.org
Subject: Re: [PATCH] loop: Fix lost writes caused by missing flag
Date: Thu, 8 Mar 2018 17:20:48 -0700	[thread overview]
Message-ID: <20180309002048.GA22835@linux.intel.com> (raw)
In-Reply-To: <20180212230558.5546-1-ross.zwisler@linux.intel.com>

This has gotten Reviewed-by tags from Christoph and Ming Lei.

Al, are you the right person to merge this?  Or is the correct person Jens,
whom I accidentally didn't include when I sent this out?

Just wanted to make sure this got merged, and to see whether it was targeting
v4.16 or v4.17.

Thanks,
- Ross

On Mon, Feb 12, 2018 at 04:05:58PM -0700, Ross Zwisler wrote:
> The following commit:
> 
> commit aa4d86163e4e ("block: loop: switch to VFS ITER_BVEC")
> 
> replaced __do_lo_send_write(), which used ITER_KVEC iterators, with
> lo_write_bvec() which uses ITER_BVEC iterators.  In this change, though,
> the WRITE flag was lost:
> 
> -       iov_iter_kvec(&from, ITER_KVEC | WRITE, &kvec, 1, len);
> +       iov_iter_bvec(&i, ITER_BVEC, bvec, 1, bvec->bv_len);
> 
> This flag is necessary for the DAX case because we make decisions based on
> whether or not the iterator is a READ or a WRITE in dax_iomap_actor() and
> in dax_iomap_rw().
> 
> We end up going through this path in configurations where we combine a PMEM
> device with 4k sectors, a loopback device and DAX.  The consequence of this
> missed flag is that what we intend as a write actually turns into a read in
> the DAX code, so no data is ever written.
> 
> The very simplest test case is to create a loopback device and try and
> write a small string to it, then hexdump a few bytes of the device to see
> if the write took.  Without this patch you read back all zeros, with this
> you read back the string you wrote.
> 
> For XFS this causes us to fail or panic during the following xfstests:
> 
> 	xfs/074 xfs/078 xfs/216 xfs/217 xfs/250
> 
> For ext4 we have a similar issue where writes never happen, but we don't
> currently have any xfstests that use loopback and show this issue.
> 
> Fix this by restoring the WRITE flag argument to iov_iter_bvec().  This
> causes the xfstests to all pass.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Fixes: commit aa4d86163e4e ("block: loop: switch to VFS ITER_BVEC")
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: stable@vger.kernel.org
> ---
>  drivers/block/loop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index d5fe720cf149..89d2ee00cced 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -266,7 +266,7 @@ static int lo_write_bvec(struct file *file, struct bio_vec *bvec, loff_t *ppos)
>  	struct iov_iter i;
>  	ssize_t bw;
>  
> -	iov_iter_bvec(&i, ITER_BVEC, bvec, 1, bvec->bv_len);
> +	iov_iter_bvec(&i, ITER_BVEC | WRITE, bvec, 1, bvec->bv_len);
>  
>  	file_start_write(file);
>  	bw = vfs_iter_write(file, &i, ppos, 0);
> -- 
> 2.14.3
> 

  parent reply	other threads:[~2018-03-09  0:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-12 23:05 [PATCH] loop: Fix lost writes caused by missing flag Ross Zwisler
2018-02-12 23:05 ` Ross Zwisler
2018-02-13 14:54 ` Christoph Hellwig
2018-02-13 14:54   ` Christoph Hellwig
2018-02-13 19:22   ` Ross Zwisler
2018-02-13 19:22     ` Ross Zwisler
2018-02-13 20:52     ` Dan Williams
2018-02-13 20:52       ` Dan Williams
2018-02-22  3:21 ` Ming Lei
2018-02-22  3:21   ` Ming Lei
2018-03-09  0:20 ` Ross Zwisler [this message]
2018-03-09  0:20   ` Ross Zwisler
2018-03-09 15:38   ` Jens Axboe
2018-03-09 15:38     ` Jens Axboe
2018-03-09 15:38     ` Jens Axboe
2018-03-09 15:38       ` Jens Axboe
2018-03-09 16:35       ` Ross Zwisler
2018-03-09 16:35         ` Ross Zwisler
2018-03-09 17:19         ` Jens Axboe
2018-03-09 17:19           ` Jens Axboe
2018-03-09 16:38       ` [PATCH] MAINTAINERS: add coverage for drivers/block Ross Zwisler
2018-03-09 16:38         ` Ross Zwisler

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=20180309002048.GA22835@linux.intel.com \
    --to=ross.zwisler@linux.intel.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.