[PATCH 0/3] DAX Small clean ups

[PATCH 0/3] DAX Small clean ups

[ira.weiny]

From: Ira Weiny <ira.weiny@intel.com>

The following are 3 unrelated small clean ups in the dax area.  They were found
while working on PMEM stray write protection.

Ira Weiny (3):
  fs/fuse: Remove unneeded kaddr parameter
  fs/dax: Clarify nr_pages to dax_direct_access()
  dax: Ensure errno is returned from dax_direct_access

 drivers/dax/super.c | 2 +-
 fs/dax.c            | 2 +-
 fs/fuse/dax.c       | 6 ++----
 3 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.28.0.rc0.12.gb6a658bd00c9

[PATCH 1/3] fs/fuse: Remove unneeded kaddr parameter

[ira.weiny]

From: Ira Weiny <ira.weiny@intel.com>

fuse_dax_mem_range_init() does not need the address or the pfn of the
memory requested in dax_direct_access().  It is only calling direct
access to get the number of pages.

Remove the unused variables and stop requesting the kaddr and pfn from
dax_direct_access().

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 fs/fuse/dax.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index ff99ab2a3c43..34f8a5635c7f 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1234,8 +1234,6 @@ void fuse_dax_conn_free(struct fuse_conn *fc)
 static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
 {
 	long nr_pages, nr_ranges;
-	void *kaddr;
-	pfn_t pfn;
 	struct fuse_dax_mapping *range;
 	int ret, id;
 	size_t dax_size = -1;
@@ -1247,8 +1245,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
 	INIT_DELAYED_WORK(&fcd->free_work, fuse_dax_free_mem_worker);
 
 	id = dax_read_lock();
-	nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), &kaddr,
-				     &pfn);
+	nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), NULL,
+				     NULL);
 	dax_read_unlock(id);
 	if (nr_pages < 0) {
 		pr_debug("dax_direct_access() returned %ld\n", nr_pages);
-- 
2.28.0.rc0.12.gb6a658bd00c9

[PATCH 2/3] fs/dax: Clarify nr_pages to dax_direct_access()

[ira.weiny]

From: Ira Weiny <ira.weiny@intel.com>

dax_direct_access() takes a number of pages.  PHYS_PFN(PAGE_SIZE) is a
very round about way to specify '1'.

Change the nr_pages parameter to the explicit value of '1'.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 fs/dax.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dax.c b/fs/dax.c
index b3d27fdc6775..07621322aeee 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -710,7 +710,7 @@ static int copy_cow_page_dax(struct block_device *bdev, struct dax_device *dax_d
 		return rc;
 
 	id = dax_read_lock();
-	rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(PAGE_SIZE), &kaddr, NULL);
+	rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, NULL);
 	if (rc < 0) {
 		dax_read_unlock(id);
 		return rc;
-- 
2.28.0.rc0.12.gb6a658bd00c9

[PATCH 3/3] dax: Ensure errno is returned from dax_direct_access

[ira.weiny]

From: Ira Weiny <ira.weiny@intel.com>

If the caller specifies a negative nr_pages that is an invalid
parameter.

Return -EINVAL to ensure callers get an errno if they want to check it.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/dax/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 5fa6ae9dbc8b..44736cbd446e 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -313,7 +313,7 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
 		return -ENXIO;
 
 	if (nr_pages < 0)
-		return nr_pages;
+		return -EINVAL;
 
 	avail = dax_dev->ops->direct_access(dax_dev, pgoff, nr_pages,
 			kaddr, pfn);
-- 
2.28.0.rc0.12.gb6a658bd00c9

Re: [PATCH 1/3] fs/fuse: Remove unneeded kaddr parameter

[Ira Weiny]

On Tue, May 25, 2021 at 10:24:26AM -0700, 'Ira Weiny' wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> fuse_dax_mem_range_init() does not need the address or the pfn of the
> memory requested in dax_direct_access().  It is only calling direct
> access to get the number of pages.

In looking for feedback on this small series I realize that I failed to email
Miklos for the fs/fuse patch.

I'm adding Miklos to the To line...

For the rest of the series is there any feedback?

Ira

> 
> Remove the unused variables and stop requesting the kaddr and pfn from
> dax_direct_access().
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  fs/fuse/dax.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index ff99ab2a3c43..34f8a5635c7f 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -1234,8 +1234,6 @@ void fuse_dax_conn_free(struct fuse_conn *fc)
>  static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
>  {
>  	long nr_pages, nr_ranges;
> -	void *kaddr;
> -	pfn_t pfn;
>  	struct fuse_dax_mapping *range;
>  	int ret, id;
>  	size_t dax_size = -1;
> @@ -1247,8 +1245,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
>  	INIT_DELAYED_WORK(&fcd->free_work, fuse_dax_free_mem_worker);
>  
>  	id = dax_read_lock();
> -	nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), &kaddr,
> -				     &pfn);
> +	nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), NULL,
> +				     NULL);
>  	dax_read_unlock(id);
>  	if (nr_pages < 0) {
>  		pr_debug("dax_direct_access() returned %ld\n", nr_pages);
> -- 
> 2.28.0.rc0.12.gb6a658bd00c9
> 

Re: [PATCH 1/3] fs/fuse: Remove unneeded kaddr parameter

[Miklos Szeredi]

On Fri, 11 Jun 2021 at 19:23, Ira Weiny <ira.weiny@intel.com> wrote:
>
> On Tue, May 25, 2021 at 10:24:26AM -0700, 'Ira Weiny' wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > fuse_dax_mem_range_init() does not need the address or the pfn of the
> > memory requested in dax_direct_access().  It is only calling direct
> > access to get the number of pages.
>
> In looking for feedback on this small series I realize that I failed to email
> Miklos for the fs/fuse patch.
>
> I'm adding Miklos to the To line...

LGTM, but this is Vivek's code, so adding Cc.

Thanks,
Miklos


>
> For the rest of the series is there any feedback?
>
> Ira
>
> >
> > Remove the unused variables and stop requesting the kaddr and pfn from
> > dax_direct_access().
> >
> > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
> >  fs/fuse/dax.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> > index ff99ab2a3c43..34f8a5635c7f 100644
> > --- a/fs/fuse/dax.c
> > +++ b/fs/fuse/dax.c
> > @@ -1234,8 +1234,6 @@ void fuse_dax_conn_free(struct fuse_conn *fc)
> >  static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
> >  {
> >       long nr_pages, nr_ranges;
> > -     void *kaddr;
> > -     pfn_t pfn;
> >       struct fuse_dax_mapping *range;
> >       int ret, id;
> >       size_t dax_size = -1;
> > @@ -1247,8 +1245,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
> >       INIT_DELAYED_WORK(&fcd->free_work, fuse_dax_free_mem_worker);
> >
> >       id = dax_read_lock();
> > -     nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), &kaddr,
> > -                                  &pfn);
> > +     nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), NULL,
> > +                                  NULL);
> >       dax_read_unlock(id);
> >       if (nr_pages < 0) {
> >               pr_debug("dax_direct_access() returned %ld\n", nr_pages);
> > --
> > 2.28.0.rc0.12.gb6a658bd00c9
> >

Re: [PATCH 1/3] fs/fuse: Remove unneeded kaddr parameter

[Vivek Goyal]

On Fri, Jun 18, 2021 at 09:01:39AM +0200, Miklos Szeredi wrote:
> On Fri, 11 Jun 2021 at 19:23, Ira Weiny <ira.weiny@intel.com> wrote:
> >
> > On Tue, May 25, 2021 at 10:24:26AM -0700, 'Ira Weiny' wrote:
> > > From: Ira Weiny <ira.weiny@intel.com>
> > >
> > > fuse_dax_mem_range_init() does not need the address or the pfn of the
> > > memory requested in dax_direct_access().  It is only calling direct
> > > access to get the number of pages.
> >
> > In looking for feedback on this small series I realize that I failed to email
> > Miklos for the fs/fuse patch.
> >
> > I'm adding Miklos to the To line...
> 
> LGTM, but this is Vivek's code, so adding Cc.
> 

Looks good to me as well. We are not using pfn and kaddr in
fuse_dax_mem_range_init().

Reviewed-by: Vivek Goyal <vgoyal@redhat.com>

Thanks
Vivek

> Thanks,
> Miklos
> 
> 
> >
> > For the rest of the series is there any feedback?
> >
> > Ira
> >
> > >
> > > Remove the unused variables and stop requesting the kaddr and pfn from
> > > dax_direct_access().
> > >
> > > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > > ---
> > >  fs/fuse/dax.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> > > index ff99ab2a3c43..34f8a5635c7f 100644
> > > --- a/fs/fuse/dax.c
> > > +++ b/fs/fuse/dax.c
> > > @@ -1234,8 +1234,6 @@ void fuse_dax_conn_free(struct fuse_conn *fc)
> > >  static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
> > >  {
> > >       long nr_pages, nr_ranges;
> > > -     void *kaddr;
> > > -     pfn_t pfn;
> > >       struct fuse_dax_mapping *range;
> > >       int ret, id;
> > >       size_t dax_size = -1;
> > > @@ -1247,8 +1245,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
> > >       INIT_DELAYED_WORK(&fcd->free_work, fuse_dax_free_mem_worker);
> > >
> > >       id = dax_read_lock();
> > > -     nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), &kaddr,
> > > -                                  &pfn);
> > > +     nr_pages = dax_direct_access(fcd->dev, 0, PHYS_PFN(dax_size), NULL,
> > > +                                  NULL);
> > >       dax_read_unlock(id);
> > >       if (nr_pages < 0) {
> > >               pr_debug("dax_direct_access() returned %ld\n", nr_pages);
> > > --
> > > 2.28.0.rc0.12.gb6a658bd00c9
> > >
>