diff mbox series

[1/3] cxl/port: Use __free() to drop put_device() for cxl_port

Message ID 20240813070552.3353530-1-ming4.li@intel.com
State Superseded
Headers show
Series [1/3] cxl/port: Use __free() to drop put_device() for cxl_port | expand

Commit Message

Li, Ming4 Aug. 13, 2024, 7:05 a.m. UTC
Using scope-based resource management __free() marco with a new helper
called put_cxl_port() to drop open coded the put_device() used to
dereference the 'struct device' of a cxl_port.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Li Ming <ming4.li@intel.com>
---
 drivers/cxl/core/pci.c  |  6 ++----
 drivers/cxl/core/port.c | 25 +++++++++----------------
 drivers/cxl/cxl.h       |  2 ++
 drivers/cxl/mem.c       |  5 ++---
 drivers/cxl/pci.c       |  7 ++-----
 5 files changed, 17 insertions(+), 28 deletions(-)

Comments

Ira Weiny Aug. 21, 2024, 9:37 p.m. UTC | #1
Li Ming wrote:
> Using scope-based resource management __free() marco with a new helper
> called put_cxl_port() to drop open coded the put_device() used to
> dereference the 'struct device' of a cxl_port.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Li Ming <ming4.li@intel.com>
> ---
>  drivers/cxl/core/pci.c  |  6 ++----
>  drivers/cxl/core/port.c | 25 +++++++++----------------
>  drivers/cxl/cxl.h       |  2 ++
>  drivers/cxl/mem.c       |  5 ++---
>  drivers/cxl/pci.c       |  7 ++-----
>  5 files changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 51132a575b27..4725e37d90fb 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -915,15 +915,13 @@ static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
>  	struct aer_capability_regs aer_regs;
>  	struct cxl_dport *dport;
> -	struct cxl_port *port;
>  	int severity;
>  
> -	port = cxl_pci_find_port(pdev, &dport);
> +	struct cxl_port *port __free(put_cxl_port) =
> +		cxl_pci_find_port(pdev, &dport);
>  	if (!port)
>  		return;
>  
> -	put_device(&port->dev);

I don't think this is wrong but we are not holding the lock for the
duration of the function where before we were not.

It seems like this is somewhat an abuse of the cxl_pci_find_port() call in
that we don't really need the cxl_port reference but rather the dport...  :-/

