diff mbox series

[2/2] cxl/port: Revert __free() conversion of add_port_attach_ep()

Message ID 172912105192.1414627.12908026603084784872.stgit@dwillia2-xfh.jf.intel.com
State New
Headers show
Series [1/2] cxl/port: Revert usage of scoped_guard() | expand

Commit Message

Dan Williams Oct. 16, 2024, 11:24 p.m. UTC
Dan points out via smatch [1] that the conversion of
add_port_attach_ep() to use cleanup helpers does not correctly handle a
pointer reassignment case.

    drivers/cxl/core/port.c:1591 add_port_attach_ep()
    warn: re-assigning __cleanup__ ptr 'port'

In the case that @port is not found and a new @port is created the
cleanup method changes from __free() to devm.

Revert the change for now as this function is not amenable to cleanup
helpers without a wider refactoring.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: http://lore.kernel.org/2a19289b-0bcf-42c4-82a9-268a922535f2@stanley.mountain [1]
Cc: Li Ming <ming4.li@intel.com>
Fixes: dd2617ebd2a6 ("cxl/port: Use __free() to drop put_device() for cxl_port")
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/port.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 54dd2cd450ca..8e6596e147b3 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1544,6 +1544,7 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 			      struct device *dport_dev)
 {
 	struct device *dparent = grandparent(dport_dev);
+	struct cxl_port *port, *parent_port = NULL;
 	struct cxl_dport *dport, *parent_dport;
 	resource_size_t component_reg_phys;
 	int rc;
@@ -1559,18 +1560,12 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 		return -ENXIO;
 	}
 
-	struct cxl_port *parent_port __free(put_cxl_port) =
-		find_cxl_port(dparent, &parent_dport);
+	parent_port = find_cxl_port(dparent, &parent_dport);
 	if (!parent_port) {
 		/* iterate to create this parent_port */
 		return -EAGAIN;
 	}
 
-	/*
-	 * Definition with __free() here to keep the sequence of
-	 * dereferencing the device of the port before the parent_port releasing.
-	 */
-	struct cxl_port *port __free(put_cxl_port) = NULL;
 	device_lock(&parent_port->dev);
 	if (!parent_port->dev.driver) {
 		dev_warn(&cxlmd->dev,
@@ -1605,8 +1600,10 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 			 */
 			rc = -ENXIO;
 		}
+		put_device(&port->dev);
 	}
 
+	put_device(&parent_port->dev);
 	return rc;
 }