[PATCH V3 01/21] dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c

John Groves posted 21 patches 1 month ago
[PATCH V3 01/21] dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c
Posted by John Groves 1 month ago
This function will be used by both device.c and fsdev.c, but both are
loadable modules. Moving to bus.c puts it in core and makes it available
to both.

No code changes - just relocated.

Signed-off-by: John Groves <john@groves.net>
---
 drivers/dax/bus.c    | 27 +++++++++++++++++++++++++++
 drivers/dax/device.c | 23 -----------------------
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index fde29e0ad68b..a2f9a3cc30a5 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -7,6 +7,9 @@
 #include <linux/slab.h>
 #include <linux/dax.h>
 #include <linux/io.h>
+#include <linux/backing-dev.h>
+#include <linux/range.h>
+#include <linux/uio.h>
 #include "dax-private.h"
 #include "bus.h"
 
@@ -1417,6 +1420,30 @@ static const struct device_type dev_dax_type = {
 	.groups = dax_attribute_groups,
 };
 
+/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c  */
+__weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff,
+			      unsigned long size)
+{
+	int i;
+
+	for (i = 0; i < dev_dax->nr_range; i++) {
+		struct dev_dax_range *dax_range = &dev_dax->ranges[i];
+		struct range *range = &dax_range->range;
+		unsigned long long pgoff_end;
+		phys_addr_t phys;
+
+		pgoff_end = dax_range->pgoff + PHYS_PFN(range_len(range)) - 1;
+		if (pgoff < dax_range->pgoff || pgoff > pgoff_end)
+			continue;
+		phys = PFN_PHYS(pgoff - dax_range->pgoff) + range->start;
+		if (phys + size - 1 <= range->end)
+			return phys;
+		break;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL(dax_pgoff_to_phys);
+
 static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
 {
 	struct dax_region *dax_region = data->dax_region;
diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index 22999a402e02..132c1d03fd07 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -57,29 +57,6 @@ static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
 			   vma->vm_file, func);
 }
 
-/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c */
-__weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff,
-		unsigned long size)
-{
-	int i;
-
-	for (i = 0; i < dev_dax->nr_range; i++) {
-		struct dev_dax_range *dax_range = &dev_dax->ranges[i];
-		struct range *range = &dax_range->range;
-		unsigned long long pgoff_end;
-		phys_addr_t phys;
-
-		pgoff_end = dax_range->pgoff + PHYS_PFN(range_len(range)) - 1;
-		if (pgoff < dax_range->pgoff || pgoff > pgoff_end)
-			continue;
-		phys = PFN_PHYS(pgoff - dax_range->pgoff) + range->start;
-		if (phys + size - 1 <= range->end)
-			return phys;
-		break;
-	}
-	return -1;
-}
-
 static void dax_set_mapping(struct vm_fault *vmf, unsigned long pfn,
 			      unsigned long fault_size)
 {
-- 
2.49.0
Re: [PATCH V3 01/21] dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c
Posted by Jonathan Cameron 1 month ago
On Wed,  7 Jan 2026 09:33:10 -0600
John Groves <John@Groves.net> wrote:

> This function will be used by both device.c and fsdev.c, but both are
> loadable modules. Moving to bus.c puts it in core and makes it available
> to both.
> 
> No code changes - just relocated.
> 
> Signed-off-by: John Groves <john@groves.net>
Hi John,

I don't know the code well enough to offer an opinion on whether this
move causes any issues or if this is the best location, so review is superficial
stuff only.

Jonathan

> ---
>  drivers/dax/bus.c    | 27 +++++++++++++++++++++++++++
>  drivers/dax/device.c | 23 -----------------------
>  2 files changed, 27 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index fde29e0ad68b..a2f9a3cc30a5 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -7,6 +7,9 @@
>  #include <linux/slab.h>
>  #include <linux/dax.h>
>  #include <linux/io.h>
> +#include <linux/backing-dev.h>

I'm not immediately spotting why this one.  Maybe should be in a different
patch?

> +#include <linux/range.h>
> +#include <linux/uio.h>

Why this one?

Style wise, dax seems to use reverse xmas tree for includes, so
this should keep to that.

>  #include "dax-private.h"
>  #include "bus.h"
>  
> @@ -1417,6 +1420,30 @@ static const struct device_type dev_dax_type = {
>  	.groups = dax_attribute_groups,
>  };
>  
> +/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c  */
Bonus space before that */
Curiously that wasn't there in the original.

> +__weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff,
> +			      unsigned long size)
> +{
> +	int i;
> +
> +	for (i = 0; i < dev_dax->nr_range; i++) {
> +		struct dev_dax_range *dax_range = &dev_dax->ranges[i];
> +		struct range *range = &dax_range->range;
> +		unsigned long long pgoff_end;
> +		phys_addr_t phys;
> +
> +		pgoff_end = dax_range->pgoff + PHYS_PFN(range_len(range)) - 1;
> +		if (pgoff < dax_range->pgoff || pgoff > pgoff_end)
> +			continue;
> +		phys = PFN_PHYS(pgoff - dax_range->pgoff) + range->start;
> +		if (phys + size - 1 <= range->end)
> +			return phys;
> +		break;
> +	}
> +	return -1;
> +}
> +EXPORT_SYMBOL_GPL(dax_pgoff_to_phys);
> +
>  static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
>  {
>  	struct dax_region *dax_region = data->dax_region;
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index 22999a402e02..132c1d03fd07 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -57,29 +57,6 @@ static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
>  			   vma->vm_file, func);
>  }
>  
> -/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c */
> -__weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff,
> -		unsigned long size)
> -{
> -	int i;
> -
> -	for (i = 0; i < dev_dax->nr_range; i++) {
> -		struct dev_dax_range *dax_range = &dev_dax->ranges[i];
> -		struct range *range = &dax_range->range;
> -		unsigned long long pgoff_end;
> -		phys_addr_t phys;
> -
> -		pgoff_end = dax_range->pgoff + PHYS_PFN(range_len(range)) - 1;
> -		if (pgoff < dax_range->pgoff || pgoff > pgoff_end)
> -			continue;
> -		phys = PFN_PHYS(pgoff - dax_range->pgoff) + range->start;
> -		if (phys + size - 1 <= range->end)
> -			return phys;
> -		break;
> -	}
> -	return -1;
> -}
> -
>  static void dax_set_mapping(struct vm_fault *vmf, unsigned long pfn,
>  			      unsigned long fault_size)
>  {
Re: [PATCH V3 01/21] dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c
Posted by John Groves 1 month ago
On 26/01/08 10:43AM, Jonathan Cameron wrote:
> On Wed,  7 Jan 2026 09:33:10 -0600
> John Groves <John@Groves.net> wrote:
> 
> > This function will be used by both device.c and fsdev.c, but both are
> > loadable modules. Moving to bus.c puts it in core and makes it available
> > to both.
> > 
> > No code changes - just relocated.
> > 
> > Signed-off-by: John Groves <john@groves.net>
> Hi John,
> 
> I don't know the code well enough to offer an opinion on whether this
> move causes any issues or if this is the best location, so review is superficial
> stuff only.
> 
> Jonathan
> 
> > ---
> >  drivers/dax/bus.c    | 27 +++++++++++++++++++++++++++
> >  drivers/dax/device.c | 23 -----------------------
> >  2 files changed, 27 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > index fde29e0ad68b..a2f9a3cc30a5 100644
> > --- a/drivers/dax/bus.c
> > +++ b/drivers/dax/bus.c
> > @@ -7,6 +7,9 @@
> >  #include <linux/slab.h>
> >  #include <linux/dax.h>
> >  #include <linux/io.h>
> > +#include <linux/backing-dev.h>
> 
> I'm not immediately spotting why this one.  Maybe should be in a different
> patch?
> 
> > +#include <linux/range.h>
> > +#include <linux/uio.h>
> 
> Why this one?

Good eye, thanks. These must have leaked from some of the many dead ends
that I tried before coming up with this approach.

I've dropped all new includes and it still builds :D

> 
> Style wise, dax seems to use reverse xmas tree for includes, so
> this should keep to that.
> 
> >  #include "dax-private.h"
> >  #include "bus.h"
> >  
> > @@ -1417,6 +1420,30 @@ static const struct device_type dev_dax_type = {
> >  	.groups = dax_attribute_groups,
> >  };
> >  
> > +/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c  */
> Bonus space before that */
> Curiously that wasn't there in the original.

Removed.

[ ... ]

Thanks,
John
Re: [PATCH V3 01/21] dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c
Posted by Jonathan Cameron 1 month ago
On Thu, 8 Jan 2026 07:25:47 -0600
John Groves <John@groves.net> wrote:

> On 26/01/08 10:43AM, Jonathan Cameron wrote:
> > On Wed,  7 Jan 2026 09:33:10 -0600
> > John Groves <John@Groves.net> wrote:
> >   
> > > This function will be used by both device.c and fsdev.c, but both are
> > > loadable modules. Moving to bus.c puts it in core and makes it available
> > > to both.
> > > 
> > > No code changes - just relocated.
> > > 
> > > Signed-off-by: John Groves <john@groves.net>  
> > Hi John,
> > 
> > I don't know the code well enough to offer an opinion on whether this
> > move causes any issues or if this is the best location, so review is superficial
> > stuff only.
> > 
> > Jonathan
> >   
> > > ---
> > >  drivers/dax/bus.c    | 27 +++++++++++++++++++++++++++
> > >  drivers/dax/device.c | 23 -----------------------
> > >  2 files changed, 27 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > > index fde29e0ad68b..a2f9a3cc30a5 100644
> > > --- a/drivers/dax/bus.c
> > > +++ b/drivers/dax/bus.c
> > > @@ -7,6 +7,9 @@
> > >  #include <linux/slab.h>
> > >  #include <linux/dax.h>
> > >  #include <linux/io.h>
> > > +#include <linux/backing-dev.h>  
> > 
> > I'm not immediately spotting why this one.  Maybe should be in a different
> > patch?
> >   
> > > +#include <linux/range.h>
> > > +#include <linux/uio.h>  
> > 
> > Why this one?  
> 
> Good eye, thanks. These must have leaked from some of the many dead ends
> that I tried before coming up with this approach.
> 
> I've dropped all new includes and it still builds :D

Range one should be there... 

> 
> > 
> > Style wise, dax seems to use reverse xmas tree for includes, so
> > this should keep to that.
> >   
> > >  #include "dax-private.h"
> > >  #include "bus.h"
> > >  
> > > @@ -1417,6 +1420,30 @@ static const struct device_type dev_dax_type = {
> > >  	.groups = dax_attribute_groups,
> > >  };
> > >  
> > > +/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c  */  
> > Bonus space before that */
> > Curiously that wasn't there in the original.  
> 
> Removed.
> 
> [ ... ]
> 
> Thanks,
> John