The perf subsystem today unifies various tracing and monitoring
features, from both software and hardware. One benefit of the perf
subsystem is automatically inheriting events to child tasks, which
enables process-wide events monitoring with low overheads. By default
perf events are non-intrusive, not affecting behaviour of the tasks
being monitored.
For certain use-cases, however, it makes sense to leverage the
generality of the perf events subsystem and optionally allow the tasks
being monitored to receive signals on events they are interested in.
This patch series adds the option to synchronously signal user space on
events.
To better support process-wide synchronous self-monitoring, without
events propagating to children that do not share the current process's
shared environment, two pre-requisite patches are added to optionally
restrict inheritance to CLONE_THREAD, and remove events on exec (without
affecting the parent).
Examples how to use these features can be found in the tests added at
the end of the series. In addition to the tests added, the series has
also been subjected to syzkaller fuzzing (focus on 'kernel/events/'
coverage).
Motivation and Example Uses
---------------------------
1. Our immediate motivation is low-overhead sampling-based race
detection for user space [1]. By using perf_event_open() at
process initialization, we can create hardware
breakpoint/watchpoint events that are propagated automatically
to all threads in a process. As far as we are aware, today no
existing kernel facility (such as ptrace) allows us to set up
process-wide watchpoints with minimal overheads (that are
comparable to mprotect() of whole pages).
2. Other low-overhead error detectors that rely on detecting
accesses to certain memory locations or code, process-wide and
also only in a specific set of subtasks or threads.
[1] https://llvm.org/devmtg/2020-09/slides/Morehouse-GWP-Tsan.pdf
Other ideas for use-cases we found interesting, but should only
illustrate the range of potential to further motivate the utility (we're
sure there are more):
3. Code hot patching without full stop-the-world. Specifically, by
setting a code breakpoint to entry to the patched routine, then
send signals to threads and check that they are not in the
routine, but without stopping them further. If any of the
threads will enter the routine, it will receive SIGTRAP and
pause.
4. Safepoints without mprotect(). Some Java implementations use
"load from a known memory location" as a safepoint. When threads
need to be stopped, the page containing the location is
mprotect()ed and threads get a signal. This could be replaced with
a watchpoint, which does not require a whole page nor DTLB
shootdowns.
5. Threads receiving signals on performance events to
throttle/unthrottle themselves.
6. Tracking data flow globally.
Changelog
---------
v4:
* Fix for parent and child racing to exit in sync_child_event().
* Fix race between irq_work running and task's sighand being released by
release_task().
* Generalize setting si_perf and si_addr independent of event type;
introduces perf_event_attr::sig_data, which can be set by user space
to be propagated to si_perf.
* Warning in perf_sigtrap() if ctx->task and current mismatch; we expect
this on architectures that do not properly implement
arch_irq_work_raise().
* Require events that want sigtrap to be associated with a task.
* Dropped "perf: Add breakpoint information to siginfo on SIGTRAP"
in favor of more generic solution (perf_event_attr::sig_data).
v3:
* Add patch "perf: Rework perf_event_exit_event()" to beginning of
series, courtesy of Peter Zijlstra.
* Rework "perf: Add support for event removal on exec" based on
the added "perf: Rework perf_event_exit_event()".
* Fix kselftests to work with more recent libc, due to the way it forces
using the kernel's own siginfo_t.
* Add basic perf-tool built-in test.
v2/RFC: https://lkml.kernel.org/r/[email protected]
* Patch "Support only inheriting events if cloned with CLONE_THREAD"
added to series.
* Patch "Add support for event removal on exec" added to series.
* Patch "Add kselftest for process-wide sigtrap handling" added to
series.
* Patch "Add kselftest for remove_on_exec" added to series.
* Implicitly restrict inheriting events if sigtrap, but the child was
cloned with CLONE_CLEAR_SIGHAND, because it is not generally safe if
the child cleared all signal handlers to continue sending SIGTRAP.
* Various minor fixes (see details in patches).
v1/RFC: https://lkml.kernel.org/r/[email protected]
Pre-series: The discussion at [2] led to the changes in this series. The
approach taken in "Add support for SIGTRAP on perf events" to trigger
the signal was suggested by Peter Zijlstra in [3].
[2] https://lore.kernel.org/lkml/CACT4Y+YPrXGw+AtESxAgPyZ84TYkNZdP0xpocX2jwVAbZD=-XQ@mail.gmail.com/
[3] https://lore.kernel.org/lkml/[email protected]/
Marco Elver (9):
perf: Apply PERF_EVENT_IOC_MODIFY_ATTRIBUTES to children
perf: Support only inheriting events if cloned with CLONE_THREAD
perf: Add support for event removal on exec
signal: Introduce TRAP_PERF si_code and si_perf to siginfo
perf: Add support for SIGTRAP on perf events
selftests/perf_events: Add kselftest for process-wide sigtrap handling
selftests/perf_events: Add kselftest for remove_on_exec
tools headers uapi: Sync tools/include/uapi/linux/perf_event.h
perf test: Add basic stress test for sigtrap handling
Peter Zijlstra (1):
perf: Rework perf_event_exit_event()
arch/m68k/kernel/signal.c | 3 +
arch/x86/kernel/signal_compat.c | 5 +-
fs/signalfd.c | 4 +
include/linux/compat.h | 2 +
include/linux/perf_event.h | 9 +-
include/linux/signal.h | 1 +
include/uapi/asm-generic/siginfo.h | 6 +-
include/uapi/linux/perf_event.h | 12 +-
include/uapi/linux/signalfd.h | 4 +-
kernel/events/core.c | 302 +++++++++++++-----
kernel/fork.c | 2 +-
kernel/signal.c | 11 +
tools/include/uapi/linux/perf_event.h | 12 +-
tools/perf/tests/Build | 1 +
tools/perf/tests/builtin-test.c | 5 +
tools/perf/tests/sigtrap.c | 150 +++++++++
tools/perf/tests/tests.h | 1 +
.../testing/selftests/perf_events/.gitignore | 3 +
tools/testing/selftests/perf_events/Makefile | 6 +
tools/testing/selftests/perf_events/config | 1 +
.../selftests/perf_events/remove_on_exec.c | 260 +++++++++++++++
tools/testing/selftests/perf_events/settings | 1 +
.../selftests/perf_events/sigtrap_threads.c | 210 ++++++++++++
23 files changed, 924 insertions(+), 87 deletions(-)
create mode 100644 tools/perf/tests/sigtrap.c
create mode 100644 tools/testing/selftests/perf_events/.gitignore
create mode 100644 tools/testing/selftests/perf_events/Makefile
create mode 100644 tools/testing/selftests/perf_events/config
create mode 100644 tools/testing/selftests/perf_events/remove_on_exec.c
create mode 100644 tools/testing/selftests/perf_events/settings
create mode 100644 tools/testing/selftests/perf_events/sigtrap_threads.c
--
2.31.0.208.g409f899ff0-goog
As with other ioctls (such as PERF_EVENT_IOC_{ENABLE,DISABLE}), fix up
handling of PERF_EVENT_IOC_MODIFY_ATTRIBUTES to also apply to children.
Suggested-by: Dmitry Vyukov <[email protected]>
Reviewed-by: Dmitry Vyukov <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
kernel/events/core.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e77294c7e654..a9a0a46909af 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3199,16 +3199,36 @@ static int perf_event_modify_breakpoint(struct perf_event *bp,
static int perf_event_modify_attr(struct perf_event *event,
struct perf_event_attr *attr)
{
+ int (*func)(struct perf_event *, struct perf_event_attr *);
+ struct perf_event *child;
+ int err;
+
if (event->attr.type != attr->type)
return -EINVAL;
switch (event->attr.type) {
case PERF_TYPE_BREAKPOINT:
- return perf_event_modify_breakpoint(event, attr);
+ func = perf_event_modify_breakpoint;
+ break;
default:
/* Place holder for future additions. */
return -EOPNOTSUPP;
}
+
+ WARN_ON_ONCE(event->ctx->parent_ctx);
+
+ mutex_lock(&event->child_mutex);
+ err = func(event, attr);
+ if (err)
+ goto out;
+ list_for_each_entry(child, &event->child_list, child_list) {
+ err = func(child, attr);
+ if (err)
+ goto out;
+ }
+out:
+ mutex_unlock(&event->child_mutex);
+ return err;
}
static void ctx_sched_out(struct perf_event_context *ctx,
--
2.31.0.208.g409f899ff0-goog
Adds bit perf_event_attr::inherit_thread, to restricting inheriting
events only if the child was cloned with CLONE_THREAD.
This option supports the case where an event is supposed to be
process-wide only (including subthreads), but should not propagate
beyond the current process's shared environment.
Link: https://lore.kernel.org/lkml/YBvj6eJR%[email protected]/
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
v2:
* Add patch to series.
---
include/linux/perf_event.h | 5 +++--
include/uapi/linux/perf_event.h | 3 ++-
kernel/events/core.c | 21 ++++++++++++++-------
kernel/fork.c | 2 +-
4 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 3d478abf411c..1660039199b2 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -958,7 +958,7 @@ extern void __perf_event_task_sched_in(struct task_struct *prev,
struct task_struct *task);
extern void __perf_event_task_sched_out(struct task_struct *prev,
struct task_struct *next);
-extern int perf_event_init_task(struct task_struct *child);
+extern int perf_event_init_task(struct task_struct *child, u64 clone_flags);
extern void perf_event_exit_task(struct task_struct *child);
extern void perf_event_free_task(struct task_struct *task);
extern void perf_event_delayed_put(struct task_struct *task);
@@ -1449,7 +1449,8 @@ perf_event_task_sched_in(struct task_struct *prev,
static inline void
perf_event_task_sched_out(struct task_struct *prev,
struct task_struct *next) { }
-static inline int perf_event_init_task(struct task_struct *child) { return 0; }
+static inline int perf_event_init_task(struct task_struct *child,
+ u64 clone_flags) { return 0; }
static inline void perf_event_exit_task(struct task_struct *child) { }
static inline void perf_event_free_task(struct task_struct *task) { }
static inline void perf_event_delayed_put(struct task_struct *task) { }
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index ad15e40d7f5d..813efb65fea8 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -389,7 +389,8 @@ struct perf_event_attr {
cgroup : 1, /* include cgroup events */
text_poke : 1, /* include text poke events */
build_id : 1, /* use build id in mmap2 events */
- __reserved_1 : 29;
+ inherit_thread : 1, /* children only inherit if cloned with CLONE_THREAD */
+ __reserved_1 : 28;
union {
__u32 wakeup_events; /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index a9a0a46909af..de2917b3c59e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11649,6 +11649,9 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
(attr->sample_type & PERF_SAMPLE_WEIGHT_STRUCT))
return -EINVAL;
+ if (!attr->inherit && attr->inherit_thread)
+ return -EINVAL;
+
out:
return ret;
@@ -12869,12 +12872,13 @@ static int
inherit_task_group(struct perf_event *event, struct task_struct *parent,
struct perf_event_context *parent_ctx,
struct task_struct *child, int ctxn,
- int *inherited_all)
+ u64 clone_flags, int *inherited_all)
{
int ret;
struct perf_event_context *child_ctx;
- if (!event->attr.inherit) {
+ if (!event->attr.inherit ||
+ (event->attr.inherit_thread && !(clone_flags & CLONE_THREAD))) {
*inherited_all = 0;
return 0;
}
@@ -12906,7 +12910,8 @@ inherit_task_group(struct perf_event *event, struct task_struct *parent,
/*
* Initialize the perf_event context in task_struct
*/
-static int perf_event_init_context(struct task_struct *child, int ctxn)
+static int perf_event_init_context(struct task_struct *child, int ctxn,
+ u64 clone_flags)
{
struct perf_event_context *child_ctx, *parent_ctx;
struct perf_event_context *cloned_ctx;
@@ -12946,7 +12951,8 @@ static int perf_event_init_context(struct task_struct *child, int ctxn)
*/
perf_event_groups_for_each(event, &parent_ctx->pinned_groups) {
ret = inherit_task_group(event, parent, parent_ctx,
- child, ctxn, &inherited_all);
+ child, ctxn, clone_flags,
+ &inherited_all);
if (ret)
goto out_unlock;
}
@@ -12962,7 +12968,8 @@ static int perf_event_init_context(struct task_struct *child, int ctxn)
perf_event_groups_for_each(event, &parent_ctx->flexible_groups) {
ret = inherit_task_group(event, parent, parent_ctx,
- child, ctxn, &inherited_all);
+ child, ctxn, clone_flags,
+ &inherited_all);
if (ret)
goto out_unlock;
}
@@ -13004,7 +13011,7 @@ static int perf_event_init_context(struct task_struct *child, int ctxn)
/*
* Initialize the perf_event context in task_struct
*/
-int perf_event_init_task(struct task_struct *child)
+int perf_event_init_task(struct task_struct *child, u64 clone_flags)
{
int ctxn, ret;
@@ -13013,7 +13020,7 @@ int perf_event_init_task(struct task_struct *child)
INIT_LIST_HEAD(&child->perf_event_list);
for_each_task_context_nr(ctxn) {
- ret = perf_event_init_context(child, ctxn);
+ ret = perf_event_init_context(child, ctxn, clone_flags);
if (ret) {
perf_event_free_task(child);
return ret;
diff --git a/kernel/fork.c b/kernel/fork.c
index 426cd0c51f9e..f592c9a0272a 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2084,7 +2084,7 @@ static __latent_entropy struct task_struct *copy_process(
if (retval)
goto bad_fork_cleanup_policy;
- retval = perf_event_init_task(p);
+ retval = perf_event_init_task(p, clone_flags);
if (retval)
goto bad_fork_cleanup_policy;
retval = audit_alloc(p);
--
2.31.0.208.g409f899ff0-goog
Adds bit perf_event_attr::remove_on_exec, to support removing an event
from a task on exec.
This option supports the case where an event is supposed to be
process-wide only, and should not propagate beyond exec, to limit
monitoring to the original process image only.
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
v3:
* Rework based on Peter's "perf: Rework perf_event_exit_event()" added
to the beginning of the series. Intermediate attempts between v2 and
this v3 can be found here:
https://lkml.kernel.org/r/[email protected]
v2:
* Add patch to series.
---
include/uapi/linux/perf_event.h | 3 +-
kernel/events/core.c | 70 +++++++++++++++++++++++++++++----
2 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 813efb65fea8..8c5b9f5ad63f 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -390,7 +390,8 @@ struct perf_event_attr {
text_poke : 1, /* include text poke events */
build_id : 1, /* use build id in mmap2 events */
inherit_thread : 1, /* children only inherit if cloned with CLONE_THREAD */
- __reserved_1 : 28;
+ remove_on_exec : 1, /* event is removed from task on exec */
+ __reserved_1 : 27;
union {
__u32 wakeup_events; /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index de2917b3c59e..19c045ff2b9c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4247,6 +4247,57 @@ static void perf_event_enable_on_exec(int ctxn)
put_ctx(clone_ctx);
}
+static void perf_remove_from_owner(struct perf_event *event);
+static void perf_event_exit_event(struct perf_event *event,
+ struct perf_event_context *ctx);
+
+/*
+ * Removes all events from the current task that have been marked
+ * remove-on-exec, and feeds their values back to parent events.
+ */
+static void perf_event_remove_on_exec(int ctxn)
+{
+ struct perf_event_context *ctx, *clone_ctx = NULL;
+ struct perf_event *event, *next;
+ LIST_HEAD(free_list);
+ unsigned long flags;
+ bool modified = false;
+
+ ctx = perf_pin_task_context(current, ctxn);
+ if (!ctx)
+ return;
+
+ mutex_lock(&ctx->mutex);
+
+ if (WARN_ON_ONCE(ctx->task != current))
+ goto unlock;
+
+ list_for_each_entry_safe(event, next, &ctx->event_list, event_entry) {
+ if (!event->attr.remove_on_exec)
+ continue;
+
+ if (!is_kernel_event(event))
+ perf_remove_from_owner(event);
+
+ modified = true;
+
+ perf_event_exit_event(event, ctx);
+ }
+
+ raw_spin_lock_irqsave(&ctx->lock, flags);
+ if (modified)
+ clone_ctx = unclone_ctx(ctx);
+ --ctx->pin_count;
+ raw_spin_unlock_irqrestore(&ctx->lock, flags);
+
+unlock:
+ mutex_unlock(&ctx->mutex);
+
+ put_ctx(ctx);
+ if (clone_ctx)
+ put_ctx(clone_ctx);
+}
+
struct perf_read_data {
struct perf_event *event;
bool group;
@@ -7559,18 +7610,18 @@ void perf_event_exec(void)
struct perf_event_context *ctx;
int ctxn;
- rcu_read_lock();
for_each_task_context_nr(ctxn) {
- ctx = current->perf_event_ctxp[ctxn];
- if (!ctx)
- continue;
-
perf_event_enable_on_exec(ctxn);
+ perf_event_remove_on_exec(ctxn);
- perf_iterate_ctx(ctx, perf_event_addr_filters_exec, NULL,
- true);
+ rcu_read_lock();
+ ctx = rcu_dereference(current->perf_event_ctxp[ctxn]);
+ if (ctx) {
+ perf_iterate_ctx(ctx, perf_event_addr_filters_exec,
+ NULL, true);
+ }
+ rcu_read_unlock();
}
- rcu_read_unlock();
}
struct remote_output {
@@ -11652,6 +11703,9 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
if (!attr->inherit && attr->inherit_thread)
return -EINVAL;
+ if (attr->remove_on_exec && attr->enable_on_exec)
+ return -EINVAL;
+
out:
return ret;
--
2.31.0.208.g409f899ff0-goog
Introduces the TRAP_PERF si_code, and associated siginfo_t field
si_perf. These will be used by the perf event subsystem to send signals
(if requested) to the task where an event occurred.
Acked-by: Geert Uytterhoeven <[email protected]> # m68k
Acked-by: Arnd Bergmann <[email protected]> # asm-generic
Signed-off-by: Marco Elver <[email protected]>
---
arch/m68k/kernel/signal.c | 3 +++
arch/x86/kernel/signal_compat.c | 5 ++++-
fs/signalfd.c | 4 ++++
include/linux/compat.h | 2 ++
include/linux/signal.h | 1 +
include/uapi/asm-generic/siginfo.h | 6 +++++-
include/uapi/linux/signalfd.h | 4 +++-
kernel/signal.c | 11 +++++++++++
8 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
index 349570f16a78..a4b7ee1df211 100644
--- a/arch/m68k/kernel/signal.c
+++ b/arch/m68k/kernel/signal.c
@@ -622,6 +622,9 @@ static inline void siginfo_build_tests(void)
/* _sigfault._addr_pkey */
BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x12);
+ /* _sigfault._perf */
+ BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x10);
+
/* _sigpoll */
BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c);
BUILD_BUG_ON(offsetof(siginfo_t, si_fd) != 0x10);
diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index a5330ff498f0..0e5d0a7e203b 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -29,7 +29,7 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(NSIGFPE != 15);
BUILD_BUG_ON(NSIGSEGV != 9);
BUILD_BUG_ON(NSIGBUS != 5);
- BUILD_BUG_ON(NSIGTRAP != 5);
+ BUILD_BUG_ON(NSIGTRAP != 6);
BUILD_BUG_ON(NSIGCHLD != 6);
BUILD_BUG_ON(NSIGSYS != 2);
@@ -138,6 +138,9 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x20);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_pkey) != 0x14);
+ BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x18);
+ BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf) != 0x10);
+
CHECK_CSI_OFFSET(_sigpoll);
CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int));
CHECK_SI_SIZE (_sigpoll, 4*sizeof(int));
diff --git a/fs/signalfd.c b/fs/signalfd.c
index 456046e15873..040a1142915f 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -134,6 +134,10 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
#endif
new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
break;
+ case SIL_PERF_EVENT:
+ new.ssi_addr = (long) kinfo->si_addr;
+ new.ssi_perf = kinfo->si_perf;
+ break;
case SIL_CHLD:
new.ssi_pid = kinfo->si_pid;
new.ssi_uid = kinfo->si_uid;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6e65be753603..c8821d966812 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -236,6 +236,8 @@ typedef struct compat_siginfo {
char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD];
u32 _pkey;
} _addr_pkey;
+ /* used when si_code=TRAP_PERF */
+ compat_u64 _perf;
};
} _sigfault;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 205526c4003a..1e98548d7cf6 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -43,6 +43,7 @@ enum siginfo_layout {
SIL_FAULT_MCEERR,
SIL_FAULT_BNDERR,
SIL_FAULT_PKUERR,
+ SIL_PERF_EVENT,
SIL_CHLD,
SIL_RT,
SIL_SYS,
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index d2597000407a..d0bb9125c853 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -91,6 +91,8 @@ union __sifields {
char _dummy_pkey[__ADDR_BND_PKEY_PAD];
__u32 _pkey;
} _addr_pkey;
+ /* used when si_code=TRAP_PERF */
+ __u64 _perf;
};
} _sigfault;
@@ -155,6 +157,7 @@ typedef struct siginfo {
#define si_lower _sifields._sigfault._addr_bnd._lower
#define si_upper _sifields._sigfault._addr_bnd._upper
#define si_pkey _sifields._sigfault._addr_pkey._pkey
+#define si_perf _sifields._sigfault._perf
#define si_band _sifields._sigpoll._band
#define si_fd _sifields._sigpoll._fd
#define si_call_addr _sifields._sigsys._call_addr
@@ -253,7 +256,8 @@ typedef struct siginfo {
#define TRAP_BRANCH 3 /* process taken branch trap */
#define TRAP_HWBKPT 4 /* hardware breakpoint/watchpoint */
#define TRAP_UNK 5 /* undiagnosed trap */
-#define NSIGTRAP 5
+#define TRAP_PERF 6 /* perf event with sigtrap=1 */
+#define NSIGTRAP 6
/*
* There is an additional set of SIGTRAP si_codes used by ptrace
diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h
index 83429a05b698..7e333042c7e3 100644
--- a/include/uapi/linux/signalfd.h
+++ b/include/uapi/linux/signalfd.h
@@ -39,6 +39,8 @@ struct signalfd_siginfo {
__s32 ssi_syscall;
__u64 ssi_call_addr;
__u32 ssi_arch;
+ __u32 __pad3;
+ __u64 ssi_perf;
/*
* Pad strcture to 128 bytes. Remember to update the
@@ -49,7 +51,7 @@ struct signalfd_siginfo {
* comes out of a read(2) and we really don't want to have
* a compat on read(2).
*/
- __u8 __pad[28];
+ __u8 __pad[16];
};
diff --git a/kernel/signal.c b/kernel/signal.c
index f2718350bf4b..7061e4957650 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1199,6 +1199,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
+ case SIL_PERF_EVENT:
case SIL_SYS:
ret = false;
break;
@@ -2531,6 +2532,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
+ case SIL_PERF_EVENT:
ksig->info.si_addr = arch_untagged_si_addr(
ksig->info.si_addr, ksig->sig, ksig->info.si_code);
break;
@@ -3341,6 +3343,10 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
#endif
to->si_pkey = from->si_pkey;
break;
+ case SIL_PERF_EVENT:
+ to->si_addr = ptr_to_compat(from->si_addr);
+ to->si_perf = from->si_perf;
+ break;
case SIL_CHLD:
to->si_pid = from->si_pid;
to->si_uid = from->si_uid;
@@ -3421,6 +3427,10 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
#endif
to->si_pkey = from->si_pkey;
break;
+ case SIL_PERF_EVENT:
+ to->si_addr = compat_ptr(from->si_addr);
+ to->si_perf = from->si_perf;
+ break;
case SIL_CHLD:
to->si_pid = from->si_pid;
to->si_uid = from->si_uid;
@@ -4601,6 +4611,7 @@ static inline void siginfo_buildtime_checks(void)
CHECK_OFFSET(si_lower);
CHECK_OFFSET(si_upper);
CHECK_OFFSET(si_pkey);
+ CHECK_OFFSET(si_perf);
/* sigpoll */
CHECK_OFFSET(si_band);
--
2.31.0.208.g409f899ff0-goog
Adds bit perf_event_attr::sigtrap, which can be set to cause events to
send SIGTRAP (with si_code TRAP_PERF) to the task where the event
occurred. The primary motivation is to support synchronous signals on
perf events in the task where an event (such as breakpoints) triggered.
To distinguish perf events based on the event type, the type is set in
si_errno. For events that are associated with an address, si_addr is
copied from perf_sample_data.
The new field perf_event_attr::sig_data is copied to si_perf, which
allows user space to disambiguate which event (of the same type)
triggered the signal. For example, user space could encode the relevant
information it cares about in sig_data.
We note that the choice of an opaque u64 provides the simplest and most
flexible option. Alternatives where a reference to some user space data
is passed back suffer from the problem that modification of referenced
data (be it the event fd, or the perf_event_attr) can race with the
signal being delivered (of course, the same caveat applies if user space
decides to store a pointer in sig_data, but the ABI explicitly avoids
prescribing such a design).
Link: https://lore.kernel.org/lkml/[email protected]/
Suggested-by: Peter Zijlstra <[email protected]>
Acked-by: Dmitry Vyukov <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
v4:
* Generalize setting si_perf and si_addr independent of event type;
introduces perf_event_attr::sig_data, which can be set by user space to
be propagated to si_perf.
* Fix race between irq_work running and task's sighand being released by
release_task().
* Warning in perf_sigtrap() if ctx->task and current mismatch; we expect
this on architectures that do not properly implement
arch_irq_work_raise().
* Require events that want sigtrap to be associated with a task.
v2:
* Use atomic_set(&event_count, 1), since it must always be 0 in
perf_pending_event_disable().
* Implicitly restrict inheriting events if sigtrap, but the child was
cloned with CLONE_CLEAR_SIGHAND, because it is not generally safe if
the child cleared all signal handlers to continue sending SIGTRAP.
---
include/linux/perf_event.h | 3 ++
include/uapi/linux/perf_event.h | 10 ++++++-
kernel/events/core.c | 49 ++++++++++++++++++++++++++++++++-
3 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1660039199b2..18ba1282c5c7 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -778,6 +778,9 @@ struct perf_event {
void *security;
#endif
struct list_head sb_list;
+
+ /* Address associated with event, which can be passed to siginfo_t. */
+ u64 sig_addr;
#endif /* CONFIG_PERF_EVENTS */
};
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 8c5b9f5ad63f..31b00e3b69c9 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -311,6 +311,7 @@ enum perf_event_read_format {
#define PERF_ATTR_SIZE_VER4 104 /* add: sample_regs_intr */
#define PERF_ATTR_SIZE_VER5 112 /* add: aux_watermark */
#define PERF_ATTR_SIZE_VER6 120 /* add: aux_sample_size */
+#define PERF_ATTR_SIZE_VER7 128 /* add: sig_data */
/*
* Hardware event_id to monitor via a performance monitoring event:
@@ -391,7 +392,8 @@ struct perf_event_attr {
build_id : 1, /* use build id in mmap2 events */
inherit_thread : 1, /* children only inherit if cloned with CLONE_THREAD */
remove_on_exec : 1, /* event is removed from task on exec */
- __reserved_1 : 27;
+ sigtrap : 1, /* send synchronous SIGTRAP on event */
+ __reserved_1 : 26;
union {
__u32 wakeup_events; /* wakeup every n events */
@@ -443,6 +445,12 @@ struct perf_event_attr {
__u16 __reserved_2;
__u32 aux_sample_size;
__u32 __reserved_3;
+
+ /*
+ * User provided data if sigtrap=1, passed back to user via
+ * siginfo_t::si_perf, e.g. to permit user to identify the event.
+ */
+ __u64 sig_data;
};
/*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 19c045ff2b9c..1d2077389c0c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6391,6 +6391,33 @@ void perf_event_wakeup(struct perf_event *event)
}
}
+static void perf_sigtrap(struct perf_event *event)
+{
+ struct kernel_siginfo info;
+
+ /*
+ * We'd expect this to only occur if the irq_work is delayed and either
+ * ctx->task or current has changed in the meantime. This can be the
+ * case on architectures that do not implement arch_irq_work_raise().
+ */
+ if (WARN_ON_ONCE(event->ctx->task != current))
+ return;
+
+ /*
+ * perf_pending_event() can race with the task exiting.
+ */
+ if (current->flags & PF_EXITING)
+ return;
+
+ clear_siginfo(&info);
+ info.si_signo = SIGTRAP;
+ info.si_code = TRAP_PERF;
+ info.si_errno = event->attr.type;
+ info.si_perf = event->attr.sig_data;
+ info.si_addr = (void *)event->sig_addr;
+ force_sig_info(&info);
+}
+
static void perf_pending_event_disable(struct perf_event *event)
{
int cpu = READ_ONCE(event->pending_disable);
@@ -6400,6 +6427,13 @@ static void perf_pending_event_disable(struct perf_event *event)
if (cpu == smp_processor_id()) {
WRITE_ONCE(event->pending_disable, -1);
+
+ if (event->attr.sigtrap) {
+ perf_sigtrap(event);
+ atomic_set_release(&event->event_limit, 1); /* rearm event */
+ return;
+ }
+
perf_event_disable_local(event);
return;
}
@@ -9102,6 +9136,7 @@ static int __perf_event_overflow(struct perf_event *event,
if (events && atomic_dec_and_test(&event->event_limit)) {
ret = 1;
event->pending_kill = POLL_HUP;
+ event->sig_addr = data->addr;
perf_event_disable_inatomic(event);
}
@@ -11382,6 +11417,10 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
if (!task || cpu != -1)
return ERR_PTR(-EINVAL);
}
+ if (attr->sigtrap && !task) {
+ /* Requires a task: avoid signalling random tasks. */
+ return ERR_PTR(-EINVAL);
+ }
event = kzalloc(sizeof(*event), GFP_KERNEL);
if (!event)
@@ -11428,6 +11467,9 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
event->state = PERF_EVENT_STATE_INACTIVE;
+ if (event->attr.sigtrap)
+ atomic_set(&event->event_limit, 1);
+
if (task) {
event->attach_state = PERF_ATTACH_TASK;
/*
@@ -11706,6 +11748,9 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
if (attr->remove_on_exec && attr->enable_on_exec)
return -EINVAL;
+ if (attr->sigtrap && !attr->remove_on_exec)
+ return -EINVAL;
+
out:
return ret;
@@ -12932,7 +12977,9 @@ inherit_task_group(struct perf_event *event, struct task_struct *parent,
struct perf_event_context *child_ctx;
if (!event->attr.inherit ||
- (event->attr.inherit_thread && !(clone_flags & CLONE_THREAD))) {
+ (event->attr.inherit_thread && !(clone_flags & CLONE_THREAD)) ||
+ /* Do not inherit if sigtrap and signal handlers were cleared. */
+ (event->attr.sigtrap && (clone_flags & CLONE_CLEAR_SIGHAND))) {
*inherited_all = 0;
return 0;
}
--
2.31.0.208.g409f899ff0-goog
Add a kselftest for testing process-wide perf events with synchronous
SIGTRAP on events (using breakpoints). In particular, we want to test
that changes to the event propagate to all children, and the SIGTRAPs
are in fact synchronously sent to the thread where the event occurred.
Note: The "signal_stress" test case is also added later in the series to
perf tool's built-in tests. The test here is more elaborate in that
respect, which on one hand avoids bloating the perf tool unnecessarily,
but we also benefit from structured tests with TAP-compliant output that
the kselftest framework provides.
Signed-off-by: Marco Elver <[email protected]>
---
v4:
* Update for new perf_event_attr::sig_data / si_perf handling.
v3:
* Fix for latest libc signal.h.
v2:
* Patch added to series.
---
.../testing/selftests/perf_events/.gitignore | 2 +
tools/testing/selftests/perf_events/Makefile | 6 +
tools/testing/selftests/perf_events/config | 1 +
tools/testing/selftests/perf_events/settings | 1 +
.../selftests/perf_events/sigtrap_threads.c | 210 ++++++++++++++++++
5 files changed, 220 insertions(+)
create mode 100644 tools/testing/selftests/perf_events/.gitignore
create mode 100644 tools/testing/selftests/perf_events/Makefile
create mode 100644 tools/testing/selftests/perf_events/config
create mode 100644 tools/testing/selftests/perf_events/settings
create mode 100644 tools/testing/selftests/perf_events/sigtrap_threads.c
diff --git a/tools/testing/selftests/perf_events/.gitignore b/tools/testing/selftests/perf_events/.gitignore
new file mode 100644
index 000000000000..4dc43e1bd79c
--- /dev/null
+++ b/tools/testing/selftests/perf_events/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+sigtrap_threads
diff --git a/tools/testing/selftests/perf_events/Makefile b/tools/testing/selftests/perf_events/Makefile
new file mode 100644
index 000000000000..973a2c39ca83
--- /dev/null
+++ b/tools/testing/selftests/perf_events/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include
+LDFLAGS += -lpthread
+
+TEST_GEN_PROGS := sigtrap_threads
+include ../lib.mk
diff --git a/tools/testing/selftests/perf_events/config b/tools/testing/selftests/perf_events/config
new file mode 100644
index 000000000000..ba58ff2203e4
--- /dev/null
+++ b/tools/testing/selftests/perf_events/config
@@ -0,0 +1 @@
+CONFIG_PERF_EVENTS=y
diff --git a/tools/testing/selftests/perf_events/settings b/tools/testing/selftests/perf_events/settings
new file mode 100644
index 000000000000..6091b45d226b
--- /dev/null
+++ b/tools/testing/selftests/perf_events/settings
@@ -0,0 +1 @@
+timeout=120
diff --git a/tools/testing/selftests/perf_events/sigtrap_threads.c b/tools/testing/selftests/perf_events/sigtrap_threads.c
new file mode 100644
index 000000000000..9c0fd442da60
--- /dev/null
+++ b/tools/testing/selftests/perf_events/sigtrap_threads.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test for perf events with SIGTRAP across all threads.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+/* We need the latest siginfo from the kernel repo. */
+#include <sys/types.h>
+#include <asm/siginfo.h>
+#define __have_siginfo_t 1
+#define __have_sigval_t 1
+#define __have_sigevent_t 1
+#define __siginfo_t_defined
+#define __sigval_t_defined
+#define __sigevent_t_defined
+#define _BITS_SIGINFO_CONSTS_H 1
+#define _BITS_SIGEVENT_CONSTS_H 1
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/perf_event.h>
+#include <pthread.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+#define NUM_THREADS 5
+
+/* Data shared between test body, threads, and signal handler. */
+static struct {
+ int tids_want_signal; /* Which threads still want a signal. */
+ int signal_count; /* Sanity check number of signals received. */
+ volatile int iterate_on; /* Variable to set breakpoint on. */
+ siginfo_t first_siginfo; /* First observed siginfo_t. */
+} ctx;
+
+/* Unique value to check si_perf is correctly set from perf_event_attr::sig_data. */
+#define TEST_SIG_DATA(addr) (~(uint64_t)(addr))
+
+static struct perf_event_attr make_event_attr(bool enabled, volatile void *addr)
+{
+ struct perf_event_attr attr = {
+ .type = PERF_TYPE_BREAKPOINT,
+ .size = sizeof(attr),
+ .sample_period = 1,
+ .disabled = !enabled,
+ .bp_addr = (unsigned long)addr,
+ .bp_type = HW_BREAKPOINT_RW,
+ .bp_len = HW_BREAKPOINT_LEN_1,
+ .inherit = 1, /* Children inherit events ... */
+ .inherit_thread = 1, /* ... but only cloned with CLONE_THREAD. */
+ .remove_on_exec = 1, /* Required by sigtrap. */
+ .sigtrap = 1, /* Request synchronous SIGTRAP on event. */
+ .sig_data = TEST_SIG_DATA(addr),
+ };
+ return attr;
+}
+
+static void sigtrap_handler(int signum, siginfo_t *info, void *ucontext)
+{
+ if (info->si_code != TRAP_PERF) {
+ fprintf(stderr, "%s: unexpected si_code %d\n", __func__, info->si_code);
+ return;
+ }
+
+ /*
+ * The data in siginfo_t we're interested in should all be the same
+ * across threads.
+ */
+ if (!__atomic_fetch_add(&ctx.signal_count, 1, __ATOMIC_RELAXED))
+ ctx.first_siginfo = *info;
+ __atomic_fetch_sub(&ctx.tids_want_signal, syscall(__NR_gettid), __ATOMIC_RELAXED);
+}
+
+static void *test_thread(void *arg)
+{
+ pthread_barrier_t *barrier = (pthread_barrier_t *)arg;
+ pid_t tid = syscall(__NR_gettid);
+ int iter;
+ int i;
+
+ pthread_barrier_wait(barrier);
+
+ __atomic_fetch_add(&ctx.tids_want_signal, tid, __ATOMIC_RELAXED);
+ iter = ctx.iterate_on; /* read */
+ for (i = 0; i < iter - 1; i++) {
+ __atomic_fetch_add(&ctx.tids_want_signal, tid, __ATOMIC_RELAXED);
+ ctx.iterate_on = iter; /* idempotent write */
+ }
+
+ return NULL;
+}
+
+FIXTURE(sigtrap_threads)
+{
+ struct sigaction oldact;
+ pthread_t threads[NUM_THREADS];
+ pthread_barrier_t barrier;
+ int fd;
+};
+
+FIXTURE_SETUP(sigtrap_threads)
+{
+ struct perf_event_attr attr = make_event_attr(false, &ctx.iterate_on);
+ struct sigaction action = {};
+ int i;
+
+ memset(&ctx, 0, sizeof(ctx));
+
+ /* Initialize sigtrap handler. */
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ ASSERT_EQ(sigaction(SIGTRAP, &action, &self->oldact), 0);
+
+ /* Initialize perf event. */
+ self->fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC);
+ ASSERT_NE(self->fd, -1);
+
+ /* Spawn threads inheriting perf event. */
+ pthread_barrier_init(&self->barrier, NULL, NUM_THREADS + 1);
+ for (i = 0; i < NUM_THREADS; i++)
+ ASSERT_EQ(pthread_create(&self->threads[i], NULL, test_thread, &self->barrier), 0);
+}
+
+FIXTURE_TEARDOWN(sigtrap_threads)
+{
+ pthread_barrier_destroy(&self->barrier);
+ close(self->fd);
+ sigaction(SIGTRAP, &self->oldact, NULL);
+}
+
+static void run_test_threads(struct __test_metadata *_metadata,
+ FIXTURE_DATA(sigtrap_threads) *self)
+{
+ int i;
+
+ pthread_barrier_wait(&self->barrier);
+ for (i = 0; i < NUM_THREADS; i++)
+ ASSERT_EQ(pthread_join(self->threads[i], NULL), 0);
+}
+
+TEST_F(sigtrap_threads, remain_disabled)
+{
+ run_test_threads(_metadata, self);
+ EXPECT_EQ(ctx.signal_count, 0);
+ EXPECT_NE(ctx.tids_want_signal, 0);
+}
+
+TEST_F(sigtrap_threads, enable_event)
+{
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ run_test_threads(_metadata, self);
+
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS);
+ EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
+ EXPECT_EQ(ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+ EXPECT_EQ(ctx.first_siginfo.si_perf, TEST_SIG_DATA(&ctx.iterate_on));
+
+ /* Check enabled for parent. */
+ ctx.iterate_on = 0;
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1);
+}
+
+/* Test that modification propagates to all inherited events. */
+TEST_F(sigtrap_threads, modify_and_enable_event)
+{
+ struct perf_event_attr new_attr = make_event_attr(true, &ctx.iterate_on);
+
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &new_attr), 0);
+ run_test_threads(_metadata, self);
+
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS);
+ EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
+ EXPECT_EQ(ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+ EXPECT_EQ(ctx.first_siginfo.si_perf, TEST_SIG_DATA(&ctx.iterate_on));
+
+ /* Check enabled for parent. */
+ ctx.iterate_on = 0;
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1);
+}
+
+/* Stress test event + signal handling. */
+TEST_F(sigtrap_threads, signal_stress)
+{
+ ctx.iterate_on = 3000;
+
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ run_test_threads(_metadata, self);
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_DISABLE, 0), 0);
+
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS * ctx.iterate_on);
+ EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
+ EXPECT_EQ(ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+ EXPECT_EQ(ctx.first_siginfo.si_perf, TEST_SIG_DATA(&ctx.iterate_on));
+}
+
+TEST_HARNESS_MAIN
--
2.31.0.208.g409f899ff0-goog
Add kselftest to test that remove_on_exec removes inherited events from
child tasks.
Signed-off-by: Marco Elver <[email protected]>
---
v3:
* Fix for latest libc signal.h.
v2:
* Add patch to series.
---
.../testing/selftests/perf_events/.gitignore | 1 +
tools/testing/selftests/perf_events/Makefile | 2 +-
.../selftests/perf_events/remove_on_exec.c | 260 ++++++++++++++++++
3 files changed, 262 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/perf_events/remove_on_exec.c
diff --git a/tools/testing/selftests/perf_events/.gitignore b/tools/testing/selftests/perf_events/.gitignore
index 4dc43e1bd79c..790c47001e77 100644
--- a/tools/testing/selftests/perf_events/.gitignore
+++ b/tools/testing/selftests/perf_events/.gitignore
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
sigtrap_threads
+remove_on_exec
diff --git a/tools/testing/selftests/perf_events/Makefile b/tools/testing/selftests/perf_events/Makefile
index 973a2c39ca83..fcafa5f0d34c 100644
--- a/tools/testing/selftests/perf_events/Makefile
+++ b/tools/testing/selftests/perf_events/Makefile
@@ -2,5 +2,5 @@
CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include
LDFLAGS += -lpthread
-TEST_GEN_PROGS := sigtrap_threads
+TEST_GEN_PROGS := sigtrap_threads remove_on_exec
include ../lib.mk
diff --git a/tools/testing/selftests/perf_events/remove_on_exec.c b/tools/testing/selftests/perf_events/remove_on_exec.c
new file mode 100644
index 000000000000..5814611a1dc7
--- /dev/null
+++ b/tools/testing/selftests/perf_events/remove_on_exec.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test for remove_on_exec.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+/* We need the latest siginfo from the kernel repo. */
+#include <sys/types.h>
+#include <asm/siginfo.h>
+#define __have_siginfo_t 1
+#define __have_sigval_t 1
+#define __have_sigevent_t 1
+#define __siginfo_t_defined
+#define __sigval_t_defined
+#define __sigevent_t_defined
+#define _BITS_SIGINFO_CONSTS_H 1
+#define _BITS_SIGEVENT_CONSTS_H 1
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <linux/perf_event.h>
+#include <pthread.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+static volatile int signal_count;
+
+static struct perf_event_attr make_event_attr(void)
+{
+ struct perf_event_attr attr = {
+ .type = PERF_TYPE_HARDWARE,
+ .size = sizeof(attr),
+ .config = PERF_COUNT_HW_INSTRUCTIONS,
+ .sample_period = 1000,
+ .exclude_kernel = 1,
+ .exclude_hv = 1,
+ .disabled = 1,
+ .inherit = 1,
+ /*
+ * Children normally retain their inherited event on exec; with
+ * remove_on_exec, we'll remove their event, but the parent and
+ * any other non-exec'd children will keep their events.
+ */
+ .remove_on_exec = 1,
+ .sigtrap = 1,
+ };
+ return attr;
+}
+
+static void sigtrap_handler(int signum, siginfo_t *info, void *ucontext)
+{
+ if (info->si_code != TRAP_PERF) {
+ fprintf(stderr, "%s: unexpected si_code %d\n", __func__, info->si_code);
+ return;
+ }
+
+ signal_count++;
+}
+
+FIXTURE(remove_on_exec)
+{
+ struct sigaction oldact;
+ int fd;
+};
+
+FIXTURE_SETUP(remove_on_exec)
+{
+ struct perf_event_attr attr = make_event_attr();
+ struct sigaction action = {};
+
+ signal_count = 0;
+
+ /* Initialize sigtrap handler. */
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ ASSERT_EQ(sigaction(SIGTRAP, &action, &self->oldact), 0);
+
+ /* Initialize perf event. */
+ self->fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC);
+ ASSERT_NE(self->fd, -1);
+}
+
+FIXTURE_TEARDOWN(remove_on_exec)
+{
+ close(self->fd);
+ sigaction(SIGTRAP, &self->oldact, NULL);
+}
+
+/* Verify event propagates to fork'd child. */
+TEST_F(remove_on_exec, fork_only)
+{
+ int status;
+ pid_t pid = fork();
+
+ if (pid == 0) {
+ ASSERT_EQ(signal_count, 0);
+ ASSERT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ while (!signal_count);
+ _exit(42);
+ }
+
+ while (!signal_count); /* Child enables event. */
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(WEXITSTATUS(status), 42);
+}
+
+/*
+ * Verify that event does _not_ propagate to fork+exec'd child; event enabled
+ * after fork+exec.
+ */
+TEST_F(remove_on_exec, fork_exec_then_enable)
+{
+ pid_t pid_exec, pid_only_fork;
+ int pipefd[2];
+ int tmp;
+
+ /*
+ * Non-exec child, to ensure exec does not affect inherited events of
+ * other children.
+ */
+ pid_only_fork = fork();
+ if (pid_only_fork == 0) {
+ /* Block until parent enables event. */
+ while (!signal_count);
+ _exit(42);
+ }
+
+ ASSERT_NE(pipe(pipefd), -1);
+ pid_exec = fork();
+ if (pid_exec == 0) {
+ ASSERT_NE(dup2(pipefd[1], STDOUT_FILENO), -1);
+ close(pipefd[0]);
+ execl("/proc/self/exe", "exec_child", NULL);
+ _exit((perror("exec failed"), 1));
+ }
+ close(pipefd[1]);
+
+ ASSERT_EQ(waitpid(pid_exec, &tmp, WNOHANG), 0); /* Child is running. */
+ /* Wait for exec'd child to start spinning. */
+ EXPECT_EQ(read(pipefd[0], &tmp, sizeof(int)), sizeof(int));
+ EXPECT_EQ(tmp, 42);
+ close(pipefd[0]);
+ /* Now we can enable the event, knowing the child is doing work. */
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ /* If the event propagated to the exec'd child, it will exit normally... */
+ usleep(100000); /* ... give time for event to trigger (in case of bug). */
+ EXPECT_EQ(waitpid(pid_exec, &tmp, WNOHANG), 0); /* Should still be running. */
+ EXPECT_EQ(kill(pid_exec, SIGKILL), 0);
+
+ /* Verify removal from child did not affect this task's event. */
+ tmp = signal_count;
+ while (signal_count == tmp); /* Should not hang! */
+ /* Nor should it have affected the first child. */
+ EXPECT_EQ(waitpid(pid_only_fork, &tmp, 0), pid_only_fork);
+ EXPECT_EQ(WEXITSTATUS(tmp), 42);
+}
+
+/*
+ * Verify that event does _not_ propagate to fork+exec'd child; event enabled
+ * before fork+exec.
+ */
+TEST_F(remove_on_exec, enable_then_fork_exec)
+{
+ pid_t pid_exec;
+ int tmp;
+
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+
+ pid_exec = fork();
+ if (pid_exec == 0) {
+ execl("/proc/self/exe", "exec_child", NULL);
+ _exit((perror("exec failed"), 1));
+ }
+
+ /*
+ * The child may exit abnormally at any time if the event propagated and
+ * a SIGTRAP is sent before the handler was set up.
+ */
+ usleep(100000); /* ... give time for event to trigger (in case of bug). */
+ EXPECT_EQ(waitpid(pid_exec, &tmp, WNOHANG), 0); /* Should still be running. */
+ EXPECT_EQ(kill(pid_exec, SIGKILL), 0);
+
+ /* Verify removal from child did not affect this task's event. */
+ tmp = signal_count;
+ while (signal_count == tmp); /* Should not hang! */
+}
+
+TEST_F(remove_on_exec, exec_stress)
+{
+ pid_t pids[30];
+ int i, tmp;
+
+ for (i = 0; i < sizeof(pids) / sizeof(pids[0]); i++) {
+ pids[i] = fork();
+ if (pids[i] == 0) {
+ execl("/proc/self/exe", "exec_child", NULL);
+ _exit((perror("exec failed"), 1));
+ }
+
+ /* Some forked with event disabled, rest with enabled. */
+ if (i > 10)
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ }
+
+ usleep(100000); /* ... give time for event to trigger (in case of bug). */
+
+ for (i = 0; i < sizeof(pids) / sizeof(pids[0]); i++) {
+ /* All children should still be running. */
+ EXPECT_EQ(waitpid(pids[i], &tmp, WNOHANG), 0);
+ EXPECT_EQ(kill(pids[i], SIGKILL), 0);
+ }
+
+ /* Verify event is still alive. */
+ tmp = signal_count;
+ while (signal_count == tmp);
+}
+
+/* For exec'd child. */
+static void exec_child(void)
+{
+ struct sigaction action = {};
+ const int val = 42;
+
+ /* Set up sigtrap handler in case we erroneously receive a trap. */
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ if (sigaction(SIGTRAP, &action, NULL))
+ _exit((perror("sigaction failed"), 1));
+
+ /* Signal parent that we're starting to spin. */
+ if (write(STDOUT_FILENO, &val, sizeof(int)) == -1)
+ _exit((perror("write failed"), 1));
+
+ /* Should hang here until killed. */
+ while (!signal_count);
+}
+
+#define main test_main
+TEST_HARNESS_MAIN
+#undef main
+int main(int argc, char *argv[])
+{
+ if (!strcmp(argv[0], "exec_child")) {
+ exec_child();
+ return 1;
+ }
+
+ return test_main(argc, argv);
+}
--
2.31.0.208.g409f899ff0-goog
Sync tool's uapi to pick up the changes adding inherit_thread,
remove_on_exec, and sigtrap fields to perf_event_attr.
Signed-off-by: Marco Elver <[email protected]>
---
v4:
* Update for new perf_event_attr::sig_data.
v3:
* Added to series.
---
tools/include/uapi/linux/perf_event.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index ad15e40d7f5d..31b00e3b69c9 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -311,6 +311,7 @@ enum perf_event_read_format {
#define PERF_ATTR_SIZE_VER4 104 /* add: sample_regs_intr */
#define PERF_ATTR_SIZE_VER5 112 /* add: aux_watermark */
#define PERF_ATTR_SIZE_VER6 120 /* add: aux_sample_size */
+#define PERF_ATTR_SIZE_VER7 128 /* add: sig_data */
/*
* Hardware event_id to monitor via a performance monitoring event:
@@ -389,7 +390,10 @@ struct perf_event_attr {
cgroup : 1, /* include cgroup events */
text_poke : 1, /* include text poke events */
build_id : 1, /* use build id in mmap2 events */
- __reserved_1 : 29;
+ inherit_thread : 1, /* children only inherit if cloned with CLONE_THREAD */
+ remove_on_exec : 1, /* event is removed from task on exec */
+ sigtrap : 1, /* send synchronous SIGTRAP on event */
+ __reserved_1 : 26;
union {
__u32 wakeup_events; /* wakeup every n events */
@@ -441,6 +445,12 @@ struct perf_event_attr {
__u16 __reserved_2;
__u32 aux_sample_size;
__u32 __reserved_3;
+
+ /*
+ * User provided data if sigtrap=1, passed back to user via
+ * siginfo_t::si_perf, e.g. to permit user to identify the event.
+ */
+ __u64 sig_data;
};
/*
--
2.31.0.208.g409f899ff0-goog
Add basic stress test for sigtrap handling as a perf tool built-in test.
This allows sanity checking the basic sigtrap functionality from within
the perf tool.
Note: A more elaborate kselftest version of this test can also be found
in tools/testing/selftests/perf_events/sigtrap_threads.c.
Signed-off-by: Marco Elver <[email protected]>
---
v4:
* Update for new perf_event_attr::sig_data / si_perf handling.
v3:
* Added to series (per suggestion from Ian Rogers).
---
tools/perf/tests/Build | 1 +
tools/perf/tests/builtin-test.c | 5 ++
tools/perf/tests/sigtrap.c | 150 ++++++++++++++++++++++++++++++++
tools/perf/tests/tests.h | 1 +
4 files changed, 157 insertions(+)
create mode 100644 tools/perf/tests/sigtrap.c
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 650aec19d490..a429c7a02b37 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -64,6 +64,7 @@ perf-y += parse-metric.o
perf-y += pe-file-parsing.o
perf-y += expand-cgroup.o
perf-y += perf-time-to-tsc.o
+perf-y += sigtrap.o
$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
$(call rule_mkdir)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index c4b888f18e9c..28a1cb5eaa77 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -359,6 +359,11 @@ static struct test generic_tests[] = {
.func = test__perf_time_to_tsc,
.is_supported = test__tsc_is_supported,
},
+ {
+ .desc = "Sigtrap support",
+ .func = test__sigtrap,
+ .is_supported = test__wp_is_supported, /* uses wp for test */
+ },
{
.func = NULL,
},
diff --git a/tools/perf/tests/sigtrap.c b/tools/perf/tests/sigtrap.c
new file mode 100644
index 000000000000..c367cc2f64d5
--- /dev/null
+++ b/tools/perf/tests/sigtrap.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Basic test for sigtrap support.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#include <stdint.h>
+#include <stdlib.h>
+#include <linux/hw_breakpoint.h>
+#include <pthread.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "cloexec.h"
+#include "debug.h"
+#include "event.h"
+#include "tests.h"
+#include "../perf-sys.h"
+
+#define NUM_THREADS 5
+
+static struct {
+ int tids_want_signal; /* Which threads still want a signal. */
+ int signal_count; /* Sanity check number of signals received. */
+ volatile int iterate_on; /* Variable to set breakpoint on. */
+ siginfo_t first_siginfo; /* First observed siginfo_t. */
+} ctx;
+
+#define TEST_SIG_DATA (~(uint64_t)(&ctx.iterate_on))
+
+static struct perf_event_attr make_event_attr(void)
+{
+ struct perf_event_attr attr = {
+ .type = PERF_TYPE_BREAKPOINT,
+ .size = sizeof(attr),
+ .sample_period = 1,
+ .disabled = 1,
+ .bp_addr = (unsigned long)&ctx.iterate_on,
+ .bp_type = HW_BREAKPOINT_RW,
+ .bp_len = HW_BREAKPOINT_LEN_1,
+ .inherit = 1, /* Children inherit events ... */
+ .inherit_thread = 1, /* ... but only cloned with CLONE_THREAD. */
+ .remove_on_exec = 1, /* Required by sigtrap. */
+ .sigtrap = 1, /* Request synchronous SIGTRAP on event. */
+ .sig_data = TEST_SIG_DATA,
+ };
+ return attr;
+}
+
+static void
+sigtrap_handler(int signum __maybe_unused, siginfo_t *info, void *ucontext __maybe_unused)
+{
+ if (!__atomic_fetch_add(&ctx.signal_count, 1, __ATOMIC_RELAXED))
+ ctx.first_siginfo = *info;
+ __atomic_fetch_sub(&ctx.tids_want_signal, syscall(SYS_gettid), __ATOMIC_RELAXED);
+}
+
+static void *test_thread(void *arg)
+{
+ pthread_barrier_t *barrier = (pthread_barrier_t *)arg;
+ pid_t tid = syscall(SYS_gettid);
+ int i;
+
+ pthread_barrier_wait(barrier);
+
+ __atomic_fetch_add(&ctx.tids_want_signal, tid, __ATOMIC_RELAXED);
+ for (i = 0; i < ctx.iterate_on - 1; i++)
+ __atomic_fetch_add(&ctx.tids_want_signal, tid, __ATOMIC_RELAXED);
+
+ return NULL;
+}
+
+static int run_test_threads(pthread_t *threads, pthread_barrier_t *barrier)
+{
+ int i;
+
+ pthread_barrier_wait(barrier);
+ for (i = 0; i < NUM_THREADS; i++)
+ TEST_ASSERT_EQUAL("pthread_join() failed", pthread_join(threads[i], NULL), 0);
+
+ return TEST_OK;
+}
+
+static int run_stress_test(int fd, pthread_t *threads, pthread_barrier_t *barrier)
+{
+ int ret;
+
+ ctx.iterate_on = 3000;
+
+ TEST_ASSERT_EQUAL("misfired signal?", ctx.signal_count, 0);
+ TEST_ASSERT_EQUAL("enable failed", ioctl(fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ ret = run_test_threads(threads, barrier);
+ TEST_ASSERT_EQUAL("disable failed", ioctl(fd, PERF_EVENT_IOC_DISABLE, 0), 0);
+
+ TEST_ASSERT_EQUAL("unexpected sigtraps", ctx.signal_count, NUM_THREADS * ctx.iterate_on);
+ TEST_ASSERT_EQUAL("missing signals or incorrectly delivered", ctx.tids_want_signal, 0);
+ TEST_ASSERT_VAL("unexpected si_addr", ctx.first_siginfo.si_addr == &ctx.iterate_on);
+ TEST_ASSERT_EQUAL("unexpected si_errno", ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+#if 0 /* FIXME: test build and enable when libc's signal.h has si_perf. */
+ TEST_ASSERT_VAL("unexpected si_perf", ctx.first_siginfo.si_perf == TEST_SIG_DATA);
+#endif
+
+ return ret;
+}
+
+int test__sigtrap(struct test *test __maybe_unused, int subtest __maybe_unused)
+{
+ struct perf_event_attr attr = make_event_attr();
+ struct sigaction action = {};
+ struct sigaction oldact;
+ pthread_t threads[NUM_THREADS];
+ pthread_barrier_t barrier;
+ int i, fd, ret = TEST_FAIL;
+
+ pthread_barrier_init(&barrier, NULL, NUM_THREADS + 1);
+
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ if (sigaction(SIGTRAP, &action, &oldact)) {
+ pr_debug("FAILED sigaction()\n");
+ goto out;
+ }
+
+ fd = sys_perf_event_open(&attr, 0, -1, -1, perf_event_open_cloexec_flag());
+ if (fd < 0) {
+ pr_debug("FAILED sys_perf_event_open()\n");
+ goto out_restore_sigaction;
+ }
+
+ for (i = 0; i < NUM_THREADS; i++) {
+ if (pthread_create(&threads[i], NULL, test_thread, &barrier)) {
+ pr_debug("FAILED pthread_create()");
+ goto out_close_perf_event;
+ }
+ }
+
+ ret = run_stress_test(fd, threads, &barrier);
+
+out_close_perf_event:
+ close(fd);
+out_restore_sigaction:
+ sigaction(SIGTRAP, &oldact, NULL);
+out:
+ pthread_barrier_destroy(&barrier);
+ return ret;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index b85f005308a3..c3f2e2ecbfd6 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -127,6 +127,7 @@ int test__parse_metric(struct test *test, int subtest);
int test__pe_file_parsing(struct test *test, int subtest);
int test__expand_cgroup_events(struct test *test, int subtest);
int test__perf_time_to_tsc(struct test *test, int subtest);
+int test__sigtrap(struct test *test, int subtest);
bool test__bp_signal_is_supported(void);
bool test__bp_account_is_supported(void);
--
2.31.0.208.g409f899ff0-goog
On Thu, Apr 08, 2021 at 12:35:55PM +0200, Marco Elver wrote:
> Marco Elver (9):
> perf: Apply PERF_EVENT_IOC_MODIFY_ATTRIBUTES to children
> perf: Support only inheriting events if cloned with CLONE_THREAD
> perf: Add support for event removal on exec
> signal: Introduce TRAP_PERF si_code and si_perf to siginfo
> perf: Add support for SIGTRAP on perf events
> selftests/perf_events: Add kselftest for process-wide sigtrap handling
> selftests/perf_events: Add kselftest for remove_on_exec
Thanks!, I've picked up the above 8 patches. Arnaldo, do you want to
carry the last 2 patches or are you fine with me taking them as well?
> tools headers uapi: Sync tools/include/uapi/linux/perf_event.h
> perf test: Add basic stress test for sigtrap handling
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 6216798bf98e82c382922f1b71ecc4a13d6e65cb
Gitweb: https://git.kernel.org/tip/6216798bf98e82c382922f1b71ecc4a13d6e65cb
Author: Marco Elver <[email protected]>
AuthorDate: Thu, 08 Apr 2021 12:36:03 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 16 Apr 2021 16:32:42 +02:00
selftests/perf_events: Add kselftest for remove_on_exec
Add kselftest to test that remove_on_exec removes inherited events from
child tasks.
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
tools/testing/selftests/perf_events/.gitignore | 1 +-
tools/testing/selftests/perf_events/Makefile | 2 +-
tools/testing/selftests/perf_events/remove_on_exec.c | 260 ++++++++++-
3 files changed, 262 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/perf_events/remove_on_exec.c
diff --git a/tools/testing/selftests/perf_events/.gitignore b/tools/testing/selftests/perf_events/.gitignore
index 4dc43e1..790c470 100644
--- a/tools/testing/selftests/perf_events/.gitignore
+++ b/tools/testing/selftests/perf_events/.gitignore
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
sigtrap_threads
+remove_on_exec
diff --git a/tools/testing/selftests/perf_events/Makefile b/tools/testing/selftests/perf_events/Makefile
index 973a2c3..fcafa5f 100644
--- a/tools/testing/selftests/perf_events/Makefile
+++ b/tools/testing/selftests/perf_events/Makefile
@@ -2,5 +2,5 @@
CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include
LDFLAGS += -lpthread
-TEST_GEN_PROGS := sigtrap_threads
+TEST_GEN_PROGS := sigtrap_threads remove_on_exec
include ../lib.mk
diff --git a/tools/testing/selftests/perf_events/remove_on_exec.c b/tools/testing/selftests/perf_events/remove_on_exec.c
new file mode 100644
index 0000000..5814611
--- /dev/null
+++ b/tools/testing/selftests/perf_events/remove_on_exec.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test for remove_on_exec.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+/* We need the latest siginfo from the kernel repo. */
+#include <sys/types.h>
+#include <asm/siginfo.h>
+#define __have_siginfo_t 1
+#define __have_sigval_t 1
+#define __have_sigevent_t 1
+#define __siginfo_t_defined
+#define __sigval_t_defined
+#define __sigevent_t_defined
+#define _BITS_SIGINFO_CONSTS_H 1
+#define _BITS_SIGEVENT_CONSTS_H 1
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <linux/perf_event.h>
+#include <pthread.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+static volatile int signal_count;
+
+static struct perf_event_attr make_event_attr(void)
+{
+ struct perf_event_attr attr = {
+ .type = PERF_TYPE_HARDWARE,
+ .size = sizeof(attr),
+ .config = PERF_COUNT_HW_INSTRUCTIONS,
+ .sample_period = 1000,
+ .exclude_kernel = 1,
+ .exclude_hv = 1,
+ .disabled = 1,
+ .inherit = 1,
+ /*
+ * Children normally retain their inherited event on exec; with
+ * remove_on_exec, we'll remove their event, but the parent and
+ * any other non-exec'd children will keep their events.
+ */
+ .remove_on_exec = 1,
+ .sigtrap = 1,
+ };
+ return attr;
+}
+
+static void sigtrap_handler(int signum, siginfo_t *info, void *ucontext)
+{
+ if (info->si_code != TRAP_PERF) {
+ fprintf(stderr, "%s: unexpected si_code %d\n", __func__, info->si_code);
+ return;
+ }
+
+ signal_count++;
+}
+
+FIXTURE(remove_on_exec)
+{
+ struct sigaction oldact;
+ int fd;
+};
+
+FIXTURE_SETUP(remove_on_exec)
+{
+ struct perf_event_attr attr = make_event_attr();
+ struct sigaction action = {};
+
+ signal_count = 0;
+
+ /* Initialize sigtrap handler. */
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ ASSERT_EQ(sigaction(SIGTRAP, &action, &self->oldact), 0);
+
+ /* Initialize perf event. */
+ self->fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC);
+ ASSERT_NE(self->fd, -1);
+}
+
+FIXTURE_TEARDOWN(remove_on_exec)
+{
+ close(self->fd);
+ sigaction(SIGTRAP, &self->oldact, NULL);
+}
+
+/* Verify event propagates to fork'd child. */
+TEST_F(remove_on_exec, fork_only)
+{
+ int status;
+ pid_t pid = fork();
+
+ if (pid == 0) {
+ ASSERT_EQ(signal_count, 0);
+ ASSERT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ while (!signal_count);
+ _exit(42);
+ }
+
+ while (!signal_count); /* Child enables event. */
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(WEXITSTATUS(status), 42);
+}
+
+/*
+ * Verify that event does _not_ propagate to fork+exec'd child; event enabled
+ * after fork+exec.
+ */
+TEST_F(remove_on_exec, fork_exec_then_enable)
+{
+ pid_t pid_exec, pid_only_fork;
+ int pipefd[2];
+ int tmp;
+
+ /*
+ * Non-exec child, to ensure exec does not affect inherited events of
+ * other children.
+ */
+ pid_only_fork = fork();
+ if (pid_only_fork == 0) {
+ /* Block until parent enables event. */
+ while (!signal_count);
+ _exit(42);
+ }
+
+ ASSERT_NE(pipe(pipefd), -1);
+ pid_exec = fork();
+ if (pid_exec == 0) {
+ ASSERT_NE(dup2(pipefd[1], STDOUT_FILENO), -1);
+ close(pipefd[0]);
+ execl("/proc/self/exe", "exec_child", NULL);
+ _exit((perror("exec failed"), 1));
+ }
+ close(pipefd[1]);
+
+ ASSERT_EQ(waitpid(pid_exec, &tmp, WNOHANG), 0); /* Child is running. */
+ /* Wait for exec'd child to start spinning. */
+ EXPECT_EQ(read(pipefd[0], &tmp, sizeof(int)), sizeof(int));
+ EXPECT_EQ(tmp, 42);
+ close(pipefd[0]);
+ /* Now we can enable the event, knowing the child is doing work. */
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ /* If the event propagated to the exec'd child, it will exit normally... */
+ usleep(100000); /* ... give time for event to trigger (in case of bug). */
+ EXPECT_EQ(waitpid(pid_exec, &tmp, WNOHANG), 0); /* Should still be running. */
+ EXPECT_EQ(kill(pid_exec, SIGKILL), 0);
+
+ /* Verify removal from child did not affect this task's event. */
+ tmp = signal_count;
+ while (signal_count == tmp); /* Should not hang! */
+ /* Nor should it have affected the first child. */
+ EXPECT_EQ(waitpid(pid_only_fork, &tmp, 0), pid_only_fork);
+ EXPECT_EQ(WEXITSTATUS(tmp), 42);
+}
+
+/*
+ * Verify that event does _not_ propagate to fork+exec'd child; event enabled
+ * before fork+exec.
+ */
+TEST_F(remove_on_exec, enable_then_fork_exec)
+{
+ pid_t pid_exec;
+ int tmp;
+
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+
+ pid_exec = fork();
+ if (pid_exec == 0) {
+ execl("/proc/self/exe", "exec_child", NULL);
+ _exit((perror("exec failed"), 1));
+ }
+
+ /*
+ * The child may exit abnormally at any time if the event propagated and
+ * a SIGTRAP is sent before the handler was set up.
+ */
+ usleep(100000); /* ... give time for event to trigger (in case of bug). */
+ EXPECT_EQ(waitpid(pid_exec, &tmp, WNOHANG), 0); /* Should still be running. */
+ EXPECT_EQ(kill(pid_exec, SIGKILL), 0);
+
+ /* Verify removal from child did not affect this task's event. */
+ tmp = signal_count;
+ while (signal_count == tmp); /* Should not hang! */
+}
+
+TEST_F(remove_on_exec, exec_stress)
+{
+ pid_t pids[30];
+ int i, tmp;
+
+ for (i = 0; i < sizeof(pids) / sizeof(pids[0]); i++) {
+ pids[i] = fork();
+ if (pids[i] == 0) {
+ execl("/proc/self/exe", "exec_child", NULL);
+ _exit((perror("exec failed"), 1));
+ }
+
+ /* Some forked with event disabled, rest with enabled. */
+ if (i > 10)
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ }
+
+ usleep(100000); /* ... give time for event to trigger (in case of bug). */
+
+ for (i = 0; i < sizeof(pids) / sizeof(pids[0]); i++) {
+ /* All children should still be running. */
+ EXPECT_EQ(waitpid(pids[i], &tmp, WNOHANG), 0);
+ EXPECT_EQ(kill(pids[i], SIGKILL), 0);
+ }
+
+ /* Verify event is still alive. */
+ tmp = signal_count;
+ while (signal_count == tmp);
+}
+
+/* For exec'd child. */
+static void exec_child(void)
+{
+ struct sigaction action = {};
+ const int val = 42;
+
+ /* Set up sigtrap handler in case we erroneously receive a trap. */
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ if (sigaction(SIGTRAP, &action, NULL))
+ _exit((perror("sigaction failed"), 1));
+
+ /* Signal parent that we're starting to spin. */
+ if (write(STDOUT_FILENO, &val, sizeof(int)) == -1)
+ _exit((perror("write failed"), 1));
+
+ /* Should hang here until killed. */
+ while (!signal_count);
+}
+
+#define main test_main
+TEST_HARNESS_MAIN
+#undef main
+int main(int argc, char *argv[])
+{
+ if (!strcmp(argv[0], "exec_child")) {
+ exec_child();
+ return 1;
+ }
+
+ return test_main(argc, argv);
+}
The following commit has been merged into the perf/core branch of tip:
Commit-ID: f2c3c32f45002de19c6dec33f32fd259e82f2557
Gitweb: https://git.kernel.org/tip/f2c3c32f45002de19c6dec33f32fd259e82f2557
Author: Marco Elver <[email protected]>
AuthorDate: Thu, 08 Apr 2021 12:36:02 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 16 Apr 2021 16:32:42 +02:00
selftests/perf_events: Add kselftest for process-wide sigtrap handling
Add a kselftest for testing process-wide perf events with synchronous
SIGTRAP on events (using breakpoints). In particular, we want to test
that changes to the event propagate to all children, and the SIGTRAPs
are in fact synchronously sent to the thread where the event occurred.
Note: The "signal_stress" test case is also added later in the series to
perf tool's built-in tests. The test here is more elaborate in that
respect, which on one hand avoids bloating the perf tool unnecessarily,
but we also benefit from structured tests with TAP-compliant output that
the kselftest framework provides.
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
tools/testing/selftests/perf_events/.gitignore | 2 +-
tools/testing/selftests/perf_events/Makefile | 6 +-
tools/testing/selftests/perf_events/config | 1 +-
tools/testing/selftests/perf_events/settings | 1 +-
tools/testing/selftests/perf_events/sigtrap_threads.c | 210 +++++++++-
5 files changed, 220 insertions(+)
create mode 100644 tools/testing/selftests/perf_events/.gitignore
create mode 100644 tools/testing/selftests/perf_events/Makefile
create mode 100644 tools/testing/selftests/perf_events/config
create mode 100644 tools/testing/selftests/perf_events/settings
create mode 100644 tools/testing/selftests/perf_events/sigtrap_threads.c
diff --git a/tools/testing/selftests/perf_events/.gitignore b/tools/testing/selftests/perf_events/.gitignore
new file mode 100644
index 0000000..4dc43e1
--- /dev/null
+++ b/tools/testing/selftests/perf_events/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+sigtrap_threads
diff --git a/tools/testing/selftests/perf_events/Makefile b/tools/testing/selftests/perf_events/Makefile
new file mode 100644
index 0000000..973a2c3
--- /dev/null
+++ b/tools/testing/selftests/perf_events/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -Wl,-no-as-needed -Wall -I../../../../usr/include
+LDFLAGS += -lpthread
+
+TEST_GEN_PROGS := sigtrap_threads
+include ../lib.mk
diff --git a/tools/testing/selftests/perf_events/config b/tools/testing/selftests/perf_events/config
new file mode 100644
index 0000000..ba58ff2
--- /dev/null
+++ b/tools/testing/selftests/perf_events/config
@@ -0,0 +1 @@
+CONFIG_PERF_EVENTS=y
diff --git a/tools/testing/selftests/perf_events/settings b/tools/testing/selftests/perf_events/settings
new file mode 100644
index 0000000..6091b45
--- /dev/null
+++ b/tools/testing/selftests/perf_events/settings
@@ -0,0 +1 @@
+timeout=120
diff --git a/tools/testing/selftests/perf_events/sigtrap_threads.c b/tools/testing/selftests/perf_events/sigtrap_threads.c
new file mode 100644
index 0000000..9c0fd44
--- /dev/null
+++ b/tools/testing/selftests/perf_events/sigtrap_threads.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test for perf events with SIGTRAP across all threads.
+ *
+ * Copyright (C) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+/* We need the latest siginfo from the kernel repo. */
+#include <sys/types.h>
+#include <asm/siginfo.h>
+#define __have_siginfo_t 1
+#define __have_sigval_t 1
+#define __have_sigevent_t 1
+#define __siginfo_t_defined
+#define __sigval_t_defined
+#define __sigevent_t_defined
+#define _BITS_SIGINFO_CONSTS_H 1
+#define _BITS_SIGEVENT_CONSTS_H 1
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/perf_event.h>
+#include <pthread.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+#define NUM_THREADS 5
+
+/* Data shared between test body, threads, and signal handler. */
+static struct {
+ int tids_want_signal; /* Which threads still want a signal. */
+ int signal_count; /* Sanity check number of signals received. */
+ volatile int iterate_on; /* Variable to set breakpoint on. */
+ siginfo_t first_siginfo; /* First observed siginfo_t. */
+} ctx;
+
+/* Unique value to check si_perf is correctly set from perf_event_attr::sig_data. */
+#define TEST_SIG_DATA(addr) (~(uint64_t)(addr))
+
+static struct perf_event_attr make_event_attr(bool enabled, volatile void *addr)
+{
+ struct perf_event_attr attr = {
+ .type = PERF_TYPE_BREAKPOINT,
+ .size = sizeof(attr),
+ .sample_period = 1,
+ .disabled = !enabled,
+ .bp_addr = (unsigned long)addr,
+ .bp_type = HW_BREAKPOINT_RW,
+ .bp_len = HW_BREAKPOINT_LEN_1,
+ .inherit = 1, /* Children inherit events ... */
+ .inherit_thread = 1, /* ... but only cloned with CLONE_THREAD. */
+ .remove_on_exec = 1, /* Required by sigtrap. */
+ .sigtrap = 1, /* Request synchronous SIGTRAP on event. */
+ .sig_data = TEST_SIG_DATA(addr),
+ };
+ return attr;
+}
+
+static void sigtrap_handler(int signum, siginfo_t *info, void *ucontext)
+{
+ if (info->si_code != TRAP_PERF) {
+ fprintf(stderr, "%s: unexpected si_code %d\n", __func__, info->si_code);
+ return;
+ }
+
+ /*
+ * The data in siginfo_t we're interested in should all be the same
+ * across threads.
+ */
+ if (!__atomic_fetch_add(&ctx.signal_count, 1, __ATOMIC_RELAXED))
+ ctx.first_siginfo = *info;
+ __atomic_fetch_sub(&ctx.tids_want_signal, syscall(__NR_gettid), __ATOMIC_RELAXED);
+}
+
+static void *test_thread(void *arg)
+{
+ pthread_barrier_t *barrier = (pthread_barrier_t *)arg;
+ pid_t tid = syscall(__NR_gettid);
+ int iter;
+ int i;
+
+ pthread_barrier_wait(barrier);
+
+ __atomic_fetch_add(&ctx.tids_want_signal, tid, __ATOMIC_RELAXED);
+ iter = ctx.iterate_on; /* read */
+ for (i = 0; i < iter - 1; i++) {
+ __atomic_fetch_add(&ctx.tids_want_signal, tid, __ATOMIC_RELAXED);
+ ctx.iterate_on = iter; /* idempotent write */
+ }
+
+ return NULL;
+}
+
+FIXTURE(sigtrap_threads)
+{
+ struct sigaction oldact;
+ pthread_t threads[NUM_THREADS];
+ pthread_barrier_t barrier;
+ int fd;
+};
+
+FIXTURE_SETUP(sigtrap_threads)
+{
+ struct perf_event_attr attr = make_event_attr(false, &ctx.iterate_on);
+ struct sigaction action = {};
+ int i;
+
+ memset(&ctx, 0, sizeof(ctx));
+
+ /* Initialize sigtrap handler. */
+ action.sa_flags = SA_SIGINFO | SA_NODEFER;
+ action.sa_sigaction = sigtrap_handler;
+ sigemptyset(&action.sa_mask);
+ ASSERT_EQ(sigaction(SIGTRAP, &action, &self->oldact), 0);
+
+ /* Initialize perf event. */
+ self->fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC);
+ ASSERT_NE(self->fd, -1);
+
+ /* Spawn threads inheriting perf event. */
+ pthread_barrier_init(&self->barrier, NULL, NUM_THREADS + 1);
+ for (i = 0; i < NUM_THREADS; i++)
+ ASSERT_EQ(pthread_create(&self->threads[i], NULL, test_thread, &self->barrier), 0);
+}
+
+FIXTURE_TEARDOWN(sigtrap_threads)
+{
+ pthread_barrier_destroy(&self->barrier);
+ close(self->fd);
+ sigaction(SIGTRAP, &self->oldact, NULL);
+}
+
+static void run_test_threads(struct __test_metadata *_metadata,
+ FIXTURE_DATA(sigtrap_threads) *self)
+{
+ int i;
+
+ pthread_barrier_wait(&self->barrier);
+ for (i = 0; i < NUM_THREADS; i++)
+ ASSERT_EQ(pthread_join(self->threads[i], NULL), 0);
+}
+
+TEST_F(sigtrap_threads, remain_disabled)
+{
+ run_test_threads(_metadata, self);
+ EXPECT_EQ(ctx.signal_count, 0);
+ EXPECT_NE(ctx.tids_want_signal, 0);
+}
+
+TEST_F(sigtrap_threads, enable_event)
+{
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ run_test_threads(_metadata, self);
+
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS);
+ EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
+ EXPECT_EQ(ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+ EXPECT_EQ(ctx.first_siginfo.si_perf, TEST_SIG_DATA(&ctx.iterate_on));
+
+ /* Check enabled for parent. */
+ ctx.iterate_on = 0;
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1);
+}
+
+/* Test that modification propagates to all inherited events. */
+TEST_F(sigtrap_threads, modify_and_enable_event)
+{
+ struct perf_event_attr new_attr = make_event_attr(true, &ctx.iterate_on);
+
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &new_attr), 0);
+ run_test_threads(_metadata, self);
+
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS);
+ EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
+ EXPECT_EQ(ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+ EXPECT_EQ(ctx.first_siginfo.si_perf, TEST_SIG_DATA(&ctx.iterate_on));
+
+ /* Check enabled for parent. */
+ ctx.iterate_on = 0;
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1);
+}
+
+/* Stress test event + signal handling. */
+TEST_F(sigtrap_threads, signal_stress)
+{
+ ctx.iterate_on = 3000;
+
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
+ run_test_threads(_metadata, self);
+ EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_DISABLE, 0), 0);
+
+ EXPECT_EQ(ctx.signal_count, NUM_THREADS * ctx.iterate_on);
+ EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
+ EXPECT_EQ(ctx.first_siginfo.si_errno, PERF_TYPE_BREAKPOINT);
+ EXPECT_EQ(ctx.first_siginfo.si_perf, TEST_SIG_DATA(&ctx.iterate_on));
+}
+
+TEST_HARNESS_MAIN
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 2e498d0a74e5b88a6689ae1b811f247f91ff188e
Gitweb: https://git.kernel.org/tip/2e498d0a74e5b88a6689ae1b811f247f91ff188e
Author: Marco Elver <[email protected]>
AuthorDate: Thu, 08 Apr 2021 12:35:59 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 16 Apr 2021 16:32:41 +02:00
perf: Add support for event removal on exec
Adds bit perf_event_attr::remove_on_exec, to support removing an event
from a task on exec.
This option supports the case where an event is supposed to be
process-wide only, and should not propagate beyond exec, to limit
monitoring to the original process image only.
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
include/uapi/linux/perf_event.h | 3 +-
kernel/events/core.c | 70 ++++++++++++++++++++++++++++----
2 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 813efb6..8c5b9f5 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -390,7 +390,8 @@ struct perf_event_attr {
text_poke : 1, /* include text poke events */
build_id : 1, /* use build id in mmap2 events */
inherit_thread : 1, /* children only inherit if cloned with CLONE_THREAD */
- __reserved_1 : 28;
+ remove_on_exec : 1, /* event is removed from task on exec */
+ __reserved_1 : 27;
union {
__u32 wakeup_events; /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3e3c00f..e4a584b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4248,6 +4248,57 @@ out:
put_ctx(clone_ctx);
}
+static void perf_remove_from_owner(struct perf_event *event);
+static void perf_event_exit_event(struct perf_event *event,
+ struct perf_event_context *ctx);
+
+/*
+ * Removes all events from the current task that have been marked
+ * remove-on-exec, and feeds their values back to parent events.
+ */
+static void perf_event_remove_on_exec(int ctxn)
+{
+ struct perf_event_context *ctx, *clone_ctx = NULL;
+ struct perf_event *event, *next;
+ LIST_HEAD(free_list);
+ unsigned long flags;
+ bool modified = false;
+
+ ctx = perf_pin_task_context(current, ctxn);
+ if (!ctx)
+ return;
+
+ mutex_lock(&ctx->mutex);
+
+ if (WARN_ON_ONCE(ctx->task != current))
+ goto unlock;
+
+ list_for_each_entry_safe(event, next, &ctx->event_list, event_entry) {
+ if (!event->attr.remove_on_exec)
+ continue;
+
+ if (!is_kernel_event(event))
+ perf_remove_from_owner(event);
+
+ modified = true;
+
+ perf_event_exit_event(event, ctx);
+ }
+
+ raw_spin_lock_irqsave(&ctx->lock, flags);
+ if (modified)
+ clone_ctx = unclone_ctx(ctx);
+ --ctx->pin_count;
+ raw_spin_unlock_irqrestore(&ctx->lock, flags);
+
+unlock:
+ mutex_unlock(&ctx->mutex);
+
+ put_ctx(ctx);
+ if (clone_ctx)
+ put_ctx(clone_ctx);
+}
+
struct perf_read_data {
struct perf_event *event;
bool group;
@@ -7560,18 +7611,18 @@ void perf_event_exec(void)
struct perf_event_context *ctx;
int ctxn;
- rcu_read_lock();
for_each_task_context_nr(ctxn) {
- ctx = current->perf_event_ctxp[ctxn];
- if (!ctx)
- continue;
-
perf_event_enable_on_exec(ctxn);
+ perf_event_remove_on_exec(ctxn);
- perf_iterate_ctx(ctx, perf_event_addr_filters_exec, NULL,
- true);
+ rcu_read_lock();
+ ctx = rcu_dereference(current->perf_event_ctxp[ctxn]);
+ if (ctx) {
+ perf_iterate_ctx(ctx, perf_event_addr_filters_exec,
+ NULL, true);
+ }
+ rcu_read_unlock();
}
- rcu_read_unlock();
}
struct remote_output {
@@ -11656,6 +11707,9 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
if (!attr->inherit && attr->inherit_thread)
return -EINVAL;
+ if (attr->remove_on_exec && attr->enable_on_exec)
+ return -EINVAL;
+
out:
return ret;
The following commit has been merged into the perf/core branch of tip:
Commit-ID: fb6cc127e0b6e629252cdd0f77d5a1f49db95b92
Gitweb: https://git.kernel.org/tip/fb6cc127e0b6e629252cdd0f77d5a1f49db95b92
Author: Marco Elver <[email protected]>
AuthorDate: Thu, 08 Apr 2021 12:36:00 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 16 Apr 2021 16:32:41 +02:00
signal: Introduce TRAP_PERF si_code and si_perf to siginfo
Introduces the TRAP_PERF si_code, and associated siginfo_t field
si_perf. These will be used by the perf event subsystem to send signals
(if requested) to the task where an event occurred.
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Geert Uytterhoeven <[email protected]> # m68k
Acked-by: Arnd Bergmann <[email protected]> # asm-generic
Link: https://lkml.kernel.org/r/[email protected]
---
arch/m68k/kernel/signal.c | 3 +++
arch/x86/kernel/signal_compat.c | 5 ++++-
fs/signalfd.c | 4 ++++
include/linux/compat.h | 2 ++
include/linux/signal.h | 1 +
include/uapi/asm-generic/siginfo.h | 6 +++++-
include/uapi/linux/signalfd.h | 4 +++-
kernel/signal.c | 11 +++++++++++
8 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
index 349570f..a4b7ee1 100644
--- a/arch/m68k/kernel/signal.c
+++ b/arch/m68k/kernel/signal.c
@@ -622,6 +622,9 @@ static inline void siginfo_build_tests(void)
/* _sigfault._addr_pkey */
BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x12);
+ /* _sigfault._perf */
+ BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x10);
+
/* _sigpoll */
BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c);
BUILD_BUG_ON(offsetof(siginfo_t, si_fd) != 0x10);
diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
index a5330ff..0e5d0a7 100644
--- a/arch/x86/kernel/signal_compat.c
+++ b/arch/x86/kernel/signal_compat.c
@@ -29,7 +29,7 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(NSIGFPE != 15);
BUILD_BUG_ON(NSIGSEGV != 9);
BUILD_BUG_ON(NSIGBUS != 5);
- BUILD_BUG_ON(NSIGTRAP != 5);
+ BUILD_BUG_ON(NSIGTRAP != 6);
BUILD_BUG_ON(NSIGCHLD != 6);
BUILD_BUG_ON(NSIGSYS != 2);
@@ -138,6 +138,9 @@ static inline void signal_compat_build_tests(void)
BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x20);
BUILD_BUG_ON(offsetof(compat_siginfo_t, si_pkey) != 0x14);
+ BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x18);
+ BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf) != 0x10);
+
CHECK_CSI_OFFSET(_sigpoll);
CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int));
CHECK_SI_SIZE (_sigpoll, 4*sizeof(int));
diff --git a/fs/signalfd.c b/fs/signalfd.c
index 456046e..040a114 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -134,6 +134,10 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
#endif
new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
break;
+ case SIL_PERF_EVENT:
+ new.ssi_addr = (long) kinfo->si_addr;
+ new.ssi_perf = kinfo->si_perf;
+ break;
case SIL_CHLD:
new.ssi_pid = kinfo->si_pid;
new.ssi_uid = kinfo->si_uid;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 6e65be7..c8821d9 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -236,6 +236,8 @@ typedef struct compat_siginfo {
char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD];
u32 _pkey;
} _addr_pkey;
+ /* used when si_code=TRAP_PERF */
+ compat_u64 _perf;
};
} _sigfault;
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 205526c..1e98548 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -43,6 +43,7 @@ enum siginfo_layout {
SIL_FAULT_MCEERR,
SIL_FAULT_BNDERR,
SIL_FAULT_PKUERR,
+ SIL_PERF_EVENT,
SIL_CHLD,
SIL_RT,
SIL_SYS,
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index d259700..d0bb912 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -91,6 +91,8 @@ union __sifields {
char _dummy_pkey[__ADDR_BND_PKEY_PAD];
__u32 _pkey;
} _addr_pkey;
+ /* used when si_code=TRAP_PERF */
+ __u64 _perf;
};
} _sigfault;
@@ -155,6 +157,7 @@ typedef struct siginfo {
#define si_lower _sifields._sigfault._addr_bnd._lower
#define si_upper _sifields._sigfault._addr_bnd._upper
#define si_pkey _sifields._sigfault._addr_pkey._pkey
+#define si_perf _sifields._sigfault._perf
#define si_band _sifields._sigpoll._band
#define si_fd _sifields._sigpoll._fd
#define si_call_addr _sifields._sigsys._call_addr
@@ -253,7 +256,8 @@ typedef struct siginfo {
#define TRAP_BRANCH 3 /* process taken branch trap */
#define TRAP_HWBKPT 4 /* hardware breakpoint/watchpoint */
#define TRAP_UNK 5 /* undiagnosed trap */
-#define NSIGTRAP 5
+#define TRAP_PERF 6 /* perf event with sigtrap=1 */
+#define NSIGTRAP 6
/*
* There is an additional set of SIGTRAP si_codes used by ptrace
diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h
index 83429a0..7e33304 100644
--- a/include/uapi/linux/signalfd.h
+++ b/include/uapi/linux/signalfd.h
@@ -39,6 +39,8 @@ struct signalfd_siginfo {
__s32 ssi_syscall;
__u64 ssi_call_addr;
__u32 ssi_arch;
+ __u32 __pad3;
+ __u64 ssi_perf;
/*
* Pad strcture to 128 bytes. Remember to update the
@@ -49,7 +51,7 @@ struct signalfd_siginfo {
* comes out of a read(2) and we really don't want to have
* a compat on read(2).
*/
- __u8 __pad[28];
+ __u8 __pad[16];
};
diff --git a/kernel/signal.c b/kernel/signal.c
index ba4d1ef..f683518 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1199,6 +1199,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
+ case SIL_PERF_EVENT:
case SIL_SYS:
ret = false;
break;
@@ -2531,6 +2532,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
case SIL_FAULT_MCEERR:
case SIL_FAULT_BNDERR:
case SIL_FAULT_PKUERR:
+ case SIL_PERF_EVENT:
ksig->info.si_addr = arch_untagged_si_addr(
ksig->info.si_addr, ksig->sig, ksig->info.si_code);
break;
@@ -3333,6 +3335,10 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
#endif
to->si_pkey = from->si_pkey;
break;
+ case SIL_PERF_EVENT:
+ to->si_addr = ptr_to_compat(from->si_addr);
+ to->si_perf = from->si_perf;
+ break;
case SIL_CHLD:
to->si_pid = from->si_pid;
to->si_uid = from->si_uid;
@@ -3413,6 +3419,10 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
#endif
to->si_pkey = from->si_pkey;
break;
+ case SIL_PERF_EVENT:
+ to->si_addr = compat_ptr(from->si_addr);
+ to->si_perf = from->si_perf;
+ break;
case SIL_CHLD:
to->si_pid = from->si_pid;
to->si_uid = from->si_uid;
@@ -4593,6 +4603,7 @@ static inline void siginfo_buildtime_checks(void)
CHECK_OFFSET(si_lower);
CHECK_OFFSET(si_upper);
CHECK_OFFSET(si_pkey);
+ CHECK_OFFSET(si_perf);
/* sigpoll */
CHECK_OFFSET(si_band);
The following commit has been merged into the perf/core branch of tip:
Commit-ID: 47f661eca0700928012e11c57ea0328f5ccfc3b9
Gitweb: https://git.kernel.org/tip/47f661eca0700928012e11c57ea0328f5ccfc3b9
Author: Marco Elver <[email protected]>
AuthorDate: Thu, 08 Apr 2021 12:35:57 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 16 Apr 2021 16:32:40 +02:00
perf: Apply PERF_EVENT_IOC_MODIFY_ATTRIBUTES to children
As with other ioctls (such as PERF_EVENT_IOC_{ENABLE,DISABLE}), fix up
handling of PERF_EVENT_IOC_MODIFY_ATTRIBUTES to also apply to children.
Suggested-by: Dmitry Vyukov <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Dmitry Vyukov <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/events/core.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 318ff7b..10ed2cd 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3200,16 +3200,36 @@ static int perf_event_modify_breakpoint(struct perf_event *bp,
static int perf_event_modify_attr(struct perf_event *event,
struct perf_event_attr *attr)
{
+ int (*func)(struct perf_event *, struct perf_event_attr *);
+ struct perf_event *child;
+ int err;
+
if (event->attr.type != attr->type)
return -EINVAL;
switch (event->attr.type) {
case PERF_TYPE_BREAKPOINT:
- return perf_event_modify_breakpoint(event, attr);
+ func = perf_event_modify_breakpoint;
+ break;
default:
/* Place holder for future additions. */
return -EOPNOTSUPP;
}
+
+ WARN_ON_ONCE(event->ctx->parent_ctx);
+
+ mutex_lock(&event->child_mutex);
+ err = func(event, attr);
+ if (err)
+ goto out;
+ list_for_each_entry(child, &event->child_list, child_list) {
+ err = func(child, attr);
+ if (err)
+ goto out;
+ }
+out:
+ mutex_unlock(&event->child_mutex);
+ return err;
}
static void ctx_sched_out(struct perf_event_context *ctx,
Hi Marco,
On 08.04.2021 12:36, Marco Elver wrote:
> Introduces the TRAP_PERF si_code, and associated siginfo_t field
> si_perf. These will be used by the perf event subsystem to send signals
> (if requested) to the task where an event occurred.
>
> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
> Signed-off-by: Marco Elver <[email protected]>
This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
regression on my test systems (arm 32bit and 64bit). Most systems fails
to boot in the given time frame. I've observed that there is a timeout
waiting for udev to populate /dev and then also during the network
interfaces configuration. Reverting this commit, together with
97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to let it
compile, on top of next-20210420 fixes the issue.
> ---
> arch/m68k/kernel/signal.c | 3 +++
> arch/x86/kernel/signal_compat.c | 5 ++++-
> fs/signalfd.c | 4 ++++
> include/linux/compat.h | 2 ++
> include/linux/signal.h | 1 +
> include/uapi/asm-generic/siginfo.h | 6 +++++-
> include/uapi/linux/signalfd.h | 4 +++-
> kernel/signal.c | 11 +++++++++++
> 8 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index 349570f16a78..a4b7ee1df211 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -622,6 +622,9 @@ static inline void siginfo_build_tests(void)
> /* _sigfault._addr_pkey */
> BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x12);
>
> + /* _sigfault._perf */
> + BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x10);
> +
> /* _sigpoll */
> BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c);
> BUILD_BUG_ON(offsetof(siginfo_t, si_fd) != 0x10);
> diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
> index a5330ff498f0..0e5d0a7e203b 100644
> --- a/arch/x86/kernel/signal_compat.c
> +++ b/arch/x86/kernel/signal_compat.c
> @@ -29,7 +29,7 @@ static inline void signal_compat_build_tests(void)
> BUILD_BUG_ON(NSIGFPE != 15);
> BUILD_BUG_ON(NSIGSEGV != 9);
> BUILD_BUG_ON(NSIGBUS != 5);
> - BUILD_BUG_ON(NSIGTRAP != 5);
> + BUILD_BUG_ON(NSIGTRAP != 6);
> BUILD_BUG_ON(NSIGCHLD != 6);
> BUILD_BUG_ON(NSIGSYS != 2);
>
> @@ -138,6 +138,9 @@ static inline void signal_compat_build_tests(void)
> BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x20);
> BUILD_BUG_ON(offsetof(compat_siginfo_t, si_pkey) != 0x14);
>
> + BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x18);
> + BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf) != 0x10);
> +
> CHECK_CSI_OFFSET(_sigpoll);
> CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int));
> CHECK_SI_SIZE (_sigpoll, 4*sizeof(int));
> diff --git a/fs/signalfd.c b/fs/signalfd.c
> index 456046e15873..040a1142915f 100644
> --- a/fs/signalfd.c
> +++ b/fs/signalfd.c
> @@ -134,6 +134,10 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
> #endif
> new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
> break;
> + case SIL_PERF_EVENT:
> + new.ssi_addr = (long) kinfo->si_addr;
> + new.ssi_perf = kinfo->si_perf;
> + break;
> case SIL_CHLD:
> new.ssi_pid = kinfo->si_pid;
> new.ssi_uid = kinfo->si_uid;
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index 6e65be753603..c8821d966812 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -236,6 +236,8 @@ typedef struct compat_siginfo {
> char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD];
> u32 _pkey;
> } _addr_pkey;
> + /* used when si_code=TRAP_PERF */
> + compat_u64 _perf;
> };
> } _sigfault;
>
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index 205526c4003a..1e98548d7cf6 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -43,6 +43,7 @@ enum siginfo_layout {
> SIL_FAULT_MCEERR,
> SIL_FAULT_BNDERR,
> SIL_FAULT_PKUERR,
> + SIL_PERF_EVENT,
> SIL_CHLD,
> SIL_RT,
> SIL_SYS,
> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
> index d2597000407a..d0bb9125c853 100644
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -91,6 +91,8 @@ union __sifields {
> char _dummy_pkey[__ADDR_BND_PKEY_PAD];
> __u32 _pkey;
> } _addr_pkey;
> + /* used when si_code=TRAP_PERF */
> + __u64 _perf;
> };
> } _sigfault;
>
> @@ -155,6 +157,7 @@ typedef struct siginfo {
> #define si_lower _sifields._sigfault._addr_bnd._lower
> #define si_upper _sifields._sigfault._addr_bnd._upper
> #define si_pkey _sifields._sigfault._addr_pkey._pkey
> +#define si_perf _sifields._sigfault._perf
> #define si_band _sifields._sigpoll._band
> #define si_fd _sifields._sigpoll._fd
> #define si_call_addr _sifields._sigsys._call_addr
> @@ -253,7 +256,8 @@ typedef struct siginfo {
> #define TRAP_BRANCH 3 /* process taken branch trap */
> #define TRAP_HWBKPT 4 /* hardware breakpoint/watchpoint */
> #define TRAP_UNK 5 /* undiagnosed trap */
> -#define NSIGTRAP 5
> +#define TRAP_PERF 6 /* perf event with sigtrap=1 */
> +#define NSIGTRAP 6
>
> /*
> * There is an additional set of SIGTRAP si_codes used by ptrace
> diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h
> index 83429a05b698..7e333042c7e3 100644
> --- a/include/uapi/linux/signalfd.h
> +++ b/include/uapi/linux/signalfd.h
> @@ -39,6 +39,8 @@ struct signalfd_siginfo {
> __s32 ssi_syscall;
> __u64 ssi_call_addr;
> __u32 ssi_arch;
> + __u32 __pad3;
> + __u64 ssi_perf;
>
> /*
> * Pad strcture to 128 bytes. Remember to update the
> @@ -49,7 +51,7 @@ struct signalfd_siginfo {
> * comes out of a read(2) and we really don't want to have
> * a compat on read(2).
> */
> - __u8 __pad[28];
> + __u8 __pad[16];
> };
>
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index f2718350bf4b..7061e4957650 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1199,6 +1199,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
> case SIL_FAULT_MCEERR:
> case SIL_FAULT_BNDERR:
> case SIL_FAULT_PKUERR:
> + case SIL_PERF_EVENT:
> case SIL_SYS:
> ret = false;
> break;
> @@ -2531,6 +2532,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
> case SIL_FAULT_MCEERR:
> case SIL_FAULT_BNDERR:
> case SIL_FAULT_PKUERR:
> + case SIL_PERF_EVENT:
> ksig->info.si_addr = arch_untagged_si_addr(
> ksig->info.si_addr, ksig->sig, ksig->info.si_code);
> break;
> @@ -3341,6 +3343,10 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
> #endif
> to->si_pkey = from->si_pkey;
> break;
> + case SIL_PERF_EVENT:
> + to->si_addr = ptr_to_compat(from->si_addr);
> + to->si_perf = from->si_perf;
> + break;
> case SIL_CHLD:
> to->si_pid = from->si_pid;
> to->si_uid = from->si_uid;
> @@ -3421,6 +3427,10 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
> #endif
> to->si_pkey = from->si_pkey;
> break;
> + case SIL_PERF_EVENT:
> + to->si_addr = compat_ptr(from->si_addr);
> + to->si_perf = from->si_perf;
> + break;
> case SIL_CHLD:
> to->si_pid = from->si_pid;
> to->si_uid = from->si_uid;
> @@ -4601,6 +4611,7 @@ static inline void siginfo_buildtime_checks(void)
> CHECK_OFFSET(si_lower);
> CHECK_OFFSET(si_upper);
> CHECK_OFFSET(si_pkey);
> + CHECK_OFFSET(si_perf);
>
> /* sigpoll */
> CHECK_OFFSET(si_band);
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski <[email protected]> wrote:
>
> Hi Marco,
>
> On 08.04.2021 12:36, Marco Elver wrote:
> > Introduces the TRAP_PERF si_code, and associated siginfo_t field
> > si_perf. These will be used by the perf event subsystem to send signals
> > (if requested) to the task where an event occurred.
> >
> > Acked-by: Geert Uytterhoeven <[email protected]> # m68k
> > Acked-by: Arnd Bergmann <[email protected]> # asm-generic
> > Signed-off-by: Marco Elver <[email protected]>
>
> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
> regression on my test systems (arm 32bit and 64bit). Most systems fails
> to boot in the given time frame. I've observed that there is a timeout
> waiting for udev to populate /dev and then also during the network
> interfaces configuration. Reverting this commit, together with
> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to let it
> compile, on top of next-20210420 fixes the issue.
Thanks, this is weird for sure and nothing in particular stands out.
I have questions:
-- Can you please share your config?
-- Also, can you share how you run this? Can it be reproduced in qemu?
-- How did you derive this patch to be at fault? Why not just
97ba62b27867, given you also need to revert it?
If you are unsure which patch exactly it is, can you try just
reverting 97ba62b27867 and see what happens?
Thanks,
-- Marco
> > ---
> > arch/m68k/kernel/signal.c | 3 +++
> > arch/x86/kernel/signal_compat.c | 5 ++++-
> > fs/signalfd.c | 4 ++++
> > include/linux/compat.h | 2 ++
> > include/linux/signal.h | 1 +
> > include/uapi/asm-generic/siginfo.h | 6 +++++-
> > include/uapi/linux/signalfd.h | 4 +++-
> > kernel/signal.c | 11 +++++++++++
> > 8 files changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> > index 349570f16a78..a4b7ee1df211 100644
> > --- a/arch/m68k/kernel/signal.c
> > +++ b/arch/m68k/kernel/signal.c
> > @@ -622,6 +622,9 @@ static inline void siginfo_build_tests(void)
> > /* _sigfault._addr_pkey */
> > BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x12);
> >
> > + /* _sigfault._perf */
> > + BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x10);
> > +
> > /* _sigpoll */
> > BUILD_BUG_ON(offsetof(siginfo_t, si_band) != 0x0c);
> > BUILD_BUG_ON(offsetof(siginfo_t, si_fd) != 0x10);
> > diff --git a/arch/x86/kernel/signal_compat.c b/arch/x86/kernel/signal_compat.c
> > index a5330ff498f0..0e5d0a7e203b 100644
> > --- a/arch/x86/kernel/signal_compat.c
> > +++ b/arch/x86/kernel/signal_compat.c
> > @@ -29,7 +29,7 @@ static inline void signal_compat_build_tests(void)
> > BUILD_BUG_ON(NSIGFPE != 15);
> > BUILD_BUG_ON(NSIGSEGV != 9);
> > BUILD_BUG_ON(NSIGBUS != 5);
> > - BUILD_BUG_ON(NSIGTRAP != 5);
> > + BUILD_BUG_ON(NSIGTRAP != 6);
> > BUILD_BUG_ON(NSIGCHLD != 6);
> > BUILD_BUG_ON(NSIGSYS != 2);
> >
> > @@ -138,6 +138,9 @@ static inline void signal_compat_build_tests(void)
> > BUILD_BUG_ON(offsetof(siginfo_t, si_pkey) != 0x20);
> > BUILD_BUG_ON(offsetof(compat_siginfo_t, si_pkey) != 0x14);
> >
> > + BUILD_BUG_ON(offsetof(siginfo_t, si_perf) != 0x18);
> > + BUILD_BUG_ON(offsetof(compat_siginfo_t, si_perf) != 0x10);
> > +
> > CHECK_CSI_OFFSET(_sigpoll);
> > CHECK_CSI_SIZE (_sigpoll, 2*sizeof(int));
> > CHECK_SI_SIZE (_sigpoll, 4*sizeof(int));
> > diff --git a/fs/signalfd.c b/fs/signalfd.c
> > index 456046e15873..040a1142915f 100644
> > --- a/fs/signalfd.c
> > +++ b/fs/signalfd.c
> > @@ -134,6 +134,10 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo,
> > #endif
> > new.ssi_addr_lsb = (short) kinfo->si_addr_lsb;
> > break;
> > + case SIL_PERF_EVENT:
> > + new.ssi_addr = (long) kinfo->si_addr;
> > + new.ssi_perf = kinfo->si_perf;
> > + break;
> > case SIL_CHLD:
> > new.ssi_pid = kinfo->si_pid;
> > new.ssi_uid = kinfo->si_uid;
> > diff --git a/include/linux/compat.h b/include/linux/compat.h
> > index 6e65be753603..c8821d966812 100644
> > --- a/include/linux/compat.h
> > +++ b/include/linux/compat.h
> > @@ -236,6 +236,8 @@ typedef struct compat_siginfo {
> > char _dummy_pkey[__COMPAT_ADDR_BND_PKEY_PAD];
> > u32 _pkey;
> > } _addr_pkey;
> > + /* used when si_code=TRAP_PERF */
> > + compat_u64 _perf;
> > };
> > } _sigfault;
> >
> > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > index 205526c4003a..1e98548d7cf6 100644
> > --- a/include/linux/signal.h
> > +++ b/include/linux/signal.h
> > @@ -43,6 +43,7 @@ enum siginfo_layout {
> > SIL_FAULT_MCEERR,
> > SIL_FAULT_BNDERR,
> > SIL_FAULT_PKUERR,
> > + SIL_PERF_EVENT,
> > SIL_CHLD,
> > SIL_RT,
> > SIL_SYS,
> > diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
> > index d2597000407a..d0bb9125c853 100644
> > --- a/include/uapi/asm-generic/siginfo.h
> > +++ b/include/uapi/asm-generic/siginfo.h
> > @@ -91,6 +91,8 @@ union __sifields {
> > char _dummy_pkey[__ADDR_BND_PKEY_PAD];
> > __u32 _pkey;
> > } _addr_pkey;
> > + /* used when si_code=TRAP_PERF */
> > + __u64 _perf;
> > };
> > } _sigfault;
> >
> > @@ -155,6 +157,7 @@ typedef struct siginfo {
> > #define si_lower _sifields._sigfault._addr_bnd._lower
> > #define si_upper _sifields._sigfault._addr_bnd._upper
> > #define si_pkey _sifields._sigfault._addr_pkey._pkey
> > +#define si_perf _sifields._sigfault._perf
> > #define si_band _sifields._sigpoll._band
> > #define si_fd _sifields._sigpoll._fd
> > #define si_call_addr _sifields._sigsys._call_addr
> > @@ -253,7 +256,8 @@ typedef struct siginfo {
> > #define TRAP_BRANCH 3 /* process taken branch trap */
> > #define TRAP_HWBKPT 4 /* hardware breakpoint/watchpoint */
> > #define TRAP_UNK 5 /* undiagnosed trap */
> > -#define NSIGTRAP 5
> > +#define TRAP_PERF 6 /* perf event with sigtrap=1 */
> > +#define NSIGTRAP 6
> >
> > /*
> > * There is an additional set of SIGTRAP si_codes used by ptrace
> > diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h
> > index 83429a05b698..7e333042c7e3 100644
> > --- a/include/uapi/linux/signalfd.h
> > +++ b/include/uapi/linux/signalfd.h
> > @@ -39,6 +39,8 @@ struct signalfd_siginfo {
> > __s32 ssi_syscall;
> > __u64 ssi_call_addr;
> > __u32 ssi_arch;
> > + __u32 __pad3;
> > + __u64 ssi_perf;
> >
> > /*
> > * Pad strcture to 128 bytes. Remember to update the
> > @@ -49,7 +51,7 @@ struct signalfd_siginfo {
> > * comes out of a read(2) and we really don't want to have
> > * a compat on read(2).
> > */
> > - __u8 __pad[28];
> > + __u8 __pad[16];
> > };
> >
> >
> > diff --git a/kernel/signal.c b/kernel/signal.c
> > index f2718350bf4b..7061e4957650 100644
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -1199,6 +1199,7 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
> > case SIL_FAULT_MCEERR:
> > case SIL_FAULT_BNDERR:
> > case SIL_FAULT_PKUERR:
> > + case SIL_PERF_EVENT:
> > case SIL_SYS:
> > ret = false;
> > break;
> > @@ -2531,6 +2532,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig)
> > case SIL_FAULT_MCEERR:
> > case SIL_FAULT_BNDERR:
> > case SIL_FAULT_PKUERR:
> > + case SIL_PERF_EVENT:
> > ksig->info.si_addr = arch_untagged_si_addr(
> > ksig->info.si_addr, ksig->sig, ksig->info.si_code);
> > break;
> > @@ -3341,6 +3343,10 @@ void copy_siginfo_to_external32(struct compat_siginfo *to,
> > #endif
> > to->si_pkey = from->si_pkey;
> > break;
> > + case SIL_PERF_EVENT:
> > + to->si_addr = ptr_to_compat(from->si_addr);
> > + to->si_perf = from->si_perf;
> > + break;
> > case SIL_CHLD:
> > to->si_pid = from->si_pid;
> > to->si_uid = from->si_uid;
> > @@ -3421,6 +3427,10 @@ static int post_copy_siginfo_from_user32(kernel_siginfo_t *to,
> > #endif
> > to->si_pkey = from->si_pkey;
> > break;
> > + case SIL_PERF_EVENT:
> > + to->si_addr = compat_ptr(from->si_addr);
> > + to->si_perf = from->si_perf;
> > + break;
> > case SIL_CHLD:
> > to->si_pid = from->si_pid;
> > to->si_uid = from->si_uid;
> > @@ -4601,6 +4611,7 @@ static inline void siginfo_buildtime_checks(void)
> > CHECK_OFFSET(si_lower);
> > CHECK_OFFSET(si_upper);
> > CHECK_OFFSET(si_pkey);
> > + CHECK_OFFSET(si_perf);
> >
> > /* sigpoll */
> > CHECK_OFFSET(si_band);
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Hi,
On 21.04.2021 00:42, Marco Elver wrote:
> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski <[email protected]> wrote:
>> On 08.04.2021 12:36, Marco Elver wrote:
>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
>>> si_perf. These will be used by the perf event subsystem to send signals
>>> (if requested) to the task where an event occurred.
>>>
>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
>>> Signed-off-by: Marco Elver <[email protected]>
>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
>> regression on my test systems (arm 32bit and 64bit). Most systems fails
>> to boot in the given time frame. I've observed that there is a timeout
>> waiting for udev to populate /dev and then also during the network
>> interfaces configuration. Reverting this commit, together with
>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to let it
>> compile, on top of next-20210420 fixes the issue.
> Thanks, this is weird for sure and nothing in particular stands out.
>
> I have questions:
> -- Can you please share your config?
This happens with standard multi_v7_defconfig (arm) or just defconfig
for arm64.
> -- Also, can you share how you run this? Can it be reproduced in qemu?
Nothing special. I just boot my test systems and see that they are
waiting lots of time during the udev populating /dev and network
interfaces configuration. I didn't try with qemu yet.
> -- How did you derive this patch to be at fault? Why not just
> 97ba62b27867, given you also need to revert it?
Well, I've just run my boot tests with automated 'git bisect' and that
was its result. It was a bit late in the evening, so I didn't analyze it
further, I've just posted a report about the issue I've found. It looks
that bisecting pointed to a wrong commit somehow.
> If you are unsure which patch exactly it is, can you try just
> reverting 97ba62b27867 and see what happens?
Indeed, this is a real faulty commit. Initially I've decided to revert
it to let kernel compile (it uses some symbols introduced by this
commit). Reverting only it on top of linux-next 20210420 also fixes the
issue. I'm sorry for the noise in this thread. I hope we will find what
really causes the issue.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski <[email protected]> wrote:
>
> On 21.04.2021 08:21, Marek Szyprowski wrote:
> > On 21.04.2021 00:42, Marco Elver wrote:
> >> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
> >> <[email protected]> wrote:
> >>> On 08.04.2021 12:36, Marco Elver wrote:
> >>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
> >>>> si_perf. These will be used by the perf event subsystem to send
> >>>> signals
> >>>> (if requested) to the task where an event occurred.
> >>>>
> >>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
> >>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
> >>>> Signed-off-by: Marco Elver <[email protected]>
> >>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
> >>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
> >>> regression on my test systems (arm 32bit and 64bit). Most systems fails
> >>> to boot in the given time frame. I've observed that there is a timeout
> >>> waiting for udev to populate /dev and then also during the network
> >>> interfaces configuration. Reverting this commit, together with
> >>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to let it
> >>> compile, on top of next-20210420 fixes the issue.
> >> Thanks, this is weird for sure and nothing in particular stands out.
> >>
> >> I have questions:
> >> -- Can you please share your config?
> >
> > This happens with standard multi_v7_defconfig (arm) or just defconfig
> > for arm64.
> >
> >> -- Also, can you share how you run this? Can it be reproduced in qemu?
> > Nothing special. I just boot my test systems and see that they are
> > waiting lots of time during the udev populating /dev and network
> > interfaces configuration. I didn't try with qemu yet.
> >> -- How did you derive this patch to be at fault? Why not just
> >> 97ba62b27867, given you also need to revert it?
> > Well, I've just run my boot tests with automated 'git bisect' and that
> > was its result. It was a bit late in the evening, so I didn't analyze
> > it further, I've just posted a report about the issue I've found. It
> > looks that bisecting pointed to a wrong commit somehow.
> >> If you are unsure which patch exactly it is, can you try just
> >> reverting 97ba62b27867 and see what happens?
> >
> > Indeed, this is a real faulty commit. Initially I've decided to revert
> > it to let kernel compile (it uses some symbols introduced by this
> > commit). Reverting only it on top of linux-next 20210420 also fixes
> > the issue. I'm sorry for the noise in this thread. I hope we will find
> > what really causes the issue.
>
> This was a premature conclusion. It looks that during the test I've did
> while writing that reply, the modules were not deployed properly and a
> test board (RPi4) booted without modules. In that case the board booted
> fine and there was no udev timeout. After deploying kernel modules, the
> udev timeout is back.
I'm confused now. Can you confirm that the problem is due to your
kernel modules, or do you think it's still due to 97ba62b27867? Or
fb6cc127e0b6 (this patch)?
Thanks,
-- Marco
On 21.04.2021 08:21, Marek Szyprowski wrote:
> On 21.04.2021 00:42, Marco Elver wrote:
>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
>> <[email protected]> wrote:
>>> On 08.04.2021 12:36, Marco Elver wrote:
>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
>>>> si_perf. These will be used by the perf event subsystem to send
>>>> signals
>>>> (if requested) to the task where an event occurred.
>>>>
>>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
>>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
>>>> Signed-off-by: Marco Elver <[email protected]>
>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
>>> regression on my test systems (arm 32bit and 64bit). Most systems fails
>>> to boot in the given time frame. I've observed that there is a timeout
>>> waiting for udev to populate /dev and then also during the network
>>> interfaces configuration. Reverting this commit, together with
>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to let it
>>> compile, on top of next-20210420 fixes the issue.
>> Thanks, this is weird for sure and nothing in particular stands out.
>>
>> I have questions:
>> -- Can you please share your config?
>
> This happens with standard multi_v7_defconfig (arm) or just defconfig
> for arm64.
>
>> -- Also, can you share how you run this? Can it be reproduced in qemu?
> Nothing special. I just boot my test systems and see that they are
> waiting lots of time during the udev populating /dev and network
> interfaces configuration. I didn't try with qemu yet.
>> -- How did you derive this patch to be at fault? Why not just
>> 97ba62b27867, given you also need to revert it?
> Well, I've just run my boot tests with automated 'git bisect' and that
> was its result. It was a bit late in the evening, so I didn't analyze
> it further, I've just posted a report about the issue I've found. It
> looks that bisecting pointed to a wrong commit somehow.
>> If you are unsure which patch exactly it is, can you try just
>> reverting 97ba62b27867 and see what happens?
>
> Indeed, this is a real faulty commit. Initially I've decided to revert
> it to let kernel compile (it uses some symbols introduced by this
> commit). Reverting only it on top of linux-next 20210420 also fixes
> the issue. I'm sorry for the noise in this thread. I hope we will find
> what really causes the issue.
This was a premature conclusion. It looks that during the test I've did
while writing that reply, the modules were not deployed properly and a
test board (RPi4) booted without modules. In that case the board booted
fine and there was no udev timeout. After deploying kernel modules, the
udev timeout is back.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marco,
On 21.04.2021 10:11, Marco Elver wrote:
> On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski <[email protected]> wrote:
>> On 21.04.2021 08:21, Marek Szyprowski wrote:
>>> On 21.04.2021 00:42, Marco Elver wrote:
>>>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
>>>> <[email protected]> wrote:
>>>>> On 08.04.2021 12:36, Marco Elver wrote:
>>>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
>>>>>> si_perf. These will be used by the perf event subsystem to send
>>>>>> signals
>>>>>> (if requested) to the task where an event occurred.
>>>>>>
>>>>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
>>>>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
>>>>>> Signed-off-by: Marco Elver <[email protected]>
>>>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
>>>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
>>>>> regression on my test systems (arm 32bit and 64bit). Most systems fails
>>>>> to boot in the given time frame. I've observed that there is a timeout
>>>>> waiting for udev to populate /dev and then also during the network
>>>>> interfaces configuration. Reverting this commit, together with
>>>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to let it
>>>>> compile, on top of next-20210420 fixes the issue.
>>>> Thanks, this is weird for sure and nothing in particular stands out.
>>>>
>>>> I have questions:
>>>> -- Can you please share your config?
>>> This happens with standard multi_v7_defconfig (arm) or just defconfig
>>> for arm64.
>>>
>>>> -- Also, can you share how you run this? Can it be reproduced in qemu?
>>> Nothing special. I just boot my test systems and see that they are
>>> waiting lots of time during the udev populating /dev and network
>>> interfaces configuration. I didn't try with qemu yet.
>>>> -- How did you derive this patch to be at fault? Why not just
>>>> 97ba62b27867, given you also need to revert it?
>>> Well, I've just run my boot tests with automated 'git bisect' and that
>>> was its result. It was a bit late in the evening, so I didn't analyze
>>> it further, I've just posted a report about the issue I've found. It
>>> looks that bisecting pointed to a wrong commit somehow.
>>>> If you are unsure which patch exactly it is, can you try just
>>>> reverting 97ba62b27867 and see what happens?
>>> Indeed, this is a real faulty commit. Initially I've decided to revert
>>> it to let kernel compile (it uses some symbols introduced by this
>>> commit). Reverting only it on top of linux-next 20210420 also fixes
>>> the issue. I'm sorry for the noise in this thread. I hope we will find
>>> what really causes the issue.
>> This was a premature conclusion. It looks that during the test I've did
>> while writing that reply, the modules were not deployed properly and a
>> test board (RPi4) booted without modules. In that case the board booted
>> fine and there was no udev timeout. After deploying kernel modules, the
>> udev timeout is back.
> I'm confused now. Can you confirm that the problem is due to your
> kernel modules, or do you think it's still due to 97ba62b27867? Or
> fb6cc127e0b6 (this patch)?
I don't use any custom kernel modules. I just deploy all modules that
are being built from the given kernel defconfig (arm multi_v7_defconfig
or arm64 default) and they are automatically loaded during the boot by
udev. I've checked again and bisect was right. The kernel built from
fb6cc127e0b6 suffers from the described issue, while the one build from
the previous commit (2e498d0a74e5) works fine.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Wed, 21 Apr 2021 at 12:57, Marek Szyprowski <[email protected]> wrote:
>
> On 21.04.2021 11:35, Marek Szyprowski wrote:
> > On 21.04.2021 10:11, Marco Elver wrote:
> >> On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski
> >> <[email protected]> wrote:
> >>> On 21.04.2021 08:21, Marek Szyprowski wrote:
> >>>> On 21.04.2021 00:42, Marco Elver wrote:
> >>>>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
> >>>>> <[email protected]> wrote:
> >>>>>> On 08.04.2021 12:36, Marco Elver wrote:
> >>>>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
> >>>>>>> si_perf. These will be used by the perf event subsystem to send
> >>>>>>> signals
> >>>>>>> (if requested) to the task where an event occurred.
> >>>>>>>
> >>>>>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
> >>>>>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
> >>>>>>> Signed-off-by: Marco Elver <[email protected]>
> >>>>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
> >>>>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
> >>>>>> regression on my test systems (arm 32bit and 64bit). Most systems
> >>>>>> fails
> >>>>>> to boot in the given time frame. I've observed that there is a
> >>>>>> timeout
> >>>>>> waiting for udev to populate /dev and then also during the network
> >>>>>> interfaces configuration. Reverting this commit, together with
> >>>>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to
> >>>>>> let it
> >>>>>> compile, on top of next-20210420 fixes the issue.
> >>>>> Thanks, this is weird for sure and nothing in particular stands out.
> >>>>>
> >>>>> I have questions:
> >>>>> -- Can you please share your config?
> >>>> This happens with standard multi_v7_defconfig (arm) or just defconfig
> >>>> for arm64.
> >>>>
> >>>>> -- Also, can you share how you run this? Can it be reproduced in
> >>>>> qemu?
> >>>> Nothing special. I just boot my test systems and see that they are
> >>>> waiting lots of time during the udev populating /dev and network
> >>>> interfaces configuration. I didn't try with qemu yet.
> >>>>> -- How did you derive this patch to be at fault? Why not just
> >>>>> 97ba62b27867, given you also need to revert it?
> >>>> Well, I've just run my boot tests with automated 'git bisect' and that
> >>>> was its result. It was a bit late in the evening, so I didn't analyze
> >>>> it further, I've just posted a report about the issue I've found. It
> >>>> looks that bisecting pointed to a wrong commit somehow.
> >>>>> If you are unsure which patch exactly it is, can you try just
> >>>>> reverting 97ba62b27867 and see what happens?
> >>>> Indeed, this is a real faulty commit. Initially I've decided to revert
> >>>> it to let kernel compile (it uses some symbols introduced by this
> >>>> commit). Reverting only it on top of linux-next 20210420 also fixes
> >>>> the issue. I'm sorry for the noise in this thread. I hope we will find
> >>>> what really causes the issue.
> >>> This was a premature conclusion. It looks that during the test I've did
> >>> while writing that reply, the modules were not deployed properly and a
> >>> test board (RPi4) booted without modules. In that case the board booted
> >>> fine and there was no udev timeout. After deploying kernel modules, the
> >>> udev timeout is back.
> >> I'm confused now. Can you confirm that the problem is due to your
> >> kernel modules, or do you think it's still due to 97ba62b27867? Or
> >> fb6cc127e0b6 (this patch)?
> >
> > I don't use any custom kernel modules. I just deploy all modules that
> > are being built from the given kernel defconfig (arm
> > multi_v7_defconfig or arm64 default) and they are automatically loaded
> > during the boot by udev. I've checked again and bisect was right. The
> > kernel built from fb6cc127e0b6 suffers from the described issue, while
> > the one build from the previous commit (2e498d0a74e5) works fine.
>
> I've managed to reproduce this issue with qemu. I've compiled the kernel
> for arm 32bit with multi_v7_defconfig and used some older Debian rootfs
> image. The log and qemu parameters are here:
> https://paste.debian.net/1194526/
>
> Check the timestamp for the 'EXT4-fs (vda): re-mounted' message and
> 'done (timeout)' status for the 'Waiting for /dev to be fully populated'
> message. This happens only when kernel modules build from the
> multi_v7_defconfig are deployed on the rootfs.
Still hard to say what is going on and what is at fault. But being
able to repro this in qemu helps debug quicker -- would you also be
able to share the precise rootfs.img, i.e. upload it somewhere I can
fetch it? And just to be sure, please also share your .config, as it
might have compiler-version dependent configuration that might help
repro (unlikely, but you never know).
Thanks,
-- Marco
On 21.04.2021 11:35, Marek Szyprowski wrote:
> On 21.04.2021 10:11, Marco Elver wrote:
>> On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski
>> <[email protected]> wrote:
>>> On 21.04.2021 08:21, Marek Szyprowski wrote:
>>>> On 21.04.2021 00:42, Marco Elver wrote:
>>>>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
>>>>> <[email protected]> wrote:
>>>>>> On 08.04.2021 12:36, Marco Elver wrote:
>>>>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
>>>>>>> si_perf. These will be used by the perf event subsystem to send
>>>>>>> signals
>>>>>>> (if requested) to the task where an event occurred.
>>>>>>>
>>>>>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
>>>>>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
>>>>>>> Signed-off-by: Marco Elver <[email protected]>
>>>>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
>>>>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
>>>>>> regression on my test systems (arm 32bit and 64bit). Most systems
>>>>>> fails
>>>>>> to boot in the given time frame. I've observed that there is a
>>>>>> timeout
>>>>>> waiting for udev to populate /dev and then also during the network
>>>>>> interfaces configuration. Reverting this commit, together with
>>>>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to
>>>>>> let it
>>>>>> compile, on top of next-20210420 fixes the issue.
>>>>> Thanks, this is weird for sure and nothing in particular stands out.
>>>>>
>>>>> I have questions:
>>>>> -- Can you please share your config?
>>>> This happens with standard multi_v7_defconfig (arm) or just defconfig
>>>> for arm64.
>>>>
>>>>> -- Also, can you share how you run this? Can it be reproduced in
>>>>> qemu?
>>>> Nothing special. I just boot my test systems and see that they are
>>>> waiting lots of time during the udev populating /dev and network
>>>> interfaces configuration. I didn't try with qemu yet.
>>>>> -- How did you derive this patch to be at fault? Why not just
>>>>> 97ba62b27867, given you also need to revert it?
>>>> Well, I've just run my boot tests with automated 'git bisect' and that
>>>> was its result. It was a bit late in the evening, so I didn't analyze
>>>> it further, I've just posted a report about the issue I've found. It
>>>> looks that bisecting pointed to a wrong commit somehow.
>>>>> If you are unsure which patch exactly it is, can you try just
>>>>> reverting 97ba62b27867 and see what happens?
>>>> Indeed, this is a real faulty commit. Initially I've decided to revert
>>>> it to let kernel compile (it uses some symbols introduced by this
>>>> commit). Reverting only it on top of linux-next 20210420 also fixes
>>>> the issue. I'm sorry for the noise in this thread. I hope we will find
>>>> what really causes the issue.
>>> This was a premature conclusion. It looks that during the test I've did
>>> while writing that reply, the modules were not deployed properly and a
>>> test board (RPi4) booted without modules. In that case the board booted
>>> fine and there was no udev timeout. After deploying kernel modules, the
>>> udev timeout is back.
>> I'm confused now. Can you confirm that the problem is due to your
>> kernel modules, or do you think it's still due to 97ba62b27867? Or
>> fb6cc127e0b6 (this patch)?
>
> I don't use any custom kernel modules. I just deploy all modules that
> are being built from the given kernel defconfig (arm
> multi_v7_defconfig or arm64 default) and they are automatically loaded
> during the boot by udev. I've checked again and bisect was right. The
> kernel built from fb6cc127e0b6 suffers from the described issue, while
> the one build from the previous commit (2e498d0a74e5) works fine.
I've managed to reproduce this issue with qemu. I've compiled the kernel
for arm 32bit with multi_v7_defconfig and used some older Debian rootfs
image. The log and qemu parameters are here:
https://paste.debian.net/1194526/
Check the timestamp for the 'EXT4-fs (vda): re-mounted' message and
'done (timeout)' status for the 'Waiting for /dev to be fully populated'
message. This happens only when kernel modules build from the
multi_v7_defconfig are deployed on the rootfs.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marco,
On 21.04.2021 13:03, Marco Elver wrote:
> On Wed, 21 Apr 2021 at 12:57, Marek Szyprowski <[email protected]> wrote:
>> On 21.04.2021 11:35, Marek Szyprowski wrote:
>>> On 21.04.2021 10:11, Marco Elver wrote:
>>>> On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski
>>>> <[email protected]> wrote:
>>>>> On 21.04.2021 08:21, Marek Szyprowski wrote:
>>>>>> On 21.04.2021 00:42, Marco Elver wrote:
>>>>>>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
>>>>>>> <[email protected]> wrote:
>>>>>>>> On 08.04.2021 12:36, Marco Elver wrote:
>>>>>>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
>>>>>>>>> si_perf. These will be used by the perf event subsystem to send
>>>>>>>>> signals
>>>>>>>>> (if requested) to the task where an event occurred.
>>>>>>>>>
>>>>>>>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
>>>>>>>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
>>>>>>>>> Signed-off-by: Marco Elver <[email protected]>
>>>>>>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
>>>>>>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
>>>>>>>> regression on my test systems (arm 32bit and 64bit). Most systems
>>>>>>>> fails
>>>>>>>> to boot in the given time frame. I've observed that there is a
>>>>>>>> timeout
>>>>>>>> waiting for udev to populate /dev and then also during the network
>>>>>>>> interfaces configuration. Reverting this commit, together with
>>>>>>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to
>>>>>>>> let it
>>>>>>>> compile, on top of next-20210420 fixes the issue.
>>>>>>> Thanks, this is weird for sure and nothing in particular stands out.
>>>>>>>
>>>>>>> I have questions:
>>>>>>> -- Can you please share your config?
>>>>>> This happens with standard multi_v7_defconfig (arm) or just defconfig
>>>>>> for arm64.
>>>>>>
>>>>>>> -- Also, can you share how you run this? Can it be reproduced in
>>>>>>> qemu?
>>>>>> Nothing special. I just boot my test systems and see that they are
>>>>>> waiting lots of time during the udev populating /dev and network
>>>>>> interfaces configuration. I didn't try with qemu yet.
>>>>>>> -- How did you derive this patch to be at fault? Why not just
>>>>>>> 97ba62b27867, given you also need to revert it?
>>>>>> Well, I've just run my boot tests with automated 'git bisect' and that
>>>>>> was its result. It was a bit late in the evening, so I didn't analyze
>>>>>> it further, I've just posted a report about the issue I've found. It
>>>>>> looks that bisecting pointed to a wrong commit somehow.
>>>>>>> If you are unsure which patch exactly it is, can you try just
>>>>>>> reverting 97ba62b27867 and see what happens?
>>>>>> Indeed, this is a real faulty commit. Initially I've decided to revert
>>>>>> it to let kernel compile (it uses some symbols introduced by this
>>>>>> commit). Reverting only it on top of linux-next 20210420 also fixes
>>>>>> the issue. I'm sorry for the noise in this thread. I hope we will find
>>>>>> what really causes the issue.
>>>>> This was a premature conclusion. It looks that during the test I've did
>>>>> while writing that reply, the modules were not deployed properly and a
>>>>> test board (RPi4) booted without modules. In that case the board booted
>>>>> fine and there was no udev timeout. After deploying kernel modules, the
>>>>> udev timeout is back.
>>>> I'm confused now. Can you confirm that the problem is due to your
>>>> kernel modules, or do you think it's still due to 97ba62b27867? Or
>>>> fb6cc127e0b6 (this patch)?
>>> I don't use any custom kernel modules. I just deploy all modules that
>>> are being built from the given kernel defconfig (arm
>>> multi_v7_defconfig or arm64 default) and they are automatically loaded
>>> during the boot by udev. I've checked again and bisect was right. The
>>> kernel built from fb6cc127e0b6 suffers from the described issue, while
>>> the one build from the previous commit (2e498d0a74e5) works fine.
>> I've managed to reproduce this issue with qemu. I've compiled the kernel
>> for arm 32bit with multi_v7_defconfig and used some older Debian rootfs
>> image. The log and qemu parameters are here:
>> https://protect2.fireeye.com/v1/url?k=7cfc23a2-23671aa9-7cfda8ed-002590f5b904-dab7e2ec39dae1f9&q=1&e=36a5ed13-6ad5-430c-8f44-e95c4f0af5c3&u=https%3A%2F%2Fpaste.debian.net%2F1194526%2F
>>
>> Check the timestamp for the 'EXT4-fs (vda): re-mounted' message and
>> 'done (timeout)' status for the 'Waiting for /dev to be fully populated'
>> message. This happens only when kernel modules build from the
>> multi_v7_defconfig are deployed on the rootfs.
> Still hard to say what is going on and what is at fault. But being
> able to repro this in qemu helps debug quicker -- would you also be
> able to share the precise rootfs.img, i.e. upload it somewhere I can
> fetch it? And just to be sure, please also share your .config, as it
> might have compiler-version dependent configuration that might help
> repro (unlikely, but you never know).
I've managed to reproduce this issue with a public Raspberry Pi OS Lite
rootfs image, even without deploying kernel modules:
https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
# qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
-monitor null -device virtio-blk-device,drive=virtio-blk -drive
file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
-netdev user,id=user -device virtio-net-device,netdev=user
The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
and boots if compiled from 2e498d0a74e5. In both cases I've used default
arm/multi_v7_defconfig and
gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
+Cc linux-arm-kernel
On Wed, 21 Apr 2021 at 15:19, Marek Szyprowski <[email protected]> wrote:
>
> Hi Marco,
>
> On 21.04.2021 13:03, Marco Elver wrote:
> > On Wed, 21 Apr 2021 at 12:57, Marek Szyprowski <[email protected]> wrote:
> >> On 21.04.2021 11:35, Marek Szyprowski wrote:
> >>> On 21.04.2021 10:11, Marco Elver wrote:
> >>>> On Wed, 21 Apr 2021 at 09:35, Marek Szyprowski
> >>>> <[email protected]> wrote:
> >>>>> On 21.04.2021 08:21, Marek Szyprowski wrote:
> >>>>>> On 21.04.2021 00:42, Marco Elver wrote:
> >>>>>>> On Tue, 20 Apr 2021 at 23:26, Marek Szyprowski
> >>>>>>> <[email protected]> wrote:
> >>>>>>>> On 08.04.2021 12:36, Marco Elver wrote:
> >>>>>>>>> Introduces the TRAP_PERF si_code, and associated siginfo_t field
> >>>>>>>>> si_perf. These will be used by the perf event subsystem to send
> >>>>>>>>> signals
> >>>>>>>>> (if requested) to the task where an event occurred.
> >>>>>>>>>
> >>>>>>>>> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
> >>>>>>>>> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
> >>>>>>>>> Signed-off-by: Marco Elver <[email protected]>
> >>>>>>>> This patch landed in linux-next as commit fb6cc127e0b6 ("signal:
> >>>>>>>> Introduce TRAP_PERF si_code and si_perf to siginfo"). It causes
> >>>>>>>> regression on my test systems (arm 32bit and 64bit). Most systems
> >>>>>>>> fails
> >>>>>>>> to boot in the given time frame. I've observed that there is a
> >>>>>>>> timeout
> >>>>>>>> waiting for udev to populate /dev and then also during the network
> >>>>>>>> interfaces configuration. Reverting this commit, together with
> >>>>>>>> 97ba62b27867 ("perf: Add support for SIGTRAP on perf events") to
> >>>>>>>> let it
> >>>>>>>> compile, on top of next-20210420 fixes the issue.
> >>>>>>> Thanks, this is weird for sure and nothing in particular stands out.
> >>>>>>>
> >>>>>>> I have questions:
> >>>>>>> -- Can you please share your config?
> >>>>>> This happens with standard multi_v7_defconfig (arm) or just defconfig
> >>>>>> for arm64.
> >>>>>>
> >>>>>>> -- Also, can you share how you run this? Can it be reproduced in
> >>>>>>> qemu?
> >>>>>> Nothing special. I just boot my test systems and see that they are
> >>>>>> waiting lots of time during the udev populating /dev and network
> >>>>>> interfaces configuration. I didn't try with qemu yet.
> >>>>>>> -- How did you derive this patch to be at fault? Why not just
> >>>>>>> 97ba62b27867, given you also need to revert it?
> >>>>>> Well, I've just run my boot tests with automated 'git bisect' and that
> >>>>>> was its result. It was a bit late in the evening, so I didn't analyze
> >>>>>> it further, I've just posted a report about the issue I've found. It
> >>>>>> looks that bisecting pointed to a wrong commit somehow.
> >>>>>>> If you are unsure which patch exactly it is, can you try just
> >>>>>>> reverting 97ba62b27867 and see what happens?
> >>>>>> Indeed, this is a real faulty commit. Initially I've decided to revert
> >>>>>> it to let kernel compile (it uses some symbols introduced by this
> >>>>>> commit). Reverting only it on top of linux-next 20210420 also fixes
> >>>>>> the issue. I'm sorry for the noise in this thread. I hope we will find
> >>>>>> what really causes the issue.
> >>>>> This was a premature conclusion. It looks that during the test I've did
> >>>>> while writing that reply, the modules were not deployed properly and a
> >>>>> test board (RPi4) booted without modules. In that case the board booted
> >>>>> fine and there was no udev timeout. After deploying kernel modules, the
> >>>>> udev timeout is back.
> >>>> I'm confused now. Can you confirm that the problem is due to your
> >>>> kernel modules, or do you think it's still due to 97ba62b27867? Or
> >>>> fb6cc127e0b6 (this patch)?
> >>> I don't use any custom kernel modules. I just deploy all modules that
> >>> are being built from the given kernel defconfig (arm
> >>> multi_v7_defconfig or arm64 default) and they are automatically loaded
> >>> during the boot by udev. I've checked again and bisect was right. The
> >>> kernel built from fb6cc127e0b6 suffers from the described issue, while
> >>> the one build from the previous commit (2e498d0a74e5) works fine.
> >> I've managed to reproduce this issue with qemu. I've compiled the kernel
> >> for arm 32bit with multi_v7_defconfig and used some older Debian rootfs
> >> image. The log and qemu parameters are here:
> >> https://protect2.fireeye.com/v1/url?k=7cfc23a2-23671aa9-7cfda8ed-002590f5b904-dab7e2ec39dae1f9&q=1&e=36a5ed13-6ad5-430c-8f44-e95c4f0af5c3&u=https%3A%2F%2Fpaste.debian.net%2F1194526%2F
> >>
> >> Check the timestamp for the 'EXT4-fs (vda): re-mounted' message and
> >> 'done (timeout)' status for the 'Waiting for /dev to be fully populated'
> >> message. This happens only when kernel modules build from the
> >> multi_v7_defconfig are deployed on the rootfs.
> > Still hard to say what is going on and what is at fault. But being
> > able to repro this in qemu helps debug quicker -- would you also be
> > able to share the precise rootfs.img, i.e. upload it somewhere I can
> > fetch it? And just to be sure, please also share your .config, as it
> > might have compiler-version dependent configuration that might help
> > repro (unlikely, but you never know).
>
> I've managed to reproduce this issue with a public Raspberry Pi OS Lite
> rootfs image, even without deploying kernel modules:
>
> https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
>
> # qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
> console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
> -monitor null -device virtio-blk-device,drive=virtio-blk -drive
> file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
> -netdev user,id=user -device virtio-net-device,netdev=user
>
> The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
> and boots if compiled from 2e498d0a74e5. In both cases I've used default
> arm/multi_v7_defconfig and
> gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.
Yup, I've narrowed it down to the addition of "__u64 _perf" to
siginfo_t. My guess is the __u64 causes a different alignment for a
bunch of adjacent fields. It seems that x86 and m68k are the only ones
that have compile-time tests for the offsets. Arm should probably add
those -- I have added a bucket of static_assert() in
arch/arm/kernel/signal.c and see that something's off.
I'll hopefully have a fix in a day or so.
Thanks,
-- Marco
Hello!
On 08/04/2021 11:36, Marco Elver wrote:
> Introduces the TRAP_PERF si_code, and associated siginfo_t field
> si_perf. These will be used by the perf event subsystem to send signals
> (if requested) to the task where an event occurred.
>
> Acked-by: Geert Uytterhoeven <[email protected]> # m68k
> Acked-by: Arnd Bergmann <[email protected]> # asm-generic
> Signed-off-by: Marco Elver <[email protected]>
Since next-20210420 I have noticed a boot regression on all 32-bit Tegra
that we are testing. Bisect is pointing to this commit and reverting
this patch and patch 6/10 does resolve the issue.
Interestingly there is no apparent crash, but these systems just appear
to hang silently after mounting the rootfs. If anyone has any thoughts
let me know!
Thanks
Jon
--
nvpublic
On Wed, Apr 21, 2021 at 06:27PM +0200, Marco Elver wrote:
> On Wed, Apr 21, 2021 at 05:11PM +0200, Marco Elver wrote:
> > +Cc linux-arm-kernel
> >
> [...]
> > >
> > > I've managed to reproduce this issue with a public Raspberry Pi OS Lite
> > > rootfs image, even without deploying kernel modules:
> > >
> > > https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
> > >
> > > # qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
> > > console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
> > > -monitor null -device virtio-blk-device,drive=virtio-blk -drive
> > > file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
> > > -netdev user,id=user -device virtio-net-device,netdev=user
> > >
> > > The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
> > > and boots if compiled from 2e498d0a74e5. In both cases I've used default
> > > arm/multi_v7_defconfig and
> > > gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.
> >
> > Yup, I've narrowed it down to the addition of "__u64 _perf" to
> > siginfo_t. My guess is the __u64 causes a different alignment for a
> > bunch of adjacent fields. It seems that x86 and m68k are the only ones
> > that have compile-time tests for the offsets. Arm should probably add
> > those -- I have added a bucket of static_assert() in
> > arch/arm/kernel/signal.c and see that something's off.
> >
> > I'll hopefully have a fix in a day or so.
>
> Arm and compiler folks: are there some special alignment requirement for
> __u64 on arm 32-bit? (And if there is for arm64, please shout as well.)
>
> With the static-asserts below, the only thing that I can do to fix it is
> to completely remove the __u64. Padding it before or after with __u32
> just does not work. It seems that the use of __u64 shifts everything
> in __sifields by 4 bytes.
>
> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
> index d0bb9125c853..b02a4ac55938 100644
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -92,7 +92,10 @@ union __sifields {
> __u32 _pkey;
> } _addr_pkey;
> /* used when si_code=TRAP_PERF */
> - __u64 _perf;
> + struct {
> + __u32 _perf1;
> + __u32 _perf2;
> + } _perf;
> };
> } _sigfault;
>
> ^^ works, but I'd hate to have to split this into 2 __u32 because it
> makes the whole design worse.
>
> What alignment trick do we have to do here to fix it for __u64?
So I think we just have to settle on 'unsigned long' here. On many
architectures, like 32-bit Arm, the alignment of a structure is that of
its largest member. This means that there is no portable way to add
64-bit integers to siginfo_t on 32-bit architectures.
In the case of the si_perf field, word size is sufficient since the data
it contains is user-defined. On 32-bit architectures, any excess bits of
perf_event_attr::sig_data will therefore be truncated when copying into
si_perf.
Feel free to test the below if you have time, but the below lets me boot
32-bit arm which previously timed out. It also passes all the
static_asserts() I added (will send those as separate patches).
Once I'm convinced this passes all others tests too, I'll send a patch.
Thanks,
-- Marco
diff --git a/include/linux/compat.h b/include/linux/compat.h
index c8821d966812..f0d2dd35d408 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -237,7 +237,7 @@ typedef struct compat_siginfo {
u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- compat_u64 _perf;
+ compat_ulong_t _perf;
};
} _sigfault;
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index d0bb9125c853..03d6f6d2c1fe 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -92,7 +92,7 @@ union __sifields {
__u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- __u64 _perf;
+ unsigned long _perf;
};
} _sigfault;
On Wed, Apr 21, 2021 at 05:11PM +0200, Marco Elver wrote:
> +Cc linux-arm-kernel
>
[...]
> >
> > I've managed to reproduce this issue with a public Raspberry Pi OS Lite
> > rootfs image, even without deploying kernel modules:
> >
> > https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
> >
> > # qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
> > console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
> > -monitor null -device virtio-blk-device,drive=virtio-blk -drive
> > file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
> > -netdev user,id=user -device virtio-net-device,netdev=user
> >
> > The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
> > and boots if compiled from 2e498d0a74e5. In both cases I've used default
> > arm/multi_v7_defconfig and
> > gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.
>
> Yup, I've narrowed it down to the addition of "__u64 _perf" to
> siginfo_t. My guess is the __u64 causes a different alignment for a
> bunch of adjacent fields. It seems that x86 and m68k are the only ones
> that have compile-time tests for the offsets. Arm should probably add
> those -- I have added a bucket of static_assert() in
> arch/arm/kernel/signal.c and see that something's off.
>
> I'll hopefully have a fix in a day or so.
Arm and compiler folks: are there some special alignment requirement for
__u64 on arm 32-bit? (And if there is for arm64, please shout as well.)
With the static-asserts below, the only thing that I can do to fix it is
to completely remove the __u64. Padding it before or after with __u32
just does not work. It seems that the use of __u64 shifts everything
in __sifields by 4 bytes.
diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
index d0bb9125c853..b02a4ac55938 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -92,7 +92,10 @@ union __sifields {
__u32 _pkey;
} _addr_pkey;
/* used when si_code=TRAP_PERF */
- __u64 _perf;
+ struct {
+ __u32 _perf1;
+ __u32 _perf2;
+ } _perf;
};
} _sigfault;
^^ works, but I'd hate to have to split this into 2 __u32 because it
makes the whole design worse.
What alignment trick do we have to do here to fix it for __u64?
------ >8 ------
diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index a3a38d0a4c85..6c558dc314c3 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -725,3 +725,41 @@ asmlinkage void do_rseq_syscall(struct pt_regs *regs)
rseq_syscall(regs);
}
#endif
+
+/*
+ * Compile-time tests for siginfo_t offsets. Changes to NSIG* likely come with
+ * new fields; new fields should be added below.
+ */
+static_assert(NSIGILL == 11);
+static_assert(NSIGFPE == 15);
+static_assert(NSIGSEGV == 9);
+static_assert(NSIGBUS == 5);
+static_assert(NSIGTRAP == 6);
+static_assert(NSIGCHLD == 6);
+static_assert(NSIGSYS == 2);
+static_assert(offsetof(siginfo_t, si_signo) == 0x00);
+static_assert(offsetof(siginfo_t, si_errno) == 0x04);
+static_assert(offsetof(siginfo_t, si_code) == 0x08);
+static_assert(offsetof(siginfo_t, si_pid) == 0x0c);
+#if 0
+static_assert(offsetof(siginfo_t, si_uid) == 0x10);
+static_assert(offsetof(siginfo_t, si_tid) == 0x0c);
+static_assert(offsetof(siginfo_t, si_overrun) == 0x10);
+static_assert(offsetof(siginfo_t, si_status) == 0x14);
+static_assert(offsetof(siginfo_t, si_utime) == 0x18);
+static_assert(offsetof(siginfo_t, si_stime) == 0x1c);
+static_assert(offsetof(siginfo_t, si_value) == 0x14);
+static_assert(offsetof(siginfo_t, si_int) == 0x14);
+static_assert(offsetof(siginfo_t, si_ptr) == 0x14);
+static_assert(offsetof(siginfo_t, si_addr) == 0x0c);
+static_assert(offsetof(siginfo_t, si_addr_lsb) == 0x10);
+static_assert(offsetof(siginfo_t, si_lower) == 0x14);
+static_assert(offsetof(siginfo_t, si_upper) == 0x18);
+static_assert(offsetof(siginfo_t, si_pkey) == 0x14);
+static_assert(offsetof(siginfo_t, si_perf) == 0x10);
+static_assert(offsetof(siginfo_t, si_band) == 0x0c);
+static_assert(offsetof(siginfo_t, si_fd) == 0x10);
+static_assert(offsetof(siginfo_t, si_call_addr) == 0x0c);
+static_assert(offsetof(siginfo_t, si_syscall) == 0x10);
+static_assert(offsetof(siginfo_t, si_arch) == 0x14);
+#endif
Hi Marco,
On 21.04.2021 20:23, Marco Elver wrote:
> On Wed, Apr 21, 2021 at 06:27PM +0200, Marco Elver wrote:
>> On Wed, Apr 21, 2021 at 05:11PM +0200, Marco Elver wrote:
>>> +Cc linux-arm-kernel
>>>
>> [...]
>>>> I've managed to reproduce this issue with a public Raspberry Pi OS Lite
>>>> rootfs image, even without deploying kernel modules:
>>>>
>>>> https://downloads.raspberrypi.org/raspios_lite_armhf/images/raspios_lite_armhf-2021-03-25/2021-03-04-raspios-buster-armhf-lite.zip
>>>>
>>>> # qemu-system-arm -M virt -smp 2 -m 512 -kernel zImage -append "earlycon
>>>> console=ttyAMA0 root=/dev/vda2 rw rootwait" -serial stdio -display none
>>>> -monitor null -device virtio-blk-device,drive=virtio-blk -drive
>>>> file=/tmp/2021-03-04-raspios-buster-armhf-lite.img,id=virtio-blk,if=none,format=raw
>>>> -netdev user,id=user -device virtio-net-device,netdev=user
>>>>
>>>> The above one doesn't boot if zImage z compiled from commit fb6cc127e0b6
>>>> and boots if compiled from 2e498d0a74e5. In both cases I've used default
>>>> arm/multi_v7_defconfig and
>>>> gcc-linaro-6.4.1-2017.11-x86_64_arm-linux-gnueabi toolchain.
>>> Yup, I've narrowed it down to the addition of "__u64 _perf" to
>>> siginfo_t. My guess is the __u64 causes a different alignment for a
>>> bunch of adjacent fields. It seems that x86 and m68k are the only ones
>>> that have compile-time tests for the offsets. Arm should probably add
>>> those -- I have added a bucket of static_assert() in
>>> arch/arm/kernel/signal.c and see that something's off.
>>>
>>> I'll hopefully have a fix in a day or so.
>> Arm and compiler folks: are there some special alignment requirement for
>> __u64 on arm 32-bit? (And if there is for arm64, please shout as well.)
>>
>> With the static-asserts below, the only thing that I can do to fix it is
>> to completely remove the __u64. Padding it before or after with __u32
>> just does not work. It seems that the use of __u64 shifts everything
>> in __sifields by 4 bytes.
>>
>> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
>> index d0bb9125c853..b02a4ac55938 100644
>> --- a/include/uapi/asm-generic/siginfo.h
>> +++ b/include/uapi/asm-generic/siginfo.h
>> @@ -92,7 +92,10 @@ union __sifields {
>> __u32 _pkey;
>> } _addr_pkey;
>> /* used when si_code=TRAP_PERF */
>> - __u64 _perf;
>> + struct {
>> + __u32 _perf1;
>> + __u32 _perf2;
>> + } _perf;
>> };
>> } _sigfault;
>>
>> ^^ works, but I'd hate to have to split this into 2 __u32 because it
>> makes the whole design worse.
>>
>> What alignment trick do we have to do here to fix it for __u64?
> So I think we just have to settle on 'unsigned long' here. On many
> architectures, like 32-bit Arm, the alignment of a structure is that of
> its largest member. This means that there is no portable way to add
> 64-bit integers to siginfo_t on 32-bit architectures.
>
> In the case of the si_perf field, word size is sufficient since the data
> it contains is user-defined. On 32-bit architectures, any excess bits of
> perf_event_attr::sig_data will therefore be truncated when copying into
> si_perf.
>
> Feel free to test the below if you have time, but the below lets me boot
> 32-bit arm which previously timed out. It also passes all the
> static_asserts() I added (will send those as separate patches).
>
> Once I'm convinced this passes all others tests too, I'll send a patch.
This fixes the issue I've observed on my test systems. Feel free to add:
Reported-by: Marek Szyprowski <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
> Thanks,
> -- Marco
>
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index c8821d966812..f0d2dd35d408 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -237,7 +237,7 @@ typedef struct compat_siginfo {
> u32 _pkey;
> } _addr_pkey;
> /* used when si_code=TRAP_PERF */
> - compat_u64 _perf;
> + compat_ulong_t _perf;
> };
> } _sigfault;
>
> diff --git a/include/uapi/asm-generic/siginfo.h b/include/uapi/asm-generic/siginfo.h
> index d0bb9125c853..03d6f6d2c1fe 100644
> --- a/include/uapi/asm-generic/siginfo.h
> +++ b/include/uapi/asm-generic/siginfo.h
> @@ -92,7 +92,7 @@ union __sifields {
> __u32 _pkey;
> } _addr_pkey;
> /* used when si_code=TRAP_PERF */
> - __u64 _perf;
> + unsigned long _perf;
> };
> } _sigfault;
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Thu, 22 Apr 2021 at 08:12, Marek Szyprowski <[email protected]> wrote:
[...]
> > So I think we just have to settle on 'unsigned long' here. On many
> > architectures, like 32-bit Arm, the alignment of a structure is that of
> > its largest member. This means that there is no portable way to add
> > 64-bit integers to siginfo_t on 32-bit architectures.
> >
> > In the case of the si_perf field, word size is sufficient since the data
> > it contains is user-defined. On 32-bit architectures, any excess bits of
> > perf_event_attr::sig_data will therefore be truncated when copying into
> > si_perf.
> >
> > Feel free to test the below if you have time, but the below lets me boot
> > 32-bit arm which previously timed out. It also passes all the
> > static_asserts() I added (will send those as separate patches).
> >
> > Once I'm convinced this passes all others tests too, I'll send a patch.
>
> This fixes the issue I've observed on my test systems. Feel free to add:
>
> Reported-by: Marek Szyprowski <[email protected]>
>
> Tested-by: Marek Szyprowski <[email protected]>
Thank you for testing! It's been sent:
https://lkml.kernel.org/r/[email protected]
Thanks,
-- Marco
On 22/04/2021 07:47, Marco Elver wrote:
> On Thu, 22 Apr 2021 at 08:12, Marek Szyprowski <[email protected]> wrote:
> [...]
>>> So I think we just have to settle on 'unsigned long' here. On many
>>> architectures, like 32-bit Arm, the alignment of a structure is that of
>>> its largest member. This means that there is no portable way to add
>>> 64-bit integers to siginfo_t on 32-bit architectures.
>>>
>>> In the case of the si_perf field, word size is sufficient since the data
>>> it contains is user-defined. On 32-bit architectures, any excess bits of
>>> perf_event_attr::sig_data will therefore be truncated when copying into
>>> si_perf.
>>>
>>> Feel free to test the below if you have time, but the below lets me boot
>>> 32-bit arm which previously timed out. It also passes all the
>>> static_asserts() I added (will send those as separate patches).
>>>
>>> Once I'm convinced this passes all others tests too, I'll send a patch.
>>
>> This fixes the issue I've observed on my test systems. Feel free to add:
>>
>> Reported-by: Marek Szyprowski <[email protected]>
>>
>> Tested-by: Marek Szyprowski <[email protected]>
>
> Thank you for testing! It's been sent:
> https://lkml.kernel.org/r/[email protected]
Thanks! This fixes the problem for Tegra as well. I have responded to
the above patch with my tested-by.
Cheers
Jon
--
nvpublic
Hi,
this also fixes s390.
strace's tests-m32 on s390 were failing.
Regards
Alex