RE: [PATCH] cxl: Clarify root_port cleanup routine for cxl_qos_class_verify()

[Dan Williams <dan.j.williams@intel.com>]

Dave Jiang wrote:
> The current __free() call for root_port in cxl_qos_class_verify() depends
> on 'struct device' to be the first member of 'struct cxl_port' and calls
> put_device() directly with the root_port pointer passed in. Add a helper
> function put_cxl_port() to handle the put_device() properly and avoid
> future issues if the 'struct device' member moves.
> 
> Suggested-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/cdat.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index cd84d87f597a..d6e64570032f 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -345,11 +345,21 @@ static void discard_dpa_perf(struct list_head *list)
>  }
>  DEFINE_FREE(dpa_perf, struct list_head *, if (!list_empty(_T)) discard_dpa_perf(_T))
>  
> +static void put_cxl_port(struct cxl_port *port)
> +{
> +	if (!port)
> +		return;
> +
> +	put_device(&port->dev);
> +}
> +
> +DEFINE_FREE(cxl_port, struct cxl_port *, put_cxl_port(_T))

I don't think there are other cases where a port reference is acquired
at runtime, so this should be root specific, i.e. put_cxl_root().

This also merits a NULL check to skip calling put_cxl_root() if the
pointer is already NULL:

	DEFINE_FREE(put_cxl_root, struct cxl_port *, if (_T) put_cxl_root(_T))

...for example if someone used no_free_ptr() to return the found root
port.

> +
>  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;
> +	struct cxl_port *root_port __free(cxl_port) = NULL;
>  	LIST_HEAD(__discard);
>  	struct list_head *discard __free(dpa_perf) = &__discard;
>  	int rc;

So in the discussion here:

http://lore.kernel.org/r/65970003ee9ef_8dc6829412@dwillia2-xfh.jf.intel.com.notmuch

...I made the assertion that if a function has multiple __free()
invocations it should make sure to arrange for it to be impossible for
declaration-order to be different from initialization order, and that
means getting rid of the "__free(x) = NULL" pattern and keep
declare/init aligned:

	struct cxl_port *root __free(put_cxl_root) = find_cxl_root(port);