[PATCH] driver core: Define dev_err_probe() as __cold

[PATCH] driver core: Define dev_err_probe() as __cold

[Christophe JAILLET]

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@wanadoo.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, ...);
 
 /* Create alias, so I can be autoloaded. */
 #define MODULE_ALIAS_CHARDEV(major,minor) \
-- 
2.34.1

Re: [PATCH] driver core: Define dev_err_probe() as __cold

[Greg KH]

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@wanadoo.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

Re: [PATCH] driver core: Define dev_err_probe() as __cold

[Christophe JAILLET]

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@wanadoo.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