| Message ID | 166993042884.1882361.5633723613683058881.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | Accepted |
| Commit | 4029c32fb601d505dfb92bdf0db9fdcc41fe1434 |
| Headers | show |
| Series | cxl: Add support for Restricted CXL hosts (RCD mode) | expand |
On Thu, 01 Dec 2022 13:33:48 -0800 Dan Williams <dan.j.williams@intel.com> wrote: > Now that the cxl_mem driver has a need to take the root device lock, the > cxl_bus_rescan() needs to run outside of the root lock context. If possible add a bit more detail here or a reference to the patch that needs to take the root device lock. Change seems fine otherwise, so FWIW Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Tested-by: Robert Richter <rrichter@amd.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/cxl/acpi.c | 17 +++++++++++++++-- > drivers/cxl/core/port.c | 19 +++++++++++++++++-- > drivers/cxl/cxl.h | 3 ++- > 3 files changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index c540da0cbf1e..b8407b77aff6 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -511,7 +511,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) > return rc; > > /* In case PCI is scanned before ACPI re-trigger memdev attach */ > - return cxl_bus_rescan(); > + cxl_bus_rescan(); > + return 0; > } > > static const struct acpi_device_id cxl_acpi_ids[] = { > @@ -535,7 +536,19 @@ static struct platform_driver cxl_acpi_driver = { > .id_table = cxl_test_ids, > }; > > -module_platform_driver(cxl_acpi_driver); > +static int __init cxl_acpi_init(void) > +{ > + return platform_driver_register(&cxl_acpi_driver); > +} > + > +static void __exit cxl_acpi_exit(void) > +{ > + platform_driver_unregister(&cxl_acpi_driver); > + cxl_bus_drain(); > +} > + > +module_init(cxl_acpi_init); > +module_exit(cxl_acpi_exit); > MODULE_LICENSE("GPL v2"); > MODULE_IMPORT_NS(CXL); > MODULE_IMPORT_NS(ACPI); > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 0d2f5eaaca7d..d225267c69bb 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -1844,12 +1844,27 @@ static void cxl_bus_remove(struct device *dev) > > static struct workqueue_struct *cxl_bus_wq; > > -int cxl_bus_rescan(void) > +static void cxl_bus_rescan_queue(struct work_struct *w) > { > - return bus_rescan_devices(&cxl_bus_type); > + int rc = bus_rescan_devices(&cxl_bus_type); > + > + pr_debug("CXL bus rescan result: %d\n", rc); > +} > + > +void cxl_bus_rescan(void) > +{ > + static DECLARE_WORK(rescan_work, cxl_bus_rescan_queue); > + > + queue_work(cxl_bus_wq, &rescan_work); > } > EXPORT_SYMBOL_NS_GPL(cxl_bus_rescan, CXL); > > +void cxl_bus_drain(void) > +{ > + drain_workqueue(cxl_bus_wq); > +} > +EXPORT_SYMBOL_NS_GPL(cxl_bus_drain, CXL); > + > bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd) > { > return queue_work(cxl_bus_wq, &cxlmd->detach_work); > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index f0ca2d768385..281b1db5a271 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -552,7 +552,8 @@ int devm_cxl_add_endpoint(struct cxl_memdev *cxlmd, > struct cxl_dport *parent_dport); > struct cxl_port *find_cxl_root(struct device *dev); > int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); > -int cxl_bus_rescan(void); > +void cxl_bus_rescan(void); > +void cxl_bus_drain(void); > struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd, > struct cxl_dport **dport); > bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd); >
Jonathan Cameron wrote: > On Thu, 01 Dec 2022 13:33:48 -0800 > Dan Williams <dan.j.williams@intel.com> wrote: > > > Now that the cxl_mem driver has a need to take the root device lock, the > > cxl_bus_rescan() needs to run outside of the root lock context. > > If possible add a bit more detail here or a reference to the patch > that needs to take the root device lock. Sure, updated to: cxl/acpi: Move rescan to the workqueue Now that the cxl_mem driver has a need to take the root device lock, the cxl_bus_rescan() needs to run outside of the root lock context. That need arises from RCH topologies and the locking that the cxl_mem driver does to attach a descendant to an upstream port. In the RCH case the lock needed is the CXL root device lock [1]. Link: http://lore.kernel.org/r/166993045621.1882361.1730100141527044744.stgit@dwillia2-xfh.jf.intel.com [1]
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index c540da0cbf1e..b8407b77aff6 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -511,7 +511,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) return rc; /* In case PCI is scanned before ACPI re-trigger memdev attach */ - return cxl_bus_rescan(); + cxl_bus_rescan(); + return 0; } static const struct acpi_device_id cxl_acpi_ids[] = { @@ -535,7 +536,19 @@ static struct platform_driver cxl_acpi_driver = { .id_table = cxl_test_ids, }; -module_platform_driver(cxl_acpi_driver); +static int __init cxl_acpi_init(void) +{ + return platform_driver_register(&cxl_acpi_driver); +} + +static void __exit cxl_acpi_exit(void) +{ + platform_driver_unregister(&cxl_acpi_driver); + cxl_bus_drain(); +} + +module_init(cxl_acpi_init); +module_exit(cxl_acpi_exit); MODULE_LICENSE("GPL v2"); MODULE_IMPORT_NS(CXL); MODULE_IMPORT_NS(ACPI); diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 0d2f5eaaca7d..d225267c69bb 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -1844,12 +1844,27 @@ static void cxl_bus_remove(struct device *dev) static struct workqueue_struct *cxl_bus_wq; -int cxl_bus_rescan(void) +static void cxl_bus_rescan_queue(struct work_struct *w) { - return bus_rescan_devices(&cxl_bus_type); + int rc = bus_rescan_devices(&cxl_bus_type); + + pr_debug("CXL bus rescan result: %d\n", rc); +} + +void cxl_bus_rescan(void) +{ + static DECLARE_WORK(rescan_work, cxl_bus_rescan_queue); + + queue_work(cxl_bus_wq, &rescan_work); } EXPORT_SYMBOL_NS_GPL(cxl_bus_rescan, CXL); +void cxl_bus_drain(void) +{ + drain_workqueue(cxl_bus_wq); +} +EXPORT_SYMBOL_NS_GPL(cxl_bus_drain, CXL); + bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd) { return queue_work(cxl_bus_wq, &cxlmd->detach_work); diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index f0ca2d768385..281b1db5a271 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -552,7 +552,8 @@ int devm_cxl_add_endpoint(struct cxl_memdev *cxlmd, struct cxl_dport *parent_dport); struct cxl_port *find_cxl_root(struct device *dev); int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); -int cxl_bus_rescan(void); +void cxl_bus_rescan(void); +void cxl_bus_drain(void); struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd, struct cxl_dport **dport); bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd);