[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240226121035.00007ca4@Huawei.com>
Date: Mon, 26 Feb 2024 12:10:35 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: John Groves <John@...ves.net>
CC: John Groves <jgroves@...ron.com>, Jonathan Corbet <corbet@....net>, "Dan
Williams" <dan.j.williams@...el.com>, Vishal Verma
<vishal.l.verma@...el.com>, Dave Jiang <dave.jiang@...el.com>, "Alexander
Viro" <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, "Jan
Kara" <jack@...e.cz>, Matthew Wilcox <willy@...radead.org>,
<linux-cxl@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<nvdimm@...ts.linux.dev>, <john@...alactic.com>, Dave Chinner
<david@...morbit.com>, Christoph Hellwig <hch@...radead.org>,
<dave.hansen@...ux.intel.com>, <gregory.price@...verge.com>
Subject: Re: [RFC PATCH 03/20] dev_dax_iomap: Move dax_pgoff_to_phys from
device.c to bus.c since both need it now
On Fri, 23 Feb 2024 11:41:47 -0600
John Groves <John@...ves.net> wrote:
> bus.c can't call functions in device.c - that creates a circular linkage
> dependency.
>
> Signed-off-by: John Groves <john@...ves.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)
> {
Powered by blists - more mailing lists