lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 26 Feb 2024 09:13:38 -0600
From: John Groves <John@...ves.net>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
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 24/02/26 12:10PM, Jonathan Cameron wrote:
> 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.

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