| Message ID | 20230922092823.478042-1-wyes.karny@amd.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Len Brown |
| Headers | show |
| Series | tools/power turbostat: Increase the limit for fd opened | expand |
On 2023.09.22 02:28 Wyes Karny wrote: > When running turbostat, a system with 512 cpus reaches the limit for Suggest" ... reaches the default limit for..." > maximum number of file descriptors that can be opened. To solve this > problem, the limit is raised to 2^15, which is a large enough number. > > Below data is collected from AMD server systems while running turbostat: > > |-----------+-------------------------------| > | # of cpus | # of opened fds for turbostat | > |-----------+-------------------------------| > | 128 | 260 | > |-----------+-------------------------------| > | 192 | 388 | > |-----------+-------------------------------| > | 512 | 1028 | > |-----------+-------------------------------| The number of open files is a function of what is being "show"ed or "hide"en. They can also increase beyond the above 2 X (# of CPUs) + 4 number via the --add directive. > > So, the new max limit would be sufficient up to 2^14 cpus. Well, not quiet, but the point is valid. Normally, I would assume that a server with a large number of CPUs would have set a much higher limit of the number of open files than the default. I use 131,072 and so this patch reduces the maximum. Unpatched: root@s19:~# cat /proc/47043/limits | grep "Max open files" Max open files 131072 131072 files Patched: root@s19:~# cat /proc/47032/limits | grep "Max open files" Max open files 32768 32768 files Anyway: Reviewed-by: Doug Smythies <dsmythies@telus.net> Tested-by: Doug Smythies <dsmythies@telus.net> > > Signed-off-by: Wyes Karny <wyes.karny@amd.com> > --- > tools/power/x86/turbostat/turbostat.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > index 9a10512e3407..23f1fe58289a 100644 > --- a/tools/power/x86/turbostat/turbostat.c > +++ b/tools/power/x86/turbostat/turbostat.c > @@ -6717,6 +6717,18 @@ void cmdline(int argc, char **argv) > } > } > > +void set_rlimit(void) > +{ > + struct rlimit limit; > + > + limit.rlim_cur = 0x8000; > + limit.rlim_max = 0x8000; > + > + if (setrlimit(RLIMIT_NOFILE, &limit) < 0) { > + err(1, "Failed to set rlimit"); > + } > +} > + > int main(int argc, char **argv) > { > outf = stderr; > @@ -6729,6 +6741,9 @@ int main(int argc, char **argv) > > probe_sysfs(); > > + if (!getuid()) > + set_rlimit(); > + > turbostat_init(); > > msr_sum_record(); > -- > 2.34.1 >
Hi Doug, Thanks for taking a look at the patch. On 22 Sep 13:48, Doug Smythies wrote: > On 2023.09.22 02:28 Wyes Karny wrote: > > > When running turbostat, a system with 512 cpus reaches the limit for > > Suggest" ... reaches the default limit for..." > > > maximum number of file descriptors that can be opened. To solve this > > problem, the limit is raised to 2^15, which is a large enough number. > > > > Below data is collected from AMD server systems while running turbostat: > > > > |-----------+-------------------------------| > > | # of cpus | # of opened fds for turbostat | > > |-----------+-------------------------------| > > | 128 | 260 | > > |-----------+-------------------------------| > > | 192 | 388 | > > |-----------+-------------------------------| > > | 512 | 1028 | > > |-----------+-------------------------------| > > The number of open files is a function of what is being "show"ed or "hide"en. > They can also increase beyond the above 2 X (# of CPUs) + 4 number > via the --add directive. > > > > So, the new max limit would be sufficient up to 2^14 cpus. > > Well, not quiet, but the point is valid. > > Normally, I would assume that a server with a large number of > CPUs would have set a much higher limit of the number of open > files than the default. I use 131,072 and so this patch reduces the > maximum. I think below will fix the problem. +#define MAX_NOFILE 0x8000 + +void set_rlimit(void) +{ + struct rlimit limit; + + if(getrlimit(RLIMIT_NOFILE, &limit) < 0) { + err(1, "Failed to get rlimit"); + } + + if (limit.rlim_max < MAX_NOFILE) + limit.rlim_max = MAX_NOFILE; + if (limit.rlim_cur < MAX_NOFILE) + limit.rlim_cur = MAX_NOFILE; + + if (setrlimit(RLIMIT_NOFILE, &limit) < 0) { + err(1, "Failed to set rlimit"); + } + return; +} Is this looks okay to you? Thanks, Wyes > > Unpatched: > root@s19:~# cat /proc/47043/limits | grep "Max open files" > Max open files 131072 131072 files > > Patched: > root@s19:~# cat /proc/47032/limits | grep "Max open files" > Max open files 32768 32768 files > > Anyway: > > Reviewed-by: Doug Smythies <dsmythies@telus.net> > Tested-by: Doug Smythies <dsmythies@telus.net> > > > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com> > > --- > > tools/power/x86/turbostat/turbostat.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > > index 9a10512e3407..23f1fe58289a 100644 > > --- a/tools/power/x86/turbostat/turbostat.c > > +++ b/tools/power/x86/turbostat/turbostat.c > > @@ -6717,6 +6717,18 @@ void cmdline(int argc, char **argv) > > } > > } > > > > +void set_rlimit(void) > > +{ > > + struct rlimit limit; > > + > > + limit.rlim_cur = 0x8000; > > + limit.rlim_max = 0x8000; > > + > > + if (setrlimit(RLIMIT_NOFILE, &limit) < 0) { > > + err(1, "Failed to set rlimit"); > > + } > > +} > > + > > int main(int argc, char **argv) > > { > > outf = stderr; > > @@ -6729,6 +6741,9 @@ int main(int argc, char **argv) > > > > probe_sysfs(); > > > > + if (!getuid()) > > + set_rlimit(); > > + > > turbostat_init(); > > > > msr_sum_record(); > > -- > > 2.34.1 > > >
Hi Wyes, On 2023.09.26 10:16 Wyes Karny wrote: > On 22 Sep 13:48, Doug Smythies wrote: > > On 2023.09.22 02:28 Wyes Karny wrote: > > > > > When running turbostat, a system with 512 cpus reaches the limit for > > > > Suggest" ... reaches the default limit for..." > > > > > maximum number of file descriptors that can be opened. To solve this > > > problem, the limit is raised to 2^15, which is a large enough number. > > > > > > Below data is collected from AMD server systems while running turbostat: > > > > > > |-----------+-------------------------------| > > > | # of cpus | # of opened fds for turbostat | > > > |-----------+-------------------------------| > > > | 128 | 260 | > > > |-----------+-------------------------------| > > > | 192 | 388 | > > > |-----------+-------------------------------| > > > | 512 | 1028 | > > > |-----------+-------------------------------| > > > > The number of open files is a function of what is being "show"ed or "hide"en. > > They can also increase beyond the above 2 X (# of CPUs) + 4 number > > via the --add directive. > > > > > > So, the new max limit would be sufficient up to 2^14 cpus. > > > > Well, not quiet, but the point is valid. > > > > Normally, I would assume that a server with a large number of > > CPUs would have set a much higher limit of the number of open > > files than the default. I use 131,072 and so this patch reduces the > > maximum. > > I think below will fix the problem. > > +#define MAX_NOFILE 0x8000 > + > +void set_rlimit(void) > +{ > + struct rlimit limit; > + > + if(getrlimit(RLIMIT_NOFILE, &limit) < 0) { > + err(1, "Failed to get rlimit"); > + } > + > + if (limit.rlim_max < MAX_NOFILE) > + limit.rlim_max = MAX_NOFILE; > + if (limit.rlim_cur < MAX_NOFILE) > + limit.rlim_cur = MAX_NOFILE; > + > + if (setrlimit(RLIMIT_NOFILE, &limit) < 0) { > + err(1, "Failed to set rlimit"); > + } > + return; > +} > > Is this looks okay to you? Either the original or the above is fine. I was just being nit-picky and annoying is all. I still added a "Reviewed-by" below. ... Doug > Thanks, > Wyes > > > > Unpatched: > > root@s19:~# cat /proc/47043/limits | grep "Max open files" > > Max open files 131072 131072 files > > > > Patched: > > root@s19:~# cat /proc/47032/limits | grep "Max open files" > > Max open files 32768 32768 files > > > > Anyway: > > > > Reviewed-by: Doug Smythies <dsmythies@telus.net> > > Tested-by: Doug Smythies <dsmythies@telus.net> > > > > > > > > Signed-off-by: Wyes Karny <wyes.karny@amd.com> > > > --- > > > tools/power/x86/turbostat/turbostat.c | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > > > index 9a10512e3407..23f1fe58289a 100644 > > > --- a/tools/power/x86/turbostat/turbostat.c > > > +++ b/tools/power/x86/turbostat/turbostat.c > > > @@ -6717,6 +6717,18 @@ void cmdline(int argc, char **argv) > > > } > > > } > > > > > > +void set_rlimit(void) > > > +{ > > > + struct rlimit limit; > > > + > > > + limit.rlim_cur = 0x8000; > > > + limit.rlim_max = 0x8000; > > > + > > > + if (setrlimit(RLIMIT_NOFILE, &limit) < 0) { > > > + err(1, "Failed to set rlimit"); > > > + } > > > +} > > > + > > > int main(int argc, char **argv) > > > { > > > outf = stderr; > > > @@ -6729,6 +6741,9 @@ int main(int argc, char **argv) > > > > > > probe_sysfs(); > > > > > > + if (!getuid()) > > > + set_rlimit(); > > > + > > > turbostat_init(); > > > > > > msr_sum_record(); > > > -- > > > 2.34.1
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 9a10512e3407..23f1fe58289a 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -6717,6 +6717,18 @@ void cmdline(int argc, char **argv) } } +void set_rlimit(void) +{ + struct rlimit limit; + + limit.rlim_cur = 0x8000; + limit.rlim_max = 0x8000; + + if (setrlimit(RLIMIT_NOFILE, &limit) < 0) { + err(1, "Failed to set rlimit"); + } +} + int main(int argc, char **argv) { outf = stderr; @@ -6729,6 +6741,9 @@ int main(int argc, char **argv) probe_sysfs(); + if (!getuid()) + set_rlimit(); + turbostat_init(); msr_sum_record();
When running turbostat, a system with 512 cpus reaches the limit for maximum number of file descriptors that can be opened. To solve this problem, the limit is raised to 2^15, which is a large enough number. Below data is collected from AMD server systems while running turbostat: |-----------+-------------------------------| | # of cpus | # of opened fds for turbostat | |-----------+-------------------------------| | 128 | 260 | |-----------+-------------------------------| | 192 | 388 | |-----------+-------------------------------| | 512 | 1028 | |-----------+-------------------------------| So, the new max limit would be sufficient up to 2^14 cpus. Signed-off-by: Wyes Karny <wyes.karny@amd.com> --- tools/power/x86/turbostat/turbostat.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)