diff mbox series

[v4,1/6] cxl: Introduce put_cxl_root() helper

Message ID 170441748869.3632067.1712163433527825314.stgit@djiang5-mobl3
State New, archived
Headers show
Series find_cxl_root() related cleanups | expand

Commit Message

Dave Jiang Jan. 5, 2024, 1:18 a.m. UTC
Add a helper function put_cxl_root() to maintain symmetry for
find_cxl_root() function instead of relying on open coding of the
put_device() in order to dereference the 'struct device' that happens via
get_device() in find_cxl_root(). Add cleanups for all code paths that
calls put_device() after find_cxl_root().

Suggested-by: Robert Richter <rrichter@amd.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Adjust ordering of this patch to front. (Dan)
v3:
- Adjust for cxl_root as parameter for find_cxl_root()
- Add NULL ptr check fore __free(). (Dan)
- Fix DEFINE_FREE() macro to name it put_cxl_root (Dan)
- Cleanup all functions calling put_cxl_root() and related calls. (Dan)

v2:
- Make put_cxl_root() an exported function to be symmetric to
  find_cxl_root(). (Robert)
---
 drivers/cxl/core/cdat.c |    5 +++--
 drivers/cxl/core/port.c |    9 +++++++++
 drivers/cxl/cxl.h       |    3 +++
 3 files changed, 15 insertions(+), 2 deletions(-)

Comments

Dan Williams Jan. 5, 2024, 3:17 a.m. UTC | #1
Dave Jiang wrote:
> Add a helper function put_cxl_root() to maintain symmetry for
> find_cxl_root() function instead of relying on open coding of the
> put_device() in order to dereference the 'struct device' that happens via
> get_device() in find_cxl_root(). Add cleanups for all code paths that
> calls put_device() after find_cxl_root().
> 
> Suggested-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v4:
> - Adjust ordering of this patch to front. (Dan)
> v3:
> - Adjust for cxl_root as parameter for find_cxl_root()
> - Add NULL ptr check fore __free(). (Dan)
> - Fix DEFINE_FREE() macro to name it put_cxl_root (Dan)
> - Cleanup all functions calling put_cxl_root() and related calls. (Dan)
> 
> v2:
> - Make put_cxl_root() an exported function to be symmetric to
>   find_cxl_root(). (Robert)
> ---
>  drivers/cxl/core/cdat.c |    5 +++--
>  drivers/cxl/core/port.c |    9 +++++++++
>  drivers/cxl/cxl.h       |    3 +++
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index cd84d87f597a..c1085fcc5428 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -349,12 +349,13 @@ static int cxl_qos_class_verify(struct cxl_memdev *cxlmd)
>  {
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> -	struct cxl_port *root_port __free(put_device) = NULL;
>  	LIST_HEAD(__discard);
>  	struct list_head *discard __free(dpa_perf) = &__discard;
>  	int rc;
>  
> -	root_port = find_cxl_root(cxlmd->endpoint);
> +	struct cxl_port *root_port __free(put_cxl_root) =
> +		find_cxl_root(cxlmd->endpoint);

This fix needs to wait until later in the series after find_cxl_root()
is converted to return 'struct cxl_root *'. Likely it should be part of
that very same patch. It's a type-safety fix that comes along for the
ride in the type-safety improvement patch.

> +
>  	if (!root_port)
>  		return -ENODEV;
>  
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 8c00fd6be730..f66650bb6128 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -986,6 +986,15 @@ struct cxl_port *find_cxl_root(struct cxl_port *port)
>  }
>  EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
>  
> +void put_cxl_root(struct cxl_port *port)

This should go straight to 'struct cxl_root *' and just have no users in
this patch.

I can shuffle that around now that the patch ordering is mostly fixed
up, but in general if you find yourself editing a line twice in the same
patch series that should set off alarm bells.
diff mbox series

Patch

diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index cd84d87f597a..c1085fcc5428 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -349,12 +349,13 @@  static int cxl_qos_class_verify(struct cxl_memdev *cxlmd)
 {
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
-	struct cxl_port *root_port __free(put_device) = NULL;
 	LIST_HEAD(__discard);
 	struct list_head *discard __free(dpa_perf) = &__discard;
 	int rc;
 
-	root_port = find_cxl_root(cxlmd->endpoint);
+	struct cxl_port *root_port __free(put_cxl_root) =
+		find_cxl_root(cxlmd->endpoint);
+
 	if (!root_port)
 		return -ENODEV;
 
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 8c00fd6be730..f66650bb6128 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -986,6 +986,15 @@  struct cxl_port *find_cxl_root(struct cxl_port *port)
 }
 EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
 
+void put_cxl_root(struct cxl_port *port)
+{
+	if (!port)
+		return;
+
+	put_device(&port->dev);
+}
+EXPORT_SYMBOL_NS_GPL(put_cxl_root);
+
 static struct cxl_dport *find_dport(struct cxl_port *port, int id)
 {
 	struct cxl_dport *dport;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 492dbf63935f..4e53604de041 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -735,6 +735,9 @@  struct cxl_port *devm_cxl_add_port(struct device *host,
 struct cxl_root *devm_cxl_add_root(struct device *host,
 				   const struct cxl_root_ops *ops);
 struct cxl_port *find_cxl_root(struct cxl_port *port);
+void put_cxl_root(struct cxl_port *port);
+DEFINE_FREE(put_cxl_root, struct cxl_port *, if (_T) put_cxl_root(_T))
+
 int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
 void cxl_bus_rescan(void);
 void cxl_bus_drain(void);