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

[Dave Jiang <dave.jiang@intel.com>]

On 1/4/24 11:22, Robert Richter wrote:
> On 04.01.24 09:08:05, 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))
> 
> Let's put this in parallel with find_cxl_root() to make it part of the
> api for users of find_cxl_root() to cleanup.

Sure that makes sense. Will do that.

> 
> Nit: remove the additional newline to have the same style for all uses
> of DEFINE_FREE() in this file.

I was contemplating that because checkpatch complained. But I guess if we are moving the function to where find_cxl_root() is then it won't matter. 

> 
> -Robert
> 
>> +
>>  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;
>>
>>