> -
>  	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
>  		return;
>  
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 1d5007e3795a..6119cb3ad25c 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1472,7 +1472,7 @@ static void cxl_detach_ep(void *data)
>  	struct cxl_memdev *cxlmd = data;
>  
>  	for (int i = cxlmd->depth - 1; i >= 1; i--) {
> -		struct cxl_port *port, *parent_port;
> +		struct cxl_port *parent_port;
>  		struct detach_ctx ctx = {
>  			.cxlmd = cxlmd,
>  			.depth = i,
> @@ -1485,7 +1485,7 @@ static void cxl_detach_ep(void *data)
>  				      port_has_memdev);
>  		if (!dev)
>  			continue;
> -		port = to_cxl_port(dev);
> +		struct cxl_port *port __free(put_cxl_port) = to_cxl_port(dev);

This threw me because 'to_cxl_port' does not take a reference...

Seems ok though.

>  
>  		parent_port = to_cxl_port(port->dev.parent);
>  		device_lock(&parent_port->dev);
> @@ -1512,7 +1512,6 @@ static void cxl_detach_ep(void *data)
>  				dev_name(&port->dev));
>  			delete_switch_port(port);
>  		}
> -		put_device(&port->dev);
>  		device_unlock(&parent_port->dev);
>  	}
>  }
> @@ -1539,8 +1538,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>  			      struct device *uport_dev,
>  			      struct device *dport_dev)
>  {
> +	struct cxl_port *port __free(put_cxl_port) = NULL;
>  	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;
> @@ -1556,7 +1555,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>  		return -ENXIO;
>  	}
>  
> -	parent_port = find_cxl_port(dparent, &parent_dport);
> +	struct cxl_port *parent_port __free(put_cxl_port) =
> +		find_cxl_port(dparent, &parent_dport);
>  	if (!parent_port) {
>  		/* iterate to create this parent_port */
>  		return -EAGAIN;
> @@ -1596,10 +1596,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>  			 */
>  			rc = -ENXIO;
>  		}
> -		put_device(&port->dev);
>  	}
>  
> -	put_device(&parent_port->dev);
>  	return rc;
>  }
>  
> @@ -1630,7 +1628,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  		struct device *dport_dev = grandparent(iter);
>  		struct device *uport_dev;
>  		struct cxl_dport *dport;
> -		struct cxl_port *port;
>  
>  		/*
>  		 * The terminal "grandparent" in PCI is NULL and @platform_bus
> @@ -1649,7 +1646,8 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  		dev_dbg(dev, "scan: iter: %s dport_dev: %s parent: %s\n",
>  			dev_name(iter), dev_name(dport_dev),
>  			dev_name(uport_dev));
> -		port = find_cxl_port(dport_dev, &dport);
> +		struct cxl_port *port __free(put_cxl_port) =

Does __free() get called before the next iteration of the loop?  I guess
it does because it would be out of scope outside the loop?

Ira

> +			find_cxl_port(dport_dev, &dport);
>  		if (port) {
>  			dev_dbg(&cxlmd->dev,
>  				"found already registered port %s:%s\n",
> @@ -1664,18 +1662,13 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  			 * the parent_port lock as the current port may be being
>  			 * reaped.
>  			 */
> -			if (rc && rc != -EBUSY) {
> -				put_device(&port->dev);
> +			if (rc && rc != -EBUSY)
>  				return rc;
> -			}
>  
>  			/* Any more ports to add between this one and the root? */
> -			if (!dev_is_cxl_root_child(&port->dev)) {
> -				put_device(&port->dev);
> +			if (!dev_is_cxl_root_child(&port->dev))
>  				continue;
> -			}
>  
> -			put_device(&port->dev);
>  			return 0;
>  		}
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 9afb407d438f..cad297fba700 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -744,6 +744,8 @@ struct cxl_root *find_cxl_root(struct cxl_port *port);
>  void put_cxl_root(struct cxl_root *cxl_root);
>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_cxl_root(_T))
>  
> +DEFINE_FREE(put_cxl_port, struct cxl_port *,
> +	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
>  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
>  void cxl_bus_rescan(void);
>  void cxl_bus_drain(void);
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 7de232eaeb17..ab9b8ab8df44 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -109,7 +109,6 @@ static int cxl_mem_probe(struct device *dev)
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct device *endpoint_parent;
> -	struct cxl_port *parent_port;
>  	struct cxl_dport *dport;
>  	struct dentry *dentry;
>  	int rc;
> @@ -146,7 +145,8 @@ static int cxl_mem_probe(struct device *dev)
>  	if (rc)
>  		return rc;
>  
> -	parent_port = cxl_mem_find_port(cxlmd, &dport);
> +	struct cxl_port *parent_port __free(put_cxl_port) =
> +		cxl_mem_find_port(cxlmd, &dport);
>  	if (!parent_port) {
>  		dev_err(dev, "CXL port topology not found\n");
>  		return -ENXIO;
> @@ -179,7 +179,6 @@ static int cxl_mem_probe(struct device *dev)
>  	rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
>  unlock:
>  	device_unlock(endpoint_parent);
> -	put_device(&parent_port->dev);
>  	if (rc)
>  		return rc;
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 4be35dc22202..26e75499abdd 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -473,7 +473,6 @@ static bool is_cxl_restricted(struct pci_dev *pdev)
>  static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
>  				  struct cxl_register_map *map)
>  {
> -	struct cxl_port *port;
>  	struct cxl_dport *dport;
>  	resource_size_t component_reg_phys;
>  
> @@ -482,14 +481,12 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
>  		.resource = CXL_RESOURCE_NONE,
>  	};
>  
> -	port = cxl_pci_find_port(pdev, &dport);
> +	struct cxl_port *port __free(put_cxl_port) =
> +		cxl_pci_find_port(pdev, &dport);
>  	if (!port)
>  		return -EPROBE_DEFER;
>  
>  	component_reg_phys = cxl_rcd_component_reg_phys(&pdev->dev, dport);
> -
> -	put_device(&port->dev);
> -
>  	if (component_reg_phys == CXL_RESOURCE_NONE)
>  		return -ENXIO;
>  
> -- 
> 2.40.1
>
Li, Ming4 Aug. 22, 2024, 1:26 a.m. UTC | #2
On 8/22/2024 5:37 AM, Ira Weiny wrote:
> Li Ming wrote:
>> Using scope-based resource management __free() marco with a new helper
>> called put_cxl_port() to drop open coded the put_device() used to
>> dereference the 'struct device' of a cxl_port.
>>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Li Ming <ming4.li@intel.com>
>> ---
>>  drivers/cxl/core/pci.c  |  6 ++----
>>  drivers/cxl/core/port.c | 25 +++++++++----------------
>>  drivers/cxl/cxl.h       |  2 ++
>>  drivers/cxl/mem.c       |  5 ++---
>>  drivers/cxl/pci.c       |  7 ++-----
>>  5 files changed, 17 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 51132a575b27..4725e37d90fb 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -915,15 +915,13 @@ static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
>>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
>>  	struct aer_capability_regs aer_regs;
>>  	struct cxl_dport *dport;
>> -	struct cxl_port *port;
>>  	int severity;
>>  
>> -	port = cxl_pci_find_port(pdev, &dport);
>> +	struct cxl_port *port __free(put_cxl_port) =
>> +		cxl_pci_find_port(pdev, &dport);
>>  	if (!port)
>>  		return;
>>  
>> -	put_device(&port->dev);
> I don't think this is wrong but we are not holding the lock for the
> duration of the function where before we were not.
>
> It seems like this is somewhat an abuse of the cxl_pci_find_port() call in
> that we don't really need the cxl_port reference but rather the dport...  :-/

Thank you for the review.


Yes, seems like that, but CXL core does not provide a helper function to find a dport via pci device without getting the cxl_port reference yet.


>> -
>>  	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
>>  		return;
>>  
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 1d5007e3795a..6119cb3ad25c 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1472,7 +1472,7 @@ static void cxl_detach_ep(void *data)
>>  	struct cxl_memdev *cxlmd = data;
>>  
>>  	for (int i = cxlmd->depth - 1; i >= 1; i--) {
>> -		struct cxl_port *port, *parent_port;
>> +		struct cxl_port *parent_port;
>>  		struct detach_ctx ctx = {
>>  			.cxlmd = cxlmd,
>>  			.depth = i,
>> @@ -1485,7 +1485,7 @@ static void cxl_detach_ep(void *data)
>>  				      port_has_memdev);
>>  		if (!dev)
>>  			continue;
>> -		port = to_cxl_port(dev);
>> +		struct cxl_port *port __free(put_cxl_port) = to_cxl_port(dev);
> This threw me because 'to_cxl_port' does not take a reference...
>
> Seems ok though.

Yes, to_cxl_port() does not take a reference here, but the 'dev' is provided by a bus_find_device() which will take a reference for the 'dev'.

maybe I should use __free() for the 'dev' like below? I think it is clearer.

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 1d5007e3795a..e4bff611e8fa 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1481,8 +1481,8 @@ static void cxl_detach_ep(void *data)
                struct cxl_ep *ep;
                bool died = false;

-               dev = bus_find_device(&cxl_bus_type, NULL, &ctx,
-                                     port_has_memdev);
+               struct device *dev __free(put_device) = bus_find_device(&cxl_bus_type, NULL,
+                                                                       &ctx, port_has_memdev);
                if (!dev)
                        continue;
                port = to_cxl_port(dev);
@@ -1512,7 +1512,6 @@ static void cxl_detach_ep(void *data)
                                dev_name(&port->dev));
                        delete_switch_port(port);
                }
