Re: [Powertop] [PATCH] Fix running failure when > 69 CPUs for open file limitation

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

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

On (06/04/13 17:31), Youquan Song wrote:
> > 
> > how about, say, 2048? unlimited resource steals possibility to detect 
> > error/bug/leak/etc.
> 
> I do not like it. If there are more CPUs on larger machine and if we
> want to monitor more performance event for each CPU, the 2048 settting will
>  be another issue.
> 

we may get the number of CPUs, multiply it by events number + probably add some 
extra fds, and set it as a limit. this is not a really big issue, though. my point 
is that limited resource gives us better chances to detect resource leak in 
powertop, however I agree that we just may fireup valgrind for that purpose.


	-ss

> We just set it to the rlim_cur to rlim_max for the powertop application
> and to cater to powertop requirement as large as possible.
> 
> > and let's make rlimit change after checkroot() call.
> > 
> 
> Good point. Agree. Here is the modified patch.
> 
> 
> diff --git a/src/main.cpp b/src/main.cpp
> index 0883424..d24a3db 100644
> --- a/src/main.cpp
> +++ b/src/main.cpp
> @@ -36,6 +36,7 @@
>  #include <getopt.h>
>  #include <unistd.h>
>  #include <locale.h>
> +#include <sys/resource.h>
>  
>  #include "cpu/cpu.h"
>  #include "process/process.h"
> @@ -283,11 +284,17 @@ static void powertop_init(void)
>  	static char initialized = 0;
>  	int ret;
>  	struct statfs st_fs;
> +	struct rlimit rlmt;
>  
>  	if (initialized)
>  		return;
>  
>  	checkroot();
> +
> +	getrlimit (RLIMIT_NOFILE, &rlmt);
> +	rlmt.rlim_cur = rlmt.rlim_max;
> +	setrlimit (RLIMIT_NOFILE, &rlmt);
> +
>  	ret = system("/sbin/modprobe cpufreq_stats > /dev/null 2>&1");
>  	ret = system("/sbin/modprobe msr > /dev/null 2>&1");
>  	statfs("/sys/kernel/debug", &st_fs);
> 
>  
>