Re: [Powertop] [PATCH] cpufreq: only try to open files under cpuX

[Sergey Senozhatsky <sergey.senozhatsky at gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]

On (12/12/12 17:16), Jaroslav Škarvada wrote:
> There are called many useless 'open' syscalls that always fail, like:
>   open("/sys/devices/system/cpu/uevent/cpufreq/scaling_governor", O_RDONLY) = -1 ENOTDIR (Not a directory)
> This patch filters this to /sys/devices/system/cpu/cpuX, where X is string
> starting with digit (from kernel sources it seems that X can only be integer).
> For match cases it will add only small overhead.
> 
> Originally reported as:
> https://bugzilla.redhat.com/show_bug.cgi?id=886185
> 
> Signed-off-by: Jaroslav Škarvada <jskarvad(a)redhat.com>
> ---
>  src/tuning/cpufreq.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/tuning/cpufreq.cpp b/src/tuning/cpufreq.cpp
> index df245ad..c448f4c 100644
> --- a/src/tuning/cpufreq.cpp
> +++ b/src/tuning/cpufreq.cpp
> @@ -36,6 +36,7 @@
>  #include <dirent.h>
>  #include <errno.h>
>  #include <sys/stat.h>
> +#include <ctype.h>
>  
>  #include "../lib.h"
>  #include "cpufreq.h"
> @@ -72,7 +73,7 @@ int cpufreq_tunable::good_bad(void)
>  		return ret;
>  
>  	while ((dirent = readdir(dir))) {
> -		if (dirent->d_name[0]=='.')
> +		if (strlen(dirent->d_name) < 4 || strncmp(dirent->d_name, "cpu", 3) != 0 || !isdigit(dirent->d_name[3]))
>  			continue;
>  		sprintf(filename, "/sys/devices/system/cpu/%s/cpufreq/scaling_governor", dirent->d_name);
>  		file = fopen(filename, "r");
> -- 
> 1.7.11.7
> 

Hello,
thanks for your patch. however, I'm afraid it does not fix all the cases. looking at src/tuning/cpufreq.cpp we
have several more places with exactly the same cpuX detection -- ::toggle*().

could you also please remove 'strlen(dirent->d_name) < 4', looks loke a small overhead there.

	-ss