-               put_device(&port->dev);
                device_unlock(&parent_port->dev);
        }
 }


>
>>  
>>  		parent_port = to_cxl_port(port->dev.parent);
>>  		device_lock(&parent_port->dev);
>> @@ -1512,7 +1512,6 @@ static void cxl_detach_ep(void *data)
>>  				dev_name(&port->dev));
>>  			delete_switch_port(port);
>>  		}
>> -		put_device(&port->dev);
>>  		device_unlock(&parent_port->dev);
>>  	}
>>  }
>> @@ -1539,8 +1538,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>>  			      struct device *uport_dev,
>>  			      struct device *dport_dev)
>>  {
>> +	struct cxl_port *port __free(put_cxl_port) = NULL;
>>  	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;
>> @@ -1556,7 +1555,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>>  		return -ENXIO;
>>  	}
>>  
>> -	parent_port = find_cxl_port(dparent, &parent_dport);
>> +	struct cxl_port *parent_port __free(put_cxl_port) =
>> +		find_cxl_port(dparent, &parent_dport);
>>  	if (!parent_port) {
>>  		/* iterate to create this parent_port */
>>  		return -EAGAIN;
>> @@ -1596,10 +1596,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>>  			 */
>>  			rc = -ENXIO;
>>  		}
>> -		put_device(&port->dev);
>>  	}
>>  
>> -	put_device(&parent_port->dev);
>>  	return rc;
>>  }
>>  
>> @@ -1630,7 +1628,6 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>>  		struct device *dport_dev = grandparent(iter);
>>  		struct device *uport_dev;
>>  		struct cxl_dport *dport;
>> -		struct cxl_port *port;
>>  
>>  		/*
>>  		 * The terminal "grandparent" in PCI is NULL and @platform_bus
>> @@ -1649,7 +1646,8 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>>  		dev_dbg(dev, "scan: iter: %s dport_dev: %s parent: %s\n",
>>  			dev_name(iter), dev_name(dport_dev),
>>  			dev_name(uport_dev));
>> -		port = find_cxl_port(dport_dev, &dport);
>> +		struct cxl_port *port __free(put_cxl_port) =
> Does __free() get called before the next iteration of the loop?  I guess
> it does because it would be out of scope outside the loop?
>
> Ira

Yes, It will get called before the next iteration of the loop. I have validated it.


>
>> +			find_cxl_port(dport_dev, &dport);
>>  		if (port) {
>>  			dev_dbg(&cxlmd->dev,
>>  				"found already registered port %s:%s\n",
>> @@ -1664,18 +1662,13 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>>  			 * the parent_port lock as the current port may be being
>>  			 * reaped.
>>  			 */
>> -			if (rc && rc != -EBUSY) {
>> -				put_device(&port->dev);
>> +			if (rc && rc != -EBUSY)
>>  				return rc;
>> -			}
>>  
>>  			/* Any more ports to add between this one and the root? */
>> -			if (!dev_is_cxl_root_child(&port->dev)) {
>> -				put_device(&port->dev);
>> +			if (!dev_is_cxl_root_child(&port->dev))
>>  				continue;
>> -			}
>>  
>> -			put_device(&port->dev);
>>  			return 0;
>>  		}
>>  
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index 9afb407d438f..cad297fba700 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -744,6 +744,8 @@ struct cxl_root *find_cxl_root(struct cxl_port *port);
>>  void put_cxl_root(struct cxl_root *cxl_root);
>>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_cxl_root(_T))
>>  
>> +DEFINE_FREE(put_cxl_port, struct cxl_port *,
>> +	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
>>  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
>>  void cxl_bus_rescan(void);
>>  void cxl_bus_drain(void);
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 7de232eaeb17..ab9b8ab8df44 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>> @@ -109,7 +109,6 @@ static int cxl_mem_probe(struct device *dev)
>>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
>>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>>  	struct device *endpoint_parent;
>> -	struct cxl_port *parent_port;
>>  	struct cxl_dport *dport;
>>  	struct dentry *dentry;
>>  	int rc;
>> @@ -146,7 +145,8 @@ static int cxl_mem_probe(struct device *dev)
>>  	if (rc)
>>  		return rc;
>>  
>> -	parent_port = cxl_mem_find_port(cxlmd, &dport);
>> +	struct cxl_port *parent_port __free(put_cxl_port) =
>> +		cxl_mem_find_port(cxlmd, &dport);
>>  	if (!parent_port) {
>>  		dev_err(dev, "CXL port topology not found\n");
>>  		return -ENXIO;
>> @@ -179,7 +179,6 @@ static int cxl_mem_probe(struct device *dev)
>>  	rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
>>  unlock:
>>  	device_unlock(endpoint_parent);
>> -	put_device(&parent_port->dev);
>>  	if (rc)
>>  		return rc;
>>  
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index 4be35dc22202..26e75499abdd 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -473,7 +473,6 @@ static bool is_cxl_restricted(struct pci_dev *pdev)
>>  static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
>>  				  struct cxl_register_map *map)
>>  {
>> -	struct cxl_port *port;
>>  	struct cxl_dport *dport;
>>  	resource_size_t component_reg_phys;
>>  
>> @@ -482,14 +481,12 @@ static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
>>  		.resource = CXL_RESOURCE_NONE,
>>  	};
>>  
>> -	port = cxl_pci_find_port(pdev, &dport);
>> +	struct cxl_port *port __free(put_cxl_port) =
>> +		cxl_pci_find_port(pdev, &dport);
>>  	if (!port)
>>  		return -EPROBE_DEFER;
>>  
>>  	component_reg_phys = cxl_rcd_component_reg_phys(&pdev->dev, dport);
>> -
>> -	put_device(&port->dev);
>> -
>>  	if (component_reg_phys == CXL_RESOURCE_NONE)
>>  		return -ENXIO;
>>  
>> -- 
>> 2.40.1
>>
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 51132a575b27..4725e37d90fb 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -915,15 +915,13 @@  static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
 	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
 	struct aer_capability_regs aer_regs;
 	struct cxl_dport *dport;
-	struct cxl_port *port;
 	int severity;
 
-	port = cxl_pci_find_port(pdev, &dport);
+	struct cxl_port *port __free(put_cxl_port) =
+		cxl_pci_find_port(pdev, &dport);
 	if (!port)
 		return;
 
-	put_device(&port->dev);
-
 	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
 		return;
 
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 1d5007e3795a..6119cb3ad25c 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1472,7 +1472,7 @@  static void cxl_detach_ep(void *data)
 	struct cxl_memdev *cxlmd = data;
 
 	for (int i = cxlmd->depth - 1; i >= 1; i--) {
-		struct cxl_port *port, *parent_port;
+		struct cxl_port *parent_port;
 		struct detach_ctx ctx = {
 			.cxlmd = cxlmd,
 			.depth = i,
@@ -1485,7 +1485,7 @@  static void cxl_detach_ep(void *data)
 				      port_has_memdev);
 		if (!dev)
 			continue;
-		port = to_cxl_port(dev);
+		struct cxl_port *port __free(put_cxl_port) = to_cxl_port(dev);
 
 		parent_port = to_cxl_port(port->dev.parent);
 		device_lock(&parent_port->dev);
@@ -1512,7 +1512,6 @@  static void cxl_detach_ep(void *data)
 				dev_name(&port->dev));
 			delete_switch_port(port);
 		}
