Re: [PATCH v3] perf test record.sh: Raise limit of open file descriptors

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Wed, Aug 14, 2024 at 12:35:55PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Aug 14, 2024 at 12:32:39PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Wed, Aug 14, 2024 at 05:17:34PM +0200, vmolnaro@redhat.com wrote:
> > > From: Veronika Molnarova <vmolnaro@redhat.com>
> > 
> > Ok? I'm applying it with this change. Thanks,
> 
> So I added this to the log message:
> 
> Committer notes:
> 
> Instead of disabling ShellCheck warnings all the uses of 'uname -n',
> i.e. those:
> 
>   In tests/shell/record.sh line 35:
>   default_fd_limit=$(ulimit -Sn)
>                             ^-^ SC3045 (warning): In POSIX sh, ulimit -S is undefined.
> 
> We can just switch from using '/bin/sh' to '/bin/bash' for this test, as
> bash _has_ 'ulimit -n', so ShellCheck will not emit that warning.
> 
> There are dozens of 'perf test' shell tests that do just that,
> '/bin/bash' is a reasonable expectation for those tests.
> 
> ----------------------------------------------------------
> 
> Please let me know if you find any issues with this course of action,

Well, I looked again and since we're switching for /bin/bash as a
requisite for this test, then we better use your v2 patch that doesn't
have the logic for checking if 'ulimit -n' is available, i.e. I'm
picking this instead:

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 7964ebd9007d8eeb..29ab077b3811d494 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 # perf record tests
 # SPDX-License-Identifier: GPL-2.0
 
@@ -23,6 +23,15 @@ br_cntr_file="/caps/branch_counter_nr"
 br_cntr_output="branch stack counters"
 br_cntr_script_output="br_cntr: A"
 
+default_fd_limit=$(ulimit -n)
+# With option --threads=cpu the number of open file descriptors should be
+# equal to sum of:    nmb_cpus * nmb_events (2+dummy),
+#                     nmb_threads for perf.data.n (equal to nmb_cpus) and
+#                     2*nmb_cpus of pipes = 4*nmb_cpus (each pipe has 2 ends)
+# All together it needs 8*nmb_cpus file descriptors plus some are also used
+# outside of testing, thus raising the limit to 16*nmb_cpus
+min_fd_limit=$(($(getconf _NPROCESSORS_ONLN) * 16))
+
 cleanup() {
   rm -rf "${perfdata}"
   rm -rf "${perfdata}".old
@@ -197,11 +206,19 @@ test_branch_counter() {
   echo "Branch counter test [Success]"
 }
 
+# raise the limit of file descriptors to minimum
+if [[ $default_fd_limit -lt $min_fd_limit ]]; then
+       ulimit -n $min_fd_limit
+fi
+
 test_per_thread
 test_register_capture
 test_system_wide
 test_workload
 test_branch_counter
 
+# restore the default value
+ulimit -n $default_fd_limit
+
 cleanup
 exit $err