From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B61B22063D761 for ; Wed, 23 May 2018 09:34:45 -0700 (PDT) Date: Wed, 23 May 2018 10:34:42 -0600 From: Ross Zwisler Subject: Re: [PATCH v3 7/9] dax: report bytes remaining in dax_iomap_actor() Message-ID: <20180523163442.GB29519@linux.intel.com> References: <152539236455.31796.7516599166555186700.stgit@dwillia2-desk3.amr.corp.intel.com> <152539240242.31796.4162676712193177396.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <152539240242.31796.4162676712193177396.stgit@dwillia2-desk3.amr.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Dan Williams Cc: tony.luck@intel.com, Andrew Morton , linux-nvdimm@lists.01.org, Peter Zijlstra , x86@kernel.org, linux-kernel@vger.kernel.org, Andy Lutomirski , Ingo Molnar , Borislav Petkov , Al Viro , linux-fsdevel@vger.kernel.org, Thomas Gleixner , Linus Torvalds , hch@lst.de List-ID: On Thu, May 03, 2018 at 05:06:42PM -0700, Dan Williams wrote: > In preparation for protecting the dax read(2) path from media errors > with copy_to_iter_mcsafe() (via dax_copy_to_iter()), convert the > implementation to report the bytes successfully transferred. > > Cc: > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Tony Luck > Cc: Al Viro > Cc: Thomas Gleixner > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Cc: Andrew Morton > Cc: Linus Torvalds > Signed-off-by: Dan Williams > --- > fs/dax.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/fs/dax.c b/fs/dax.c > index a64afdf7ec0d..34a2d435ae4b 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -991,6 +991,7 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > struct iov_iter *iter = data; > loff_t end = pos + length, done = 0; > ssize_t ret = 0; > + size_t xfer; > int id; > > if (iov_iter_rw(iter) == READ) { > @@ -1054,19 +1055,20 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > * vfs_write(), depending on which operation we are doing. > */ > if (iov_iter_rw(iter) == WRITE) > - map_len = dax_copy_from_iter(dax_dev, pgoff, kaddr, > + xfer = dax_copy_from_iter(dax_dev, pgoff, kaddr, > map_len, iter); > else > - map_len = dax_copy_to_iter(dax_dev, pgoff, kaddr, > + xfer = dax_copy_to_iter(dax_dev, pgoff, kaddr, > map_len, iter); > - if (map_len <= 0) { > - ret = map_len ? map_len : -EFAULT; > - break; > - } > > - pos += map_len; > - length -= map_len; > - done += map_len; > + pos += xfer; > + length -= xfer; > + done += xfer; > + > + if (xfer == 0) > + ret = -EFAULT; > + if (xfer < map_len) > + break; I'm confused by this error handling. So if we hit an error on a given iov and we don't transfer the expected number of bytes, we have two cases: 1) We transferred *something* on this iov but not everything - return success. 2) We didn't transfer anything on this iov - return -EFAULT. Both of these are true regardless of data transferred on previous iovs. Why the distinction? If a given iov is interrupted, regardless of whether it transferred 0 bytes or 1, shouldn't the error path be the same? > } > dax_read_unlock(id); _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm