Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 13 Mar 2023 15:51:31 -0700 | From | Ira Weiny <> | Subject | Re: [PATCH v8 4/6] cxl/region: Provide region info to the cxl_poison trace event |
| |
alison.schofield@ wrote: > From: Alison Schofield <alison.schofield@intel.com> > > User space may need to know which region, if any, maps the poison > address(es) logged in a cxl_poison trace event. Since the mapping > of DPAs (device physical addresses) to a region can change, the > kernel must provide this information at the time the poison list > is read. The event informs user space that at event <timestamp> > this <region> mapped to this <DPA>, which is poisoned. > > The cxl_poison trace event is already wired up to log the region > name and uuid if it receives param 'struct cxl_region'. > > In order to provide that cxl_region, add another method for gathering > poison - by committed endpoint decoder mappings. This method is only > available with CONFIG_CXL_REGION and is only used if a region actually > maps the memdev where poison is being read. After the poison list is > read for all the mapped resources, poison is read for the unmapped > resources, and those events are logged without the region info. > > Mixed mode decoders are not currently supported in Linux. Add a debug > message to the poison request path. That will serve as an alert that > poison list retrieval needs to add support for mixed mode. > > The default method remains: read the poison by memdev resource. > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
[...]
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index ea996057815e..5e65818d2171 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -139,14 +139,29 @@ static ssize_t trigger_poison_list_store(struct device *dev, > const char *buf, size_t len) > { > struct cxl_memdev *cxlmd = to_cxl_memdev(dev); > + struct cxl_port *port; > bool trigger; > int rc; > > if (kstrtobool(buf, &trigger) || !trigger) > return -EINVAL; > > + port = dev_get_drvdata(&cxlmd->dev); > + if (!port || !is_cxl_endpoint(port)) > + return -EINVAL; > + > down_read(&cxl_dpa_rwsem); > - rc = cxl_get_poison_by_memdev(cxlmd); > + if (port->commit_end == -1) { > + /* No regions mapped to this memdev */ > + rc = cxl_get_poison_by_memdev(cxlmd); > + } else { > + /* Regions mapped, collect poison by endpoint */ > + rc = device_for_each_child(&port->dev, port, > + cxl_get_poison_by_endpoint); > + if (rc == 1) > + rc = 0; > + } > + > up_read(&cxl_dpa_rwsem); > > return rc ? rc : len; > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index f29028148806..1a558adfe32d 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -2213,6 +2213,102 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev) > } > EXPORT_SYMBOL_NS_GPL(to_cxl_pmem_region, CXL); > > +int cxl_get_poison_by_endpoint(struct device *dev, void *data) > +{ > + struct cxl_endpoint_decoder *cxled; > + struct cxl_port *port = data; > + struct cxl_dev_state *cxlds; > + struct cxl_memdev *cxlmd; > + u64 offset, length; > + int rc = 0; > + > + down_read(&cxl_dpa_rwsem);
Oh just realized aren't we already under the read lock when this is called?
Replacing this with a lockdep_assert() could make the logic here easier.
Ira
[...]
| ![\](/images/icornerr.gif) |