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

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

[vmolnaro]

From: Veronika Molnarova <vmolnaro@redhat.com>

Subtest for system-wide record with '--threads=cpu' option fails due
to a limit of open file descriptors on systems with 128 or more CPUs
as the default limit is set to 1024.

The number of open file descriptors should be slightly above
nmb_events*nmb_cpus + nmb_cpus(for perf.data.n) + 4*nmb_cpus(for pipes),
which equals 8*nmb_cpus. Therefore, temporarily raise the limit to
16*nmb_cpus for the test.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Radostin Stoyanov <rstoyano@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
---
v2: Rebase the patch to apply upstream
v3: Fix shellcheck warnings
---
 tools/perf/tests/shell/record.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 3d1a7759a7b2..c1160d36311a 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -22,6 +22,21 @@ cpu_pmu_dir="/sys/bus/event_source/devices/cpu*"
 br_cntr_file="/caps/branch_counter_nr"
 br_cntr_output="branch stack counters"
 
+ulimit_supported=false
+# 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))
+
+# shellcheck disable=SC3045 # ulimit is required for testing --threads option
+default_fd_limit=$(ulimit -Sn)
+if [ $? -eq 0 ]; then
+  ulimit_supported=true
+fi
+
 cleanup() {
   rm -rf "${perfdata}"
   rm -rf "${perfdata}".old
@@ -107,6 +122,11 @@ test_register_capture() {
 
 test_system_wide() {
   echo "Basic --system-wide mode test"
+  if [ ! $ulimit_supported ] && [ $min_fd_limit -gt 1024 ]
+  then
+    echo "Limited file descriptors for --threads option [Skipped not available]"
+    return
+  fi
   if ! perf record -aB --synth=no -o "${perfdata}" ${testprog} 2> /dev/null
   then
     echo "System-wide record [Skipped not supported]"
@@ -190,11 +210,23 @@ test_branch_counter() {
   echo "Basic branch counter test [Success]"
 }
 
+# raise the limit of file descriptors to required minimum
+if [ $ulimit_supported ] && [ $default_fd_limit -lt $min_fd_limit ]; then
+  # shellcheck disable=SC3045 # ulimit is required for testing --threads option
+  ulimit -Sn $min_fd_limit
+fi
+
 test_per_thread
 test_register_capture
 test_system_wide
 test_workload
 test_branch_counter
 
+# restore the default value
+if [ $ulimit_supported ]; then
+  # shellcheck disable=SC3045 # ulimit is required for testing --threads option
+  ulimit -Sn $default_fd_limit
+fi
+
 cleanup
 exit $err
-- 
2.43.0

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

[Arnaldo Carvalho de Melo]

On Wed, Aug 14, 2024 at 05:17:34PM +0200, vmolnaro@redhat.com wrote:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> Subtest for system-wide record with '--threads=cpu' option fails due
> to a limit of open file descriptors on systems with 128 or more CPUs
> as the default limit is set to 1024.
> 
> The number of open file descriptors should be slightly above
> nmb_events*nmb_cpus + nmb_cpus(for perf.data.n) + 4*nmb_cpus(for pipes),
> which equals 8*nmb_cpus. Therefore, temporarily raise the limit to
> 16*nmb_cpus for the test.

So it is not applying anymore, Kan Liang changed that file, I'm fixing
it up:

⬢[acme@toolbox perf-tools-next]$        git am ./v3_20240814_vmolnaro_perf_test_record_sh_raise_limit_of_open_file_descriptors.mbx
Applying: perf test record.sh: Raise limit of open file descriptors
error: patch failed: tools/perf/tests/shell/record.sh:22
error: tools/perf/tests/shell/record.sh: patch does not apply
Patch failed at 0001 perf test record.sh: Raise limit of open file descriptors
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
⬢[acme@toolbox perf-tools-next]$

Doing it manually makes us go farther:

⬢[acme@toolbox perf-tools-next]$ patch -p1 < ./v3_20240814_vmolnaro_perf_test_record_sh_raise_limit_of_open_file_descriptors.mbx
patching file tools/perf/tests/shell/record.sh
Hunk #1 succeeded at 23 with fuzz 2 (offset 1 line).
Hunk #2 succeeded at 123 (offset 1 line).
Hunk #3 succeeded at 217 with fuzz 1 (offset 7 lines).
⬢[acme@toolbox perf-tools-next]$

After applying it with the resulting diff, I think we should not disable
those changes, just state the precise shell we think is needed for this
particular test:

⬢[acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 28f5fe3b380e0b06..de913ebeb54ba220 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
 
@@ -32,7 +32,6 @@ ulimit_supported=false
 # outside of testing, thus raising the limit to 16*nmb_cpus
 min_fd_limit=$(($(getconf _NPROCESSORS_ONLN) * 16))
 
-# shellcheck disable=SC3045 # ulimit is required for testing --threads option
 default_fd_limit=$(ulimit -Sn)
 if [ $? -eq 0 ]; then
   ulimit_supported=true
@@ -219,7 +218,6 @@ test_branch_counter() {
 
 # raise the limit of file descriptors to required minimum
 if [ $ulimit_supported ] && [ $default_fd_limit -lt $min_fd_limit ]; then
-  # shellcheck disable=SC3045 # ulimit is required for testing --threads option
   ulimit -Sn $min_fd_limit
 fi
 
@@ -231,7 +229,6 @@ test_branch_counter
 
 # restore the default value
 if [ $ulimit_supported ]; then
-  # shellcheck disable=SC3045 # ulimit is required for testing --threads option
   ulimit -Sn $default_fd_limit
 fi
 
⬢[acme@toolbox perf-tools-next]$

I.e. just state that /bin/bash is needed, like is done in multiple other
shell tests:

⬢[acme@toolbox perf-tools-next]$ head -1 tools/perf/tests/shell/*.sh | grep bash | wc -l
16
⬢[acme@toolbox perf-tools-next]$

Ok? I'm applying it with this change. Thanks,

- Arnaldo

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

[Arnaldo Carvalho de Melo]

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,

Thanks,

- Arnaldo

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

[Veronika Molnarova]

On 8/14/24 17:35, 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,
> 
> Thanks,
> 
> - Arnaldo
> 
If so then the check whether the ulimit is supported doesn't need to be done
as bash is given as a requirement. Thought that it should be supporting all
possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.

Can I send a quick fix that just changes to the '/bin/bash' so that the code
won't have unnecessary code?

Thanks,
Veronika

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

[Arnaldo Carvalho de Melo]

On Wed, Aug 14, 2024 at 05:42:01PM +0200, Veronika Molnarova wrote:
> 
> 
> On 8/14/24 17:35, 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,
> > 
> > Thanks,
> > 
> > - Arnaldo
> > 
> If so then the check whether the ulimit is supported doesn't need to be done
> as bash is given as a requirement. Thought that it should be supporting all
> possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.
> 
> Can I send a quick fix that just changes to the '/bin/bash' so that the code
> won't have unnecessary code?

See my last message, I went with your v2 + switch from /bin/sh to
/bin/bash, as you suggest above.

- Arnaldo

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

[Arnaldo Carvalho de Melo]

On Wed, Aug 14, 2024 at 12:54:51PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Aug 14, 2024 at 05:42:01PM +0200, Veronika Molnarova wrote:
> > 
> > 
> > On 8/14/24 17:35, 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,
> > > 
> > > Thanks,
> > > 
> > > - Arnaldo
> > > 
> > If so then the check whether the ulimit is supported doesn't need to be done
> > as bash is given as a requirement. Thought that it should be supporting all
> > possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.
> > 
> > Can I send a quick fix that just changes to the '/bin/bash' so that the code
> > won't have unnecessary code?
> 
> See my last message, I went with your v2 + switch from /bin/sh to
> /bin/bash, as you suggest above.

And added the '-S' to ulimit, since you changed that from v2, are you
sure that is better than using just -n? Why?

- Arnaldo

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

[Veronika Molnarova]

On 8/14/24 17:56, Arnaldo Carvalho de Melo wrote:
> On Wed, Aug 14, 2024 at 12:54:51PM -0300, Arnaldo Carvalho de Melo wrote:
>> On Wed, Aug 14, 2024 at 05:42:01PM +0200, Veronika Molnarova wrote:
>>>
>>>
>>> On 8/14/24 17:35, 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,
>>>>
>>>> Thanks,
>>>>
>>>> - Arnaldo
>>>>
>>> If so then the check whether the ulimit is supported doesn't need to be done
>>> as bash is given as a requirement. Thought that it should be supporting all
>>> possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.
>>>
>>> Can I send a quick fix that just changes to the '/bin/bash' so that the code
>>> won't have unnecessary code?
>>
>> See my last message, I went with your v2 + switch from /bin/sh to
>> /bin/bash, as you suggest above.
> 
> And added the '-S' to ulimit, since you changed that from v2, are you
> sure that is better than using just -n? Why?
> 
> - Arnaldo
> 

It doesn't matter for checking the value, 'ulimit -n' is the same as 'ulimit -Sn'.
But when setting up the value without the option -S, both soft and hard limits are
set to the provided value. For us, only raising the soft limit is important, as it
is very unlikely that that hard limit would need to be raised. Also, then the hard
limit would have to be restored separately to its original value.

- Veronika

 

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

[Arnaldo Carvalho de Melo]

On Wed, Aug 14, 2024 at 06:04:27PM +0200, Veronika Molnarova wrote:
> 
> 
> On 8/14/24 17:56, Arnaldo Carvalho de Melo wrote:
> > On Wed, Aug 14, 2024 at 12:54:51PM -0300, Arnaldo Carvalho de Melo wrote:
> >> On Wed, Aug 14, 2024 at 05:42:01PM +0200, Veronika Molnarova wrote:
> >>>
> >>>
> >>> On 8/14/24 17:35, 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,
> >>>>
> >>>> Thanks,
> >>>>
> >>>> - Arnaldo
> >>>>
> >>> If so then the check whether the ulimit is supported doesn't need to be done
> >>> as bash is given as a requirement. Thought that it should be supporting all
> >>> possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.
> >>>
> >>> Can I send a quick fix that just changes to the '/bin/bash' so that the code
> >>> won't have unnecessary code?
> >>
> >> See my last message, I went with your v2 + switch from /bin/sh to
> >> /bin/bash, as you suggest above.
> > 
> > And added the '-S' to ulimit, since you changed that from v2, are you
> > sure that is better than using just -n? Why?
> > 
> > - Arnaldo
> > 
> 
> It doesn't matter for checking the value, 'ulimit -n' is the same as 'ulimit -Sn'.
> But when setting up the value without the option -S, both soft and hard limits are
> set to the provided value. For us, only raising the soft limit is important, as it
> is very unlikely that that hard limit would need to be raised. Also, then the hard
> limit would have to be restored separately to its original value.

Thanks for the explanation, I kept the -S.

- Arnaldo

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

[Veronika Molnarova]

On 8/14/24 17:54, Arnaldo Carvalho de Melo wrote:
> On Wed, Aug 14, 2024 at 05:42:01PM +0200, Veronika Molnarova wrote:
>>
>>
>> On 8/14/24 17:35, 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,
>>>
>>> Thanks,
>>>
>>> - Arnaldo
>>>
>> If so then the check whether the ulimit is supported doesn't need to be done
>> as bash is given as a requirement. Thought that it should be supporting all
>> possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.
>>
>> Can I send a quick fix that just changes to the '/bin/bash' so that the code
>> won't have unnecessary code?
> 
> See my last message, I went with your v2 + switch from /bin/sh to
> /bin/bash, as you suggest above.
> 
> - Arnaldo
> 

Yeah, that's what I have just sent. Hopefully all good. Either way works fine.

Thanks,
Veronika

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

[Arnaldo Carvalho de Melo]

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

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

[vmolnaro]

From: Veronika Molnarova <vmolnaro@redhat.com>

Subtest for system-wide record with '--threads=cpu' option fails due
to a limit of open file descriptors on systems with 128 or more CPUs
as the default limit is set to 1024.

The number of open file descriptors should be slightly above
nmb_events*nmb_cpus + nmb_cpus(for perf.data.n) + 4*nmb_cpus(for pipes),
which equals 8*nmb_cpus. Therefore, temporarily raise the limit to
16*nmb_cpus for the test.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Radostin Stoyanov <rstoyano@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
---
v2: Rebase the patch to apply upstream
v3: Switch to bash to fix shellcheck warnings
---
 tools/perf/tests/shell/record.sh | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 3d1a7759a7b2..02b706792737 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
 
@@ -22,6 +22,15 @@ cpu_pmu_dir="/sys/bus/event_source/devices/cpu*"
 br_cntr_file="/caps/branch_counter_nr"
 br_cntr_output="branch stack counters"
 
+# 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))
+default_fd_limit=$(ulimit -Sn)
+
 cleanup() {
   rm -rf "${perfdata}"
   rm -rf "${perfdata}".old
@@ -190,11 +199,19 @@ test_branch_counter() {
   echo "Basic branch counter test [Success]"
 }
 
+# raise the limit of file descriptors to required minimum
+if [ $default_fd_limit -lt $min_fd_limit ]; then
+  ulimit -Sn $min_fd_limit
+fi
+
 test_per_thread
 test_register_capture
 test_system_wide
 test_workload
 test_branch_counter
 
+# restore the default value
+ulimit -Sn $default_fd_limit
+
 cleanup
 exit $err
-- 
2.43.0