Re: [PATCH 1/5] drm/xe/configfs: Fix pci_dev reference leak

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 17.07.2025 23:16, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Michal Wajdeczko
> Sent: Thursday, July 17, 2025 11:48 AM
> To: intel-xe@lists.freedesktop.org
> Cc: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
> Subject: [PATCH 1/5] drm/xe/configfs: Fix pci_dev reference leak
>>
>> We are using pci_get_domain_bus_and_slot() function to verify if
>> the given config directory name matches any existing PCI device,
>> but we missed to call matching pci_dev_put() to release reference.
>>
>> While around, also change error code in case of no device match,
>> to make it more specific than generic formatting error.
>>
>> Fixes: 16280ded45fb ("drm/xe: Add configfs to enable survivability mode")
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Since we're already being fairly granular with the changes in this patch series,
> it probably would've been okay for us to separate the pci_dev_put addition
> from the errno change here.  

while we could split this small patch even further, note that changes
are still around the same logical feature ("check for existing PCI
device") so IMO combining them in single patch is still ok (and since it
has Fixes tag we have a chance to propagate both together)

> But I won't make it a requirement.

hint: usually such comments are then prefixed with "nit:" to make it
clear that comment is just a suggestion or personal preference

> 
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> -Jonathan Cavitt
> 
>> ---
>>  drivers/gpu/drm/xe/xe_configfs.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
>> index 8ec1ff1e4e80..e9b46a2d0019 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.c
>> +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> @@ -267,7 +267,8 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
>>  
>>  	pdev = pci_get_domain_bus_and_slot(domain, bus, PCI_DEVFN(slot, function));
>>  	if (!pdev)
>> -		return ERR_PTR(-EINVAL);
>> +		return ERR_PTR(-ENODEV);
>> +	pci_dev_put(pdev);
>>  
>>  	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>  	if (!dev)
>> -- 
>> 2.47.1
>>
>>