linux-kernel - Re: [PATCH] driver core: Define dev_err_probe() as _

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]

Date:   Sat, 6 Aug 2022 09:53:49 +0200
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     tglx@...utronix.de, jgg@...pe.ca, ira.weiny@...el.com,
        dan.j.williams@...el.com, andriy.shevchenko@...ux.intel.com,
        wonchung@...gle.com, list@...l.com, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] driver core: Define dev_err_probe() as __cold

Le 06/08/2022 à 09:12, Greg KH a écrit :
> On Sat, Aug 06, 2022 at 08:49:23AM +0200, Christophe JAILLET wrote:
>> Give a hint to the compiler that dev_err_probe() is used for error
>> handling. So calling paths are unlikely.
>>
>> >From gcc documentation:
>> 	The paths leading to calls of cold functions within code are marked
>> 	as unlikely by the branch prediction mechanism.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
>> ---
>>   include/linux/device.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 424b55df0272..4ac16bde9bf7 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -1093,7 +1093,7 @@ void device_links_supplier_sync_state_pause(void);
>>   void device_links_supplier_sync_state_resume(void);
>>   
>>   extern __printf(3, 4)
>> -int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
>> +int __cold dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
> 
> As the probe() path is by default "slow", does this actually help
> anything?  I never recommend using any sort of manual likely/unlikely
> hints unless the results can be seen, otherwise the compiler and CPU
> almost always do a better job over time.
> 
> thanks,
> 
> greg k-h
> 

Based on a few tests, the generated code is different.
But it is hard to compare if it looks better or not because many things 
are shuffled.

My point is that the proposed change is easy and that the hint "should 
always be correct in this particular case".
Also _dev_err() and co. functions are already annotated with __cold.

But honestly, I agree with your POV.

Sometimes the resulting .o is slightly smaller, sometimes slightly bigger.

So, unless s.o. else cares, let leave it as is, timing of probe does not 
really matter anyway.


CJ