Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 8 Mar 2018 17:20:48 -0700
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
Message-ID: <20180309002048.GA22835@linux.intel.com>
References: <20180212230558.5546-1-ross.zwisler@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180212230558.5546-1-ross.zwisler@linux.intel.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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
>