diff mbox series

[v6,05/12] cxl/acpi: Move rescan to the workqueue

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

Commit Message

Dan Williams Dec. 1, 2022, 9:33 p.m. UTC 
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.

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(-)

Comments

Jonathan Cameron Dec. 2, 2022, 3:50 p.m. UTC | #1 
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);
>
Dan Williams Dec. 3, 2022, 7:14 a.m. UTC | #2 
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 mbox series

Patch

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);