Re: [Powertop] patch: tell user if the system is running out of FDs

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

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

On (09/19/13 11:23), Jaroslav Skarvada wrote:
> Hi,
> 
> complex servers can run out of FDs easily (due to perf).
> In such case tell user that more FDs are needed instead
> of the generic message (that kernel doesn't support
> the perf)

Hello Jaroslav,
Thank you for your patch.

Youquan Song proposed to change RLIMIT_NOFILE in "[PATCH] Fix running
failure when > 69 CPUs for open file limitation" some time ago
(v1 of the patch https://lists.01.org/pipermail/powertop/2013-May/000850.html)

after some thinking I've merged Youquan's patch to powertop2-next tree
https://github.com/sergey-senozhatsky/powertop2-next/commit/4cbb957605818dc7e4b7932dafac0aad5ed0b87a

(not upstreamed yet).

while I don't really want to unlimit fds, at the same time from the user's
point of view it'd better to handle it `transparently'. I think most likely user
will react with `ulimit -n unlimited' to any powertop rlimited fds message, since
he/she really does not control (+does not care about) the number of opened fds.

thanks,
	-ss

> regards
> 
> Jaroslav

> diff -up powertop-2.4/src/perf/perf.cpp.orig powertop-2.4/src/perf/perf.cpp
> --- powertop-2.4/src/perf/perf.cpp.orig	2013-01-31 16:39:27.000000000 -0500
> +++ powertop-2.4/src/perf/perf.cpp	2013-09-19 10:36:02.298839248 -0400
> @@ -26,6 +26,7 @@
>  #include <iostream>
>  #include <fstream>
>  
> +#include <errno.h>
>  #include <unistd.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -72,6 +73,7 @@ void perf_event::create_perf_event(char 
>  {
>  	struct perf_event_attr attr;
>  	int ret;
> +	int err;
>  
>  	struct {
>  		__u64 count;
> @@ -107,10 +109,15 @@ void perf_event::create_perf_event(char 
>  	perf_fd = sys_perf_event_open(&attr, -1, _cpu, -1, 0);
>  
>  	if (perf_fd < 0) {
> +		err = errno;
>  		reset_display();
> -		fprintf(stderr, _("PowerTOP %s needs the kernel to support the 'perf' subsystem\n"), POWERTOP_VERSION);
> -		fprintf(stderr, _("as well as support for trace points in the kernel:\n"));
> -		fprintf(stderr, "CONFIG_PERF_EVENTS=y\nCONFIG_PERF_COUNTERS=y\nCONFIG_TRACEPOINTS=y\nCONFIG_TRACING=y\n");
> +		if (err == EMFILE)
> +			fprintf(stderr, _("Too many open files, please increase the limit of open file descriptors.\n"));
> +		else {
> +			fprintf(stderr, _("PowerTOP %s needs the kernel to support the 'perf' subsystem\n"), POWERTOP_VERSION);
> +			fprintf(stderr, _("as well as support for trace points in the kernel:\n"));
> +			fprintf(stderr, "CONFIG_PERF_EVENTS=y\nCONFIG_PERF_COUNTERS=y\nCONFIG_TRACEPOINTS=y\nCONFIG_TRACING=y\n");
> +		}
>  		exit(EXIT_FAILURE);
>  	}
>  	if (read(perf_fd, &read_data, sizeof(read_data)) == -1) {

> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop