evsel__increase_rlimit() helper does nothing with evsel, and description
of the functionality is inaccurate, rename it and move to util/rlimit.c.
By the way, fix a checkppatch warning about misplaced license tag:
WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
#160: FILE: tools/perf/util/rlimit.h:3:
/* SPDX-License-Identifier: LGPL-2.1 */
No functional change.
Signed-off-by: Yang Jihong <[email protected]>
---
tools/perf/util/data.c | 4 ++--
tools/perf/util/evsel.c | 30 ++----------------------------
tools/perf/util/evsel.h | 3 ---
tools/perf/util/rlimit.c | 28 ++++++++++++++++++++++++++++
tools/perf/util/rlimit.h | 11 ++++++++++-
5 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index 098f9e3bb2e7..c29d8a382b19 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -17,7 +17,7 @@
#include "util.h" // rm_rf_perf_data()
#include "debug.h"
#include "header.h"
-#include "evsel.h"
+#include "rlimit.h"
#include <internal/lib.h>
static void close_dir(struct perf_data_file *files, int nr)
@@ -64,7 +64,7 @@ int perf_data__create_dir(struct perf_data *data, int nr)
* perf record needs at least 6 fds per CPU.
* When we run out of them try to increase the limits.
*/
- if (errno == EMFILE && evsel__increase_rlimit(&set_rlimit))
+ if (errno == EMFILE && rlimit__increase_nofile(&set_rlimit))
goto retry_open;
ret = -errno;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d5363d23f5d3..72a5dfc38d38 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -49,6 +49,7 @@
#include "off_cpu.h"
#include "pmu.h"
#include "pmus.h"
+#include "rlimit.h"
#include "../perf-sys.h"
#include "util/parse-branch-options.h"
#include "util/bpf-filter.h"
@@ -1989,33 +1990,6 @@ bool evsel__detect_missing_features(struct evsel *evsel)
}
}
-bool evsel__increase_rlimit(enum rlimit_action *set_rlimit)
-{
- int old_errno;
- struct rlimit l;
-
- if (*set_rlimit < INCREASED_MAX) {
- old_errno = errno;
-
- if (getrlimit(RLIMIT_NOFILE, &l) == 0) {
- if (*set_rlimit == NO_CHANGE) {
- l.rlim_cur = l.rlim_max;
- } else {
- l.rlim_cur = l.rlim_max + 1000;
- l.rlim_max = l.rlim_cur;
- }
- if (setrlimit(RLIMIT_NOFILE, &l) == 0) {
- (*set_rlimit) += 1;
- errno = old_errno;
- return true;
- }
- }
- errno = old_errno;
- }
-
- return false;
-}
-
static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
struct perf_thread_map *threads,
int start_cpu_map_idx, int end_cpu_map_idx)
@@ -2143,7 +2117,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
* perf stat needs between 5 and 22 fds per CPU. When we run out
* of them try to increase the limits.
*/
- if (err == -EMFILE && evsel__increase_rlimit(&set_rlimit))
+ if (err == -EMFILE && rlimit__increase_nofile(&set_rlimit))
goto retry_open;
if (err != -EINVAL || idx > 0 || thread > 0)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 815be2491938..d791316a1792 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -330,9 +330,6 @@ int evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
struct perf_thread_map *threads);
bool evsel__detect_missing_features(struct evsel *evsel);
-enum rlimit_action { NO_CHANGE, SET_TO_MAX, INCREASED_MAX };
-bool evsel__increase_rlimit(enum rlimit_action *set_rlimit);
-
bool evsel__precise_ip_fallback(struct evsel *evsel);
struct perf_sample;
diff --git a/tools/perf/util/rlimit.c b/tools/perf/util/rlimit.c
index 13521d392a22..f857405fe1aa 100644
--- a/tools/perf/util/rlimit.c
+++ b/tools/perf/util/rlimit.c
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: LGPL-2.1 */
+#include <errno.h>
#include "util/debug.h"
#include "util/rlimit.h"
#include <sys/time.h>
@@ -27,3 +28,30 @@ void rlimit__bump_memlock(void)
}
}
}
+
+bool rlimit__increase_nofile(enum rlimit_action *set_rlimit)
+{
+ int old_errno;
+ struct rlimit l;
+
+ if (*set_rlimit < INCREASED_MAX) {
+ old_errno = errno;
+
+ if (getrlimit(RLIMIT_NOFILE, &l) == 0) {
+ if (*set_rlimit == NO_CHANGE) {
+ l.rlim_cur = l.rlim_max;
+ } else {
+ l.rlim_cur = l.rlim_max + 1000;
+ l.rlim_max = l.rlim_cur;
+ }
+ if (setrlimit(RLIMIT_NOFILE, &l) == 0) {
+ (*set_rlimit) += 1;
+ errno = old_errno;
+ return true;
+ }
+ }
+ errno = old_errno;
+ }
+
+ return false;
+}
diff --git a/tools/perf/util/rlimit.h b/tools/perf/util/rlimit.h
index 9f59d8e710a3..19050d7fb9d7 100644
--- a/tools/perf/util/rlimit.h
+++ b/tools/perf/util/rlimit.h
@@ -1,6 +1,15 @@
+/* SPDX-License-Identifier: LGPL-2.1 */
#ifndef __PERF_RLIMIT_H_
#define __PERF_RLIMIT_H_
-/* SPDX-License-Identifier: LGPL-2.1 */
+
+enum rlimit_action {
+ NO_CHANGE,
+ SET_TO_MAX,
+ INCREASED_MAX
+};
void rlimit__bump_memlock(void);
+
+bool rlimit__increase_nofile(enum rlimit_action *set_rlimit);
+
#endif // __PERF_RLIMIT_H_
--
2.34.1
On Mon, 23 Oct 2023 03:31:44 +0000, Yang Jihong wrote:
> evsel__increase_rlimit() helper does nothing with evsel, and description
> of the functionality is inaccurate, rename it and move to util/rlimit.c.
>
> By the way, fix a checkppatch warning about misplaced license tag:
>
> WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
> #160: FILE: tools/perf/util/rlimit.h:3:
> /* SPDX-License-Identifier: LGPL-2.1 */
>
> [...]
Applied to perf-tools-next, thanks!
Em Mon, Oct 23, 2023 at 03:31:44AM +0000, Yang Jihong escreveu:
> evsel__increase_rlimit() helper does nothing with evsel, and description
> of the functionality is inaccurate, rename it and move to util/rlimit.c.
> By the way, fix a checkppatch warning about misplaced license tag:
> WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
> #160: FILE: tools/perf/util/rlimit.h:3:
> /* SPDX-License-Identifier: LGPL-2.1 */
> No functional change.
Please run 'perf test' before sending patches upstream, I'm checking if
what is in perf-tools-next/perf-tools-next is building and I noticed
this:
⬢[acme@toolbox perf-tools-next]$ perf test -v python
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
14: 'import perf' in python :
--- start ---
test child forked, pid 2912462
python usage test: "echo "import sys ; sys.path.insert(0, '/tmp/build/perf-tools-next/python'); import perf" | '/usr/bin/python3' "
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: /tmp/build/perf-tools-next/python/perf.cpython-311-x86_64-linux-gnu.so: undefined symbol: rlimit__increase_nofile
test child finished with -1
---- end ----
'import perf' in python: FAILED!
⬢[acme@toolbox perf-tools-next]$
The following patch cures it, Namhyung, can you please fold it and force
push perf-tools-next/perf-tools-next or let me know if you prefer that I
submit a patch fixing this separately.
- Arnaldo
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index 26e1c8d973ea0b95..593b660ec75e24e1 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -40,6 +40,7 @@ util/rwsem.c
util/hashmap.c
util/perf_regs.c
util/fncache.c
+util/rlimit.c
util/perf-regs-arch/perf_regs_aarch64.c
util/perf-regs-arch/perf_regs_arm.c
util/perf-regs-arch/perf_regs_csky.c
Hi Arnaldo,
On Thu, Oct 26, 2023 at 12:47 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Mon, Oct 23, 2023 at 03:31:44AM +0000, Yang Jihong escreveu:
> > evsel__increase_rlimit() helper does nothing with evsel, and description
> > of the functionality is inaccurate, rename it and move to util/rlimit.c.
>
> > By the way, fix a checkppatch warning about misplaced license tag:
>
> > WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
> > #160: FILE: tools/perf/util/rlimit.h:3:
> > /* SPDX-License-Identifier: LGPL-2.1 */
>
> > No functional change.
>
> Please run 'perf test' before sending patches upstream, I'm checking if
> what is in perf-tools-next/perf-tools-next is building and I noticed
> this:
>
> ⬢[acme@toolbox perf-tools-next]$ perf test -v python
> Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> 14: 'import perf' in python :
> --- start ---
> test child forked, pid 2912462
> python usage test: "echo "import sys ; sys.path.insert(0, '/tmp/build/perf-tools-next/python'); import perf" | '/usr/bin/python3' "
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> ImportError: /tmp/build/perf-tools-next/python/perf.cpython-311-x86_64-linux-gnu.so: undefined symbol: rlimit__increase_nofile
> test child finished with -1
> ---- end ----
> 'import perf' in python: FAILED!
> ⬢[acme@toolbox perf-tools-next]$
>
> The following patch cures it, Namhyung, can you please fold it and force
> push perf-tools-next/perf-tools-next or let me know if you prefer that I
> submit a patch fixing this separately.
Thanks for fixing this. I prefer having a separate fix.
Please send it as a formal patch.
Thanks,
Namhyung
>
> - Arnaldo
>
> diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
> index 26e1c8d973ea0b95..593b660ec75e24e1 100644
> --- a/tools/perf/util/python-ext-sources
> +++ b/tools/perf/util/python-ext-sources
> @@ -40,6 +40,7 @@ util/rwsem.c
> util/hashmap.c
> util/perf_regs.c
> util/fncache.c
> +util/rlimit.c
> util/perf-regs-arch/perf_regs_aarch64.c
> util/perf-regs-arch/perf_regs_arm.c
> util/perf-regs-arch/perf_regs_csky.c
Hello,
On 2023/10/27 6:08, Namhyung Kim wrote:
> Hi Arnaldo,
>
> On Thu, Oct 26, 2023 at 12:47 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
>>
>> Em Mon, Oct 23, 2023 at 03:31:44AM +0000, Yang Jihong escreveu:
>>> evsel__increase_rlimit() helper does nothing with evsel, and description
>>> of the functionality is inaccurate, rename it and move to util/rlimit.c.
>>
>>> By the way, fix a checkppatch warning about misplaced license tag:
>>
>>> WARNING: Misplaced SPDX-License-Identifier tag - use line 1 instead
>>> #160: FILE: tools/perf/util/rlimit.h:3:
>>> /* SPDX-License-Identifier: LGPL-2.1 */
>>
>>> No functional change.
>>
>> Please run 'perf test' before sending patches upstream, I'm checking if
>> what is in perf-tools-next/perf-tools-next is building and I noticed
>> this:
>>
>> ⬢[acme@toolbox perf-tools-next]$ perf test -v python
>> Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
>> 14: 'import perf' in python :
>> --- start ---
>> test child forked, pid 2912462
>> python usage test: "echo "import sys ; sys.path.insert(0, '/tmp/build/perf-tools-next/python'); import perf" | '/usr/bin/python3' "
>> Traceback (most recent call last):
>> File "<stdin>", line 1, in <module>
>> ImportError: /tmp/build/perf-tools-next/python/perf.cpython-311-x86_64-linux-gnu.so: undefined symbol: rlimit__increase_nofile
>> test child finished with -1
>> ---- end ----
>> 'import perf' in python: FAILED!
>> ⬢[acme@toolbox perf-tools-next]$
>>
>> The following patch cures it, Namhyung, can you please fold it and force
>> push perf-tools-next/perf-tools-next or let me know if you prefer that I
>> submit a patch fixing this separately.
>
> Thanks for fixing this. I prefer having a separate fix.
> Please send it as a formal patch.
>
Sorry, only rename helper and no function change.
I didn't consider the scenario that might affect test python.
I've submitted a fix that can be used if needed:
https://lore.kernel.org/lkml/[email protected]/
Also, can we consider identifying this problem at the stage of compiling
the perf tool?
Thanks,
Yang
The changes in ("perf evsel: Rename evsel__increase_rlimit to
rlimit__increase_nofile") ended up breaking the python binding that now
references the rlimit__increase_nofile function, add the util/rlimit.o
to the tools/perf/util/python-ext-sources to cure that.
This was detected by the 'perf test python' regression test:
$ perf test python
14: 'import perf' in python : FAILED!
$ perf test -v python
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
14: 'import perf' in python :
--- start ---
test child forked, pid 2912462
python usage test: "echo "import sys ; sys.path.insert(0, '/tmp/build/perf-tools-next/python'); import perf" | '/usr/bin/python3' "
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: /tmp/build/perf-tools-next/python/perf.cpython-311-x86_64-linux-gnu.so: undefined symbol: rlimit__increase_nofile
test child finished with -1
---- end ----
'import perf' in python: FAILED!
$
Fixes: e093a222d7cba1eb ("perf evsel: Rename evsel__increase_rlimit to rlimit__increase_nofile")
Acked-by: Namhyung Kim <[email protected]>
Acked-by: Yang Jihong <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Jiri Olsa <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/python-ext-sources | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index 26e1c8d973ea0b95..593b660ec75e24e1 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -40,6 +40,7 @@ util/rwsem.c
util/hashmap.c
util/perf_regs.c
util/fncache.c
+util/rlimit.c
util/perf-regs-arch/perf_regs_aarch64.c
util/perf-regs-arch/perf_regs_arm.c
util/perf-regs-arch/perf_regs_csky.c
--
2.41.0
Em Fri, Oct 27, 2023 at 10:43:32AM +0800, Yang Jihong escreveu:
> Sorry, only rename helper and no function change.
> I didn't consider the scenario that might affect test python.
> I've submitted a fix that can be used if needed:
> https://lore.kernel.org/lkml/[email protected]/
> Also, can we consider identifying this problem at the stage of compiling the
> perf tool?
Yes, I have:
$ alias m='rm -rf ~/libexec/perf-core/ ; make -k CORESIGHT=1 O=/tmp/build/$(basename $PWD) -C tools/perf install-bin && perf test python'
And use the 'm' alias to build perf.
I also run 'perf test' before sending patches upstream, in addition to:
make -C tools/perf build-test
I encourage everybody to do the same.
Additionally I run a long list of container tests on dozens of distros,
but those should really be running on some CI system :-\
- Arnaldo
On Fri, 27 Oct 2023 10:33:30 -0300, Arnaldo Carvalho de Melo wrote:
> The changes in ("perf evsel: Rename evsel__increase_rlimit to
> rlimit__increase_nofile") ended up breaking the python binding that now
> references the rlimit__increase_nofile function, add the util/rlimit.o
> to the tools/perf/util/python-ext-sources to cure that.
>
> This was detected by the 'perf test python' regression test:
>
> [...]
Applied to perf-tools-next, thanks!