* [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver [not found] <20200207202652.1439-1-vgoyal@redhat.com> @ 2020-02-07 20:26 ` Vivek Goyal 2020-02-10 20:53 ` Gerald Schaefer 0 siblings, 1 reply; 4+ messages in thread From: Vivek Goyal @ 2020-02-07 20:26 UTC (permalink / raw) To: linux-fsdevel, linux-nvdimm, hch, dan.j.williams Cc: dm-devel, vishal.l.verma, vgoyal, linux-s390 Add dax operation zero_page_range for dcssblk driver. CC: linux-s390@vger.kernel.org Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> --- 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); + if (rc < 0) + return rc; + memset(kaddr + page_offset, 0, len); + dax_flush(dax_dev, kaddr + page_offset, len); + return 0; +} + static const struct dax_operations dcssblk_dax_ops = { .direct_access = dcssblk_dax_direct_access, .dax_supported = generic_fsdax_supported, .copy_from_iter = dcssblk_dax_copy_from_iter, .copy_to_iter = dcssblk_dax_copy_to_iter, + .zero_page_range = dcssblk_dax_zero_page_range, }; struct dcssblk_dev_info { -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver 2020-02-07 20:26 ` [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver Vivek Goyal @ 2020-02-10 20:53 ` Gerald Schaefer 2020-02-11 15:11 ` Vivek Goyal 0 siblings, 1 reply; 4+ messages in thread From: Gerald Schaefer @ 2020-02-10 20:53 UTC (permalink / raw) To: Vivek Goyal Cc: linux-fsdevel, linux-nvdimm, hch, dan.j.williams, dm-devel, vishal.l.verma, linux-s390 On Fri, 7 Feb 2020 15:26:49 -0500 Vivek Goyal <vgoyal@redhat.com> wrote: > Add dax operation zero_page_range for dcssblk driver. > > CC: linux-s390@vger.kernel.org > Suggested-by: Christoph Hellwig <hch@infradead.org> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > --- > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver 2020-02-10 20:53 ` Gerald Schaefer @ 2020-02-11 15:11 ` Vivek Goyal 2020-02-11 15:49 ` Gerald Schaefer 0 siblings, 1 reply; 4+ messages in thread From: Vivek Goyal @ 2020-02-11 15:11 UTC (permalink / raw) To: Gerald Schaefer Cc: linux-fsdevel, linux-nvdimm, hch, dan.j.williams, dm-devel, vishal.l.verma, linux-s390 On Mon, Feb 10, 2020 at 09:53:15PM +0100, Gerald Schaefer wrote: > On Fri, 7 Feb 2020 15:26:49 -0500 > Vivek Goyal <vgoyal@redhat.com> wrote: > > > Add dax operation zero_page_range for dcssblk driver. > > > > CC: linux-s390@vger.kernel.org > > Suggested-by: Christoph Hellwig <hch@infradead.org> > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > --- > > 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. But if you still prefer to change it, I am open to make that change. Thanks Vivek ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver 2020-02-11 15:11 ` Vivek Goyal @ 2020-02-11 15:49 ` Gerald Schaefer 0 siblings, 0 replies; 4+ messages in thread From: Gerald Schaefer @ 2020-02-11 15:49 UTC (permalink / raw) To: Vivek Goyal Cc: linux-fsdevel, linux-nvdimm, hch, dan.j.williams, dm-devel, vishal.l.verma, linux-s390 On Tue, 11 Feb 2020 10:11:14 -0500 Vivek Goyal <vgoyal@redhat.com> 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 <vgoyal@redhat.com> wrote: > > > > > Add dax operation zero_page_range for dcssblk driver. > > > > > > CC: linux-s390@vger.kernel.org > > > Suggested-by: Christoph Hellwig <hch@infradead.org> > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > > --- > > > 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 <gerald.schaefer@de.ibm.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-11 15:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200207202652.1439-1-vgoyal@redhat.com>
2020-02-07 20:26 ` [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver Vivek Goyal
2020-02-10 20:53 ` Gerald Schaefer
2020-02-11 15:11 ` Vivek Goyal
2020-02-11 15:49 ` Gerald Schaefer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).