| Message ID | 8d062903cded81cba05cc703f61160a0edb4578a.1708709155.git.john@groves.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce the famfs shared-memory file system | expand |
On Fri, 23 Feb 2024 11:41:47 -0600 John Groves <John@Groves.net> wrote: > bus.c can't call functions in device.c - that creates a circular linkage > dependency. > > Signed-off-by: John Groves <john@groves.net> This also adds the export which you should mention! Do they need it already? Seems like tense of patch title may be wrong. > --- > drivers/dax/bus.c | 24 ++++++++++++++++++++++++ > drivers/dax/device.c | 23 ----------------------- > 2 files changed, 24 insertions(+), 23 deletions(-) > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index 1ff1ab5fa105..664e8c1b9930 100644 > --- a/drivers/dax/bus.c > +++ b/drivers/dax/bus.c > @@ -1325,6 +1325,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; Not related to your patch but returning -1 in a phys_addr_t isn't ideal. I assume aim is all bits set as a marker, in which case PHYS_ADDR_MAX from limits.h would make things clearer. > +} > +EXPORT_SYMBOL_GPL(dax_pgoff_to_phys); > + > 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 93ebedc5ec8c..40ba660013cf 100644 > --- a/drivers/dax/device.c > +++ b/drivers/dax/device.c > @@ -50,29 +50,6 @@ static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma, > return 0; > } > > -/* 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, pfn_t pfn, > unsigned long fault_size) > {
On 24/02/26 12:10PM, Jonathan Cameron wrote: > On Fri, 23 Feb 2024 11:41:47 -0600 > John Groves <John@Groves.net> wrote: > > > bus.c can't call functions in device.c - that creates a circular linkage > > dependency. > > > > Signed-off-by: John Groves <john@groves.net> > > This also adds the export which you should mention! > > Do they need it already? Seems like tense of patch title > may be wrong. I added "Also exports dax_pgoff_to_phys() since both bus.c and device.c now call it." The export is necessary because bus.c and device.c are not in the same .ko Let me know if it seems like I'm misunderstanding... > > > --- > > drivers/dax/bus.c | 24 ++++++++++++++++++++++++ > > drivers/dax/device.c | 23 ----------------------- > > 2 files changed, 24 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > > index 1ff1ab5fa105..664e8c1b9930 100644 > > --- a/drivers/dax/bus.c > > +++ b/drivers/dax/bus.c > > @@ -1325,6 +1325,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; > > Not related to your patch but returning -1 in a phys_addr_t isn't ideal. > I assume aim is all bits set as a marker, in which case > PHYS_ADDR_MAX from limits.h would make things clearer. Perhaps Dan or the other dax people can comment on this? I just moved the function verbatim, but Jonathan makes a good point! Thanks, John
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 1ff1ab5fa105..664e8c1b9930 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -1325,6 +1325,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); + 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 93ebedc5ec8c..40ba660013cf 100644 --- a/drivers/dax/device.c +++ b/drivers/dax/device.c @@ -50,29 +50,6 @@ static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma, return 0; } -/* 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, pfn_t pfn, unsigned long fault_size) {
bus.c can't call functions in device.c - that creates a circular linkage dependency. Signed-off-by: John Groves <john@groves.net> --- drivers/dax/bus.c | 24 ++++++++++++++++++++++++ drivers/dax/device.c | 23 ----------------------- 2 files changed, 24 insertions(+), 23 deletions(-)