| 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
| ||
|
Message-ID: <20250915112555.00003854@huawei.com>
Date: Mon, 15 Sep 2025 11:25:55 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Robert Richter <rrichter@....com>
CC: Dave Jiang <dave.jiang@...el.com>, Alison Schofield
<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Ira
Weiny" <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
Davidlohr Bueso <dave@...olabs.net>, <linux-cxl@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Gregory Price <gourry@...rry.net>, "Fabio M.
De Francesco" <fabio.m.de.francesco@...ux.intel.com>, Terry Bowman
<terry.bowman@....com>, Joshua Hahn <joshua.hahnjy@...il.com>
Subject: Re: [PATCH v3 03/11] cxl/region: Rename misleading variable name
@hpa to @range
On Mon, 15 Sep 2025 09:27:36 +0200
Robert Richter <rrichter@....com> wrote:
> On 12.09.25 10:33:30, Dave Jiang wrote:
> >
> >
> > On 9/12/25 7:45 AM, Robert Richter wrote:
> > > @hpa is actually a @range, rename variables accordingly.
> >
> > it's a range of HPA right? May as well call it 'hpa_range' to distinguish from 'dpa_range' or 'spa_range'
>
> To me this is ok as it is locally used only in the functions. I used
> the short version to not hit the 80 char line limit in some of the
> statements for readability. Range is most of the time HPA unless for
> special cases, which use a prefix then. It also becomes clear viewed
> in context, e.g.
>
> range = &cxld->hpa_range;
>
> Thus, I rather would like to not change that.
I'm with Dave on this one. Better to have some slightly long lines
and avoid any potential confusion.
Jonathan
>
> -Robert
>
> >
> > >
> > > Signed-off-by: Robert Richter <rrichter@....com>
> > > ---
> > > drivers/cxl/core/region.c | 29 +++++++++++++++--------------
> > > 1 file changed, 15 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > > index 777d04870180..13113920aba7 100644
> > > --- a/drivers/cxl/core/region.c
> > > +++ b/drivers/cxl/core/region.c
> > > @@ -3367,9 +3367,9 @@ static int match_decoder_by_range(struct device *dev, const void *data)
> > > }
> > >
> > > static struct cxl_decoder *
> > > -cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
> > > +cxl_port_find_switch_decoder(struct cxl_port *port, struct range *range)
> > > {
> > > - struct device *cxld_dev = device_find_child(&port->dev, hpa,
> > > + struct device *cxld_dev = device_find_child(&port->dev, range,
> > > match_decoder_by_range);
> > >
> > > return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
> > > @@ -3382,14 +3382,14 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
> > > struct cxl_port *port = cxled_to_port(cxled);
> > > struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
> > > struct cxl_decoder *root, *cxld = &cxled->cxld;
> > > - struct range *hpa = &cxld->hpa_range;
> > > + struct range *range = &cxld->hpa_range;
> > >
> > > - root = cxl_port_find_switch_decoder(&cxl_root->port, hpa);
> > > + root = cxl_port_find_switch_decoder(&cxl_root->port, range);
> > > if (!root) {
> > > dev_err(cxlmd->dev.parent,
> > > "%s:%s no CXL window for range %#llx:%#llx\n",
> > > dev_name(&cxlmd->dev), dev_name(&cxld->dev),
> > > - cxld->hpa_range.start, cxld->hpa_range.end);
> > > + range->start, range->end);
> > > return NULL;
> > > }
> > >
> > > @@ -3458,7 +3458,7 @@ static int __construct_region(struct cxl_region *cxlr,
> > > {
> > > struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> > > struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> > > - struct range *hpa = &cxled->cxld.hpa_range;
> > > + struct range *range = &cxled->cxld.hpa_range;
> > > struct cxl_region_params *p;
> > > struct resource *res;
> > > int rc;
> > > @@ -3474,13 +3474,13 @@ static int __construct_region(struct cxl_region *cxlr,
> > > }
> > >
> > > set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
> > > - cxlr->hpa_range = *hpa;
> > > + cxlr->hpa_range = *range;
> > >
> > > res = kmalloc(sizeof(*res), GFP_KERNEL);
> > > if (!res)
> > > return -ENOMEM;
> > >
> > > - *res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
> > > + *res = DEFINE_RES_MEM_NAMED(range->start, range_len(range),
> > > dev_name(&cxlr->dev));
> > >
> > > rc = cxl_extended_linear_cache_resize(cxlr, res);
> > > @@ -3559,11 +3559,11 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> > > }
> > >
> > > static struct cxl_region *
> > > -cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
> > > +cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *range)
> > > {
> > > struct device *region_dev;
> > >
> > > - region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
> > > + region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, range,
> > > match_region_by_range);
> > > if (!region_dev)
> > > return NULL;
> > > @@ -3573,7 +3573,7 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
> > >
> > > int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
> > > {
> > > - struct range *hpa = &cxled->cxld.hpa_range;
> > > + struct range *range = &cxled->cxld.hpa_range;
> > > struct cxl_region_params *p;
> > > bool attach = false;
> > > int rc;
> > > @@ -3584,12 +3584,13 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
> > > return -ENXIO;
> > >
> > > /*
> > > - * Ensure that if multiple threads race to construct_region() for @hpa
> > > - * one does the construction and the others add to that.
> > > + * Ensure that if multiple threads race to construct_region()
> > > + * for the HPA range one does the construction and the others
> > > + * add to that.
> > > */
> > > mutex_lock(&cxlrd->range_lock);
> > > struct cxl_region *cxlr __free(put_cxl_region) =
> > > - cxl_find_region_by_range(cxlrd, hpa);
> > > + cxl_find_region_by_range(cxlrd, range);
> > > if (!cxlr)
> > > cxlr = construct_region(cxlrd, cxled);
> > > mutex_unlock(&cxlrd->range_lock);
> >
Powered by blists - more mailing lists