-		put_device(&port->dev);
 		device_unlock(&parent_port->dev);
 	}
 }
@@ -1539,8 +1538,8 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 			      struct device *uport_dev,
 			      struct device *dport_dev)
 {
+	struct cxl_port *port __free(put_cxl_port) = NULL;
 	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;
@@ -1556,7 +1555,8 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 		return -ENXIO;
 	}
 
-	parent_port = find_cxl_port(dparent, &parent_dport);
+	struct cxl_port *parent_port __free(put_cxl_port) =
+		find_cxl_port(dparent, &parent_dport);
 	if (!parent_port) {
 		/* iterate to create this parent_port */
 		return -EAGAIN;
@@ -1596,10 +1596,8 @@  static int add_port_attach_ep(struct cxl_memdev *cxlmd,
 			 */
 			rc = -ENXIO;
 		}
-		put_device(&port->dev);
 	}
 
-	put_device(&parent_port->dev);
 	return rc;
 }
 
@@ -1630,7 +1628,6 @@  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 		struct device *dport_dev = grandparent(iter);
 		struct device *uport_dev;
 		struct cxl_dport *dport;
-		struct cxl_port *port;
 
 		/*
 		 * The terminal "grandparent" in PCI is NULL and @platform_bus
@@ -1649,7 +1646,8 @@  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 		dev_dbg(dev, "scan: iter: %s dport_dev: %s parent: %s\n",
 			dev_name(iter), dev_name(dport_dev),
 			dev_name(uport_dev));
-		port = find_cxl_port(dport_dev, &dport);
+		struct cxl_port *port __free(put_cxl_port) =
+			find_cxl_port(dport_dev, &dport);
 		if (port) {
 			dev_dbg(&cxlmd->dev,
 				"found already registered port %s:%s\n",
@@ -1664,18 +1662,13 @@  int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 			 * the parent_port lock as the current port may be being
 			 * reaped.
 			 */
