Re: [RFC PATCH v11 for 4.15 01/24] Restartable sequences system call
From: Mathieu Desnoyers <hidden>
Date: 2017-11-16 17:09:03
Also in:
lkml
Subsystem:
restartable sequences support, scheduler, the rest · Maintainers:
Mathieu Desnoyers, Peter Zijlstra, "Paul E. McKenney", Boqun Feng, Ingo Molnar, Juri Lelli, Vincent Guittot, Linus Torvalds
----- On Nov 16, 2017, at 11:32 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
On Thu, Nov 16, 2017 at 04:27:07PM +0000, Mathieu Desnoyers wrote:quoted
----- On Nov 16, 2017, at 11:18 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:quoted
On Tue, Nov 14, 2017 at 03:03:51PM -0500, Mathieu Desnoyers wrote:quoted
@@ -977,6 +978,13 @@ struct task_struct { unsigned long numa_pages_migrated; #endif /* CONFIG_NUMA_BALANCING */ +#ifdef CONFIG_RSEQ + struct rseq __user *rseq; + u32 rseq_len; + u32 rseq_sig; + bool rseq_preempt, rseq_signal, rseq_migrate;No bool please. Use something that has a defined size in ILP32/LP64. _Bool makes it absolutely impossible to speculate on structure layout across architectures.I should as well make all those a bitmask within a "u32 rseq_event_mask" then, sounds fair ?Sure, whatever works and isn't _Bool ;-)
So something along those lines should do the trick (including the mask request from Ben Maurer):
diff --git a/include/linux/sched.h b/include/linux/sched.h
index b995a3b..44aef30 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h@@ -982,7 +982,7 @@ struct task_struct { struct rseq __user *rseq; u32 rseq_len; u32 rseq_sig; - bool rseq_preempt, rseq_signal, rseq_migrate; + u32 rseq_event_mask; #endif struct tlbflush_unmap_batch tlb_ubc;
@@ -1676,6 +1676,16 @@ static inline void set_task_cpu(struct task_struct *p, un #endif #ifdef CONFIG_RSEQ +/* + * Map the event mask on the user-space ABI enum rseq_cs_flags + * for direct mask checks. + */ +enum rseq_event_mask { + RSEQ_EVENT_PREEMPT = RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT, + RSEQ_EVENT_SIGNAL = RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL, + RSEQ_EVENT_MIGRATE = RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE, +}; + static inline void rseq_set_notify_resume(struct task_struct *t) { if (t->rseq)
@@ -1718,16 +1728,16 @@ static inline void rseq_sched_out(struct task_struct *t) } static inline void rseq_signal_deliver(struct pt_regs *regs) { - current->rseq_signal = true; + current->rseq_event_mask |= RSEQ_EVENT_SIGNAL; rseq_handle_notify_resume(regs); } static inline void rseq_preempt(struct task_struct *t) { - t->rseq_preempt = true; + t->rseq_event_mask |= RSEQ_EVENT_PREEMPT; } static inline void rseq_migrate(struct task_struct *t) { - t->rseq_migrate = true; + t->rseq_event_mask |= RSEQ_EVENT_MIGRATE; } #else static inline void rseq_set_notify_resume(struct task_struct *t)
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 6f0d48c..d773003 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c@@ -159,7 +159,7 @@ static bool rseq_get_rseq_cs(struct task_struct *t, static int rseq_need_restart(struct task_struct *t, uint32_t cs_flags) { bool need_restart = false; - uint32_t flags; + uint32_t flags, event_mask; /* Get thread flags. */ if (__get_user(flags, &t->rseq->flags))
@@ -174,26 +174,17 @@ static int rseq_need_restart(struct task_struct *t, uint32 * a preempted signal handler could fail to restart the prior * execution context on sigreturn. */ - if (flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL) { - if (!(flags & RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE)) - return -EINVAL; - if (!(flags & RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)) + if (unlikely(flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL)) { + if ((flags & (RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE + | RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)) + != (RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE + | RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)) return -EINVAL; } - if (t->rseq_migrate - && !(flags & RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE)) - need_restart = true; - else if (t->rseq_preempt - && !(flags & RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT)) - need_restart = true; - else if (t->rseq_signal - && !(flags & RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL)) - need_restart = true; - - t->rseq_preempt = false; - t->rseq_signal = false; - t->rseq_migrate = false; - if (need_restart) + event_mask = t->rseq_event_mask; + t->rseq_event_mask = 0; + event_mask &= ~flags; + if (event_mask) return 1; return 0; }
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com