linux-kernel - Re: [PATCH] pds_fwctl: Remove the use of dev_err

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3c22983c-6aa7-44ef-a368-17ff9d96fb9a@amd.com>
Date: Wed, 20 Aug 2025 09:27:37 -0700
From: Brett Creeley <bcreeley@....com>
To: Dave Jiang <dave.jiang@...el.com>, Liao Yuanhong <liaoyuanhong@...o.com>,
 Brett Creeley <brett.creeley@....com>, Jason Gunthorpe <jgg@...pe.ca>,
 Saeed Mahameed <saeedm@...dia.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 "open list:FWCTL PDS DRIVER" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pds_fwctl: Remove the use of dev_err_probe()



On 8/20/2025 8:03 AM, Dave Jiang wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On 8/20/25 5:40 AM, Liao Yuanhong wrote:
>> Logging messages that show some type of "out of memory" error are generally
>> unnecessary as there is a generic message and a stack dump done by the
>> memory subsystem. These messages generally increase kernel size without
>> much added value[1].
>>
>> The dev_err_probe() doesn't do anything when error is '-ENOMEM'. Therefore,
>> remove the useless call to dev_err_probe(), and just return the value
>> instead.
>>
>> [1]: https://lore.kernel.org/lkml/1402419340.30479.18.camel@joe-AO725/
>>
>> Signed-off-by: Liao Yuanhong <liaoyuanhong@...o.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@...el.com>
>> ---
>>   drivers/fwctl/pds/main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/fwctl/pds/main.c b/drivers/fwctl/pds/main.c
>> index 9b9d1f6b5556..8dd659aee256 100644
>> --- a/drivers/fwctl/pds/main.c
>> +++ b/drivers/fwctl/pds/main.c
>> @@ -481,7 +481,7 @@ static int pdsfc_probe(struct auxiliary_device *adev,
>>        pdsfc = fwctl_alloc_device(&padev->vf_pdev->dev, &pdsfc_ops,
>>                                   struct pdsfc_dev, fwctl);
>>        if (!pdsfc)
>> -             return dev_err_probe(dev, -ENOMEM, "Failed to allocate fwctl device struct\n");
>> +             return -ENOMEM;

With dev_err_probe() ignoring -ENOMEM, this is a no-op, so this change 
makes sense.

In general we like to have messages like this so the failure path is 
immediately obvious.

However, in this case it is unnecessary because there are specific 
errors in all of the other possible probe failures.

Thanks for the patch.

Reviewed-by: Brett Creeley <brett.creeley@....com>

>>        pdsfc->padev = padev;
>>
>>        err = pdsfc_identify(pdsfc);
>