-			if (rc && rc != -EBUSY) {
-				put_device(&port->dev);
+			if (rc && rc != -EBUSY)
 				return rc;
-			}
 
 			/* Any more ports to add between this one and the root? */
-			if (!dev_is_cxl_root_child(&port->dev)) {
-				put_device(&port->dev);
+			if (!dev_is_cxl_root_child(&port->dev))
 				continue;
-			}
 
-			put_device(&port->dev);
 			return 0;
 		}
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 9afb407d438f..cad297fba700 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -744,6 +744,8 @@  struct cxl_root *find_cxl_root(struct cxl_port *port);
 void put_cxl_root(struct cxl_root *cxl_root);
 DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_cxl_root(_T))
 
+DEFINE_FREE(put_cxl_port, struct cxl_port *,
+	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev))
 int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
 void cxl_bus_rescan(void);
 void cxl_bus_drain(void);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 7de232eaeb17..ab9b8ab8df44 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -109,7 +109,6 @@  static int cxl_mem_probe(struct device *dev)
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct device *endpoint_parent;
-	struct cxl_port *parent_port;
 	struct cxl_dport *dport;
 	struct dentry *dentry;
 	int rc;
@@ -146,7 +145,8 @@  static int cxl_mem_probe(struct device *dev)
 	if (rc)
 		return rc;
 
