From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:13124 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728655AbgBKPuN (ORCPT ); Tue, 11 Feb 2020 10:50:13 -0500 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 01BFnRnT150679 for ; Tue, 11 Feb 2020 10:50:12 -0500 Received: from e06smtp02.uk.ibm.com (e06smtp02.uk.ibm.com [195.75.94.98]) by mx0b-001b2d01.pphosted.com with ESMTP id 2y3wxrkcx6-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 11 Feb 2020 10:50:10 -0500 Received: from localhost by e06smtp02.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 11 Feb 2020 15:50:00 -0000 Date: Tue, 11 Feb 2020 16:49:54 +0100 From: Gerald Schaefer Subject: Re: [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver In-Reply-To: <20200211151114.GA8590@redhat.com> References: <20200207202652.1439-1-vgoyal@redhat.com> <20200207202652.1439-5-vgoyal@redhat.com> <20200210215315.27b7e310@thinkpad> <20200211151114.GA8590@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20200211164954.4df79b8b@thinkpad> Sender: linux-s390-owner@vger.kernel.org List-ID: To: Vivek Goyal Cc: linux-fsdevel@vger.kernel.org, linux-nvdimm@lists.01.org, hch@infradead.org, dan.j.williams@intel.com, dm-devel@redhat.com, vishal.l.verma@intel.com, linux-s390@vger.kernel.org On Tue, 11 Feb 2020 10:11:14 -0500 Vivek Goyal wrote: > On Mon, Feb 10, 2020 at 09:53:15PM +0100, Gerald Schaefer wrote: > > On Fri, 7 Feb 2020 15:26:49 -0500 > > Vivek Goyal wrote: > > > > > Add dax operation zero_page_range for dcssblk driver. > > > > > > CC: linux-s390@vger.kernel.org > > > Suggested-by: Christoph Hellwig > > > Signed-off-by: Vivek Goyal > > > --- > > > drivers/s390/block/dcssblk.c | 17 +++++++++++++++++ > > > 1 file changed, 17 insertions(+) > > > > > > diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c > > > index 63502ca537eb..331abab5d066 100644 > > > --- a/drivers/s390/block/dcssblk.c > > > +++ b/drivers/s390/block/dcssblk.c > > > @@ -57,11 +57,28 @@ static size_t dcssblk_dax_copy_to_iter(struct dax_device *dax_dev, > > > return copy_to_iter(addr, bytes, i); > > > } > > > > > > +static int dcssblk_dax_zero_page_range(struct dax_device *dax_dev, u64 offset, > > > + size_t len) > > > +{ > > > + long rc; > > > + void *kaddr; > > > + pgoff_t pgoff = offset >> PAGE_SHIFT; > > > + unsigned page_offset = offset_in_page(offset); > > > + > > > + rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, NULL); > > > > Why do you pass only 1 page as nr_pages argument for dax_direct_access()? > > In some other patch in this series there is a comment that this will > > currently only be used for one page, but support for more pages might be > > added later. Wouldn't it make sense to rather use something like > > PAGE_ALIGN(page_offset + len) >> PAGE_SHIFT instead of 1 here, so that > > this won't have to be changed when callers will be ready to use it > > with more than one page? > > > > Of course, I guess then we'd also need some check on the return value > > from dax_direct_access(), i.e. if the returned available range is > > large enough for the requested range. > > I left it at 1 page because that's the current limitation of this > interface and there are no callers which are zeroing across page > boundaries. > > I prefer to keep it this way and modify it when we are extending this > interface to allow zeroing across page boundaries. Because even if I add > that logic, I can't test it. OK, fine with me. Reviewed-by: Gerald Schaefer