-	parent_port = cxl_mem_find_port(cxlmd, &dport);
+	struct cxl_port *parent_port __free(put_cxl_port) =
+		cxl_mem_find_port(cxlmd, &dport);
 	if (!parent_port) {
 		dev_err(dev, "CXL port topology not found\n");
 		return -ENXIO;
@@ -179,7 +179,6 @@  static int cxl_mem_probe(struct device *dev)
 	rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
 unlock:
 	device_unlock(endpoint_parent);
-	put_device(&parent_port->dev);
 	if (rc)
 		return rc;
 
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 4be35dc22202..26e75499abdd 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -473,7 +473,6 @@  static bool is_cxl_restricted(struct pci_dev *pdev)
 static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
 				  struct cxl_register_map *map)
 {
-	struct cxl_port *port;
 	struct cxl_dport *dport;
 	resource_size_t component_reg_phys;
 
@@ -482,14 +481,12 @@  static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
 		.resource = CXL_RESOURCE_NONE,
 	};
 
-	port = cxl_pci_find_port(pdev, &dport);
+	struct cxl_port *port __free(put_cxl_port) =
+		cxl_pci_find_port(pdev, &dport);
 	if (!port)
 		return -EPROBE_DEFER;
 
 	component_reg_phys = cxl_rcd_component_reg_phys(&pdev->dev, dport);
-
-	put_device(&port->dev);
-
 	if (component_reg_phys == CXL_RESOURCE_NONE)
 		return -ENXIO;