Linux 4.4.97-rt111-rc1
Subsystems (5)
Steven Rostedt
reviewercover Linux 4.4.97-rt111-rc1
Dear RT Folks,
This is the RT stable review cycle of patch 4.4.97-rt111-rc1.
Please scream at me if I messed something up. Please test the patches too.
The -rc release will be uploaded to kernel.org and will be deleted when the final release is out. This is just a review release (or release candidate).
The pre-releases will not be pushed to the git repository, only the final release is.
If all goes well, this patch will be converted to the next main release on 11/27/2017.
Enjoy,
-- Steve
To build 4.4.97-rt111-rc1 directly, the following patches should be applied:
http://www.kernel.org/pub/linux/kernel/v4.x/linux-4.4.tar.xz
http://www.kernel.org/pub/linux/kernel/v4.x/patch-4.4.97.xz
http://www.kernel.org/pub/linux/kernel/projects/rt/4.4/patch-4.4.97-rt111-rc1.patch.xz
You can also build from 4.4.97-rt110 by applying the incremental patch:
http://www.kernel.org/pub/linux/kernel/projects/rt/4.4/incr/patch-4.4.97-rt110-rt111-rc1.patch.xz
Changes from 4.4.97-rt110:
---
Peter Zijlstra (1):
sched: Remove TASK_ALL
Sebastian Andrzej Siewior (4):
timer/hrtimer: check properly for a running timer
random: avoid preempt_disable()ed section sched/migrate disable: handle updated task-mask mg-dis section kernel/locking: use an exclusive wait_q for sleepers
Steven Rostedt (VMware) (1):
Linux 4.4.97-rt111-rc1
Thomas Gleixner (2):
rtmutex: Make lock_killable work
sched: Prevent task state corruption by spurious lock wakeup
----
drivers/char/random.c | 10 +++--- include/linux/hrtimer.h | 8 ++++- include/linux/sched.h | 19 ++++++++++-- kernel/fork.c | 1 + kernel/locking/rtmutex.c | 21 +++++-------- kernel/sched/core.c | 81 +++++++++++++++++++++++++++++++++++++++++------- localversion-rt | 2 +- 7 files changed, 109 insertions(+), 33 deletions(-)
Steven Rostedt
reviewerPATCH 1/8 timer/hrtimer: check properly for a running timer
4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Sebastian Andrzej Siewior <[email protected]>
hrtimer_callback_running() checks only whether a timmer is running on a CPU in hardirq-context. This is okay for !RT. For RT environment we move most timers to the timer-softirq and therefore we therefore need to check if the timer is running in the softirq context.
--- include/linux/hrtimer.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
include/linux/hrtimer.h +7 -1
@@ -455,7 +455,13 @@ static inline int hrtimer_is_queued(struct hrtimer *timer)*/ static inline int hrtimer_callback_running (const struct hrtimer *timer) { -return timer->base->cpu_base->running == timer; +if (timer->base->cpu_base->running == timer) +return 1 ; +#ifdef CONFIG_PREEMPT_RT_BASE +if (timer->base->cpu_base->running_soft == timer) +return 1 ; +#endif +return 0 ; }/* Forward a hrtimer so it expires after now: */
Signature · 1 line
2.13.2
Steven Rostedt
reviewerPATCH 2/8 rtmutex: Make lock_killable work
In reply to Steven Rostedt4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Thomas Gleixner <[email protected]>
Locking an rt mutex killable does not work because signal handling is restricted to TASK_INTERRUPTIBLE.
Use signal_pending_state() unconditionaly.
--- kernel/locking/rtmutex.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
kernel/locking/rtmutex.c +7 -12
@@ -1672,18 +1672,13 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,if (try_to_take_rt_mutex (lock, current, waiter))break ; -/* -* TASK_INTERRUPTIBLE checks for signals and -* timeout. Ignored otherwise. -*/ -if (unlikely (state ==TASK_INTERRUPTIBLE )) { -/* Signal pending? */ -if (signal_pending (current)) - ret = -EINTR ; -if (timeout && !timeout->task) - ret = -ETIMEDOUT ; -if (ret) -break ; +if (timeout && !timeout->task) { + ret = -ETIMEDOUT ; +break ; + } +if (signal_pending_state (state, current)) { + ret = -EINTR ; +break ; }if (ww_ctx && ww_ctx->acquired >0 ) {
Signature · 1 line
2.13.2
Steven Rostedt
reviewerPATCH 3/8 random: avoid preempt_disable()ed section
In reply to Steven Rostedt4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Sebastian Andrzej Siewior <[email protected]>
extract_crng() will use sleeping locks while in a preempt_disable() section due to get_cpu_var().
Work around it with local_locks.
--- drivers/char/random.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/char/random.c +6 -4
@@ -260,6 +260,7 @@#include <linux/irq.h> #include <linux/syscalls.h> #include <linux/completion.h> +#include <linux/locallock.h> #include <asm/processor.h> #include <asm/uaccess.h>
@@ -1796,6 +1797,7 @@ int random_int_secret_init(void) static DEFINE_PER_CPU(__u32 [MD5_DIGEST_WORDS], get_random_int_hash)__aligned(sizeof(unsigned long)) ; +static DEFINE_LOCAL_IRQ_LOCK (hash_entropy_int_lock);/* * Get a random word for internal kernel use only. Similar to urandom but
@@ -1811,12 +1813,12 @@ unsigned int get_random_int(void)if (arch_get_random_int (&ret))return ret; - hash =get_cpu_var (get_random_int_hash); + hash = &get_locked_var (hash_entropy_int_lock, get_random_int_hash); hash[0 ] += current->pid + jiffies +random_get_entropy ();md5_transform (hash, random_int_secret); ret = hash[0 ]; -put_cpu_var (get_random_int_hash); +put_locked_var (hash_entropy_int_lock, get_random_int_hash);return ret; }
@@ -1833,12 +1835,12 @@ unsigned long get_random_long(void)if (arch_get_random_long (&ret))return ret; - hash =get_cpu_var (get_random_int_hash); + hash = &get_locked_var (hash_entropy_int_lock, get_random_int_hash); hash[0 ] += current->pid + jiffies +random_get_entropy ();md5_transform (hash, random_int_secret); ret = *(unsigned long *)hash; -put_cpu_var (get_random_int_hash); +put_locked_var (hash_entropy_int_lock, get_random_int_hash);return ret; }
Signature · 1 line
2.13.2
Steven Rostedt
reviewerPATCH 4/8 sched: Prevent task state corruption by spurious lock wakeup
In reply to Steven Rostedt4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Thomas Gleixner <[email protected]>
Mathias and others reported GDB failures on RT.
The following scenario leads to task state corruption:
CPU0 CPU1
T1->state = TASK_XXX;spin_lock(&lock)
rt_spin_lock_slowlock(&lock->rtmutex)
raw_spin_lock(&rtm->wait_lock);
T1->saved_state = current->state;
T1->state = TASK_UNINTERRUPTIBLE;
spin_unlock(&lock)
task_blocks_on_rt_mutex(rtm) rt_spin_lock_slowunlock(&lock->rtmutex)
queue_waiter(rtm) raw_spin_lock(&rtm->wait_lock);
pi_chain_walk(rtm)
raw_spin_unlock(&rtm->wait_lock);
wake_top_waiter(T1)
raw_spin_lock(&rtm->wait_lock);
for (;;) {
if (__try_to_take_rt_mutex()) <- Succeeds
break;
...
}
T1->state = T1->saved_state;
try_to_wake_up(T1)
ttwu_do_wakeup(T1)
T1->state = TASK_RUNNING;In most cases this is harmless because waiting for some event, which is the usual reason for TASK_[UN]INTERRUPTIBLE has to be safe against other forms of spurious wakeups anyway.
But in case of TASK_TRACED this is actually fatal, because the task loses the TASK_TRACED state. In consequence it fails to consume SIGSTOP which was sent from the debugger and actually delivers SIGSTOP to the task which breaks the ptrace mechanics and brings the debugger into an unexpected state.
The TASK_TRACED state should prevent getting there due to the state matching logic in try_to_wake_up(). But that's not true because wake_up_lock_sleeper() uses TASK_ALL as state mask. That's bogus because lock sleepers always use TASK_UNINTERRUPTIBLE, so the wakeup should use that as well.
The cure is way simpler as figuring it out:
Change the mask used in wake_up_lock_sleeper() from TASK_ALL to TASK_UNINTERRUPTIBLE.
--- kernel/sched/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/sched/core.c +1 -1
@@ -2212,7 +2212,7 @@ EXPORT_SYMBOL(wake_up_process);*/ int wake_up_lock_sleeper (struct task_struct *p) { -return try_to_wake_up (p,TASK_ALL ,WF_LOCK_SLEEPER ); +return try_to_wake_up (p,TASK_UNINTERRUPTIBLE ,WF_LOCK_SLEEPER ); }int wake_up_state (struct task_struct *p,unsigned int state)
Signature · 1 line
2.13.2
Steven Rostedt
reviewerPATCH 5/8 sched: Remove TASK_ALL
In reply to Steven Rostedt4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Peter Zijlstra <[email protected]>
It's unused:
$ git grep "\<TASK_ALL\>" | wc -l 1
And dangerous, kill the bugger.
--- include/linux/sched.h | 1 - 1 file changed, 1 deletion(-)
include/linux/sched.h +0 -1
@@ -234,7 +234,6 @@ extern char ___assert_task_state[1 - 2*!!(/* Convenience macros for the sake of wake_up */ #define TASK_NORMAL (TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE) -#define TASK_ALL (TASK_NORMAL | __TASK_STOPPED | __TASK_TRACED)/* get_task_state() */ #define TASK_REPORT (TASK_RUNNING | TASK_INTERRUPTIBLE | \
Signature · 1 line
2.13.2
Peter Zijlstra
maintainerRe: [PATCH RT 5/8] sched: Remove TASK_ALL
Quoted 2 lines · Steven Rostedt
4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
No real objection, just curious as to why its -stable material.
Steven Rostedt
reviewerRe: [PATCH RT 5/8] sched: Remove TASK_ALL
Quoted 5 lines · On Tue, 21 Nov 2017 17:19:07 +0100 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines · Steven Rostedt
4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.No real objection, just curious as to why its -stable material.
Good question ;-) But it was marked with the stable-rt tag, Thomas said it was dangerous, and it applied nicely.
-- Steve
Re: [PATCH RT 5/8] sched: Remove TASK_ALL
Quoted 11 lines · Steven Rostedt
Quoted 5 lines · On Tue, 21 Nov 2017 17:19:07 +0100 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines · Steven Rostedt
4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.No real objection, just curious as to why its -stable material.
Good question ;-) But it was marked with the stable-rt tag, Thomas said it was dangerous, and it applied nicely.
Yes, it's dangerous and I wanted to make sure that it does not get reused by chance.
Thanks,
tglx
Steven Rostedt
reviewerPATCH 6/8 sched/migrate disable: handle updated task-mask mg-dis section
In reply to Steven Rostedt4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Sebastian Andrzej Siewior <[email protected]>
If task's cpumask changes while in the task is in a migrate_disable() section then we don't react on it after a migrate_enable(). It matters however if current CPU is no longer part of the cpumask. We also miss the ->set_cpus_allowed() callback.
This patch fixes it by setting task->migrate_disable_update once we this "delayed" hook.
This bug was introduced while fixing unrelated issue in migrate_disable() in v4.4-rt3 (update_migrate_disable() got removed during that).
--- include/linux/sched.h | 1 + kernel/sched/core.c | 59 +++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 54 insertions(+), 6 deletions(-)
include/linux/sched.h +1 -0
@@ -1438,6 +1438,7 @@ struct task_struct {unsigned int policy;#ifdef CONFIG_PREEMPT_RT_FULL int migrate_disable; +int migrate_disable_update;# ifdef CONFIG_SCHED_DEBUG int migrate_disable_atomic;# endif
kernel/sched/core.c +53 -6
83 lines not on this page · raw message
Signature · 1 line
2.13.2
Steven Rostedt
reviewerPATCH 8/8 Linux 4.4.97-rt111-rc1
In reply to Steven Rostedt4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: "Steven Rostedt (VMware)" <[email protected]>
--- localversion-rt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
localversion-rt +1 -1
@@ -1 +1 @@ --rt110 +-rt111-rc1
Signature · 1 line
2.13.2
A message that is not in the archive (sent off-list or not yet received): [email protected]
Steven Rostedt
reviewerRe: [PATCH RT 7/8] kernel/locking: use an exclusive wait_q for sleepers
Quoted 7 lines · On Tue, 21 Nov 2017 11:07:01 -0500 Steven Rostedt <[email protected]> wrote:
4.4.97-rt111-rc1 stable review patch.
If anyone has any objections, please let me know.------------------
From: Sebastian Andrzej Siewior <[email protected]>
Quoted 7 lines
If a task is queued as a sleeper for a wakeup and never goes to
schedule()(because it just obtained the lock) then it will receive a spurious wake up which is not "bad", it is considered. Until that wake up happens this task can no be enqueued for any wake ups handled by theWAKE_Qinfrastructure (because a task can only be enqueued once). This wouldn't be bad if we would use the same wakeup mechanism for the wake up of sleepers as we do for "normal" wake ups. But we don'tâŠ
Strange character's in change log, made quilt mail fail to send to list (bad encoding, because quilt mail can't handle this).
-- Steve
Quoted 132 lines
So.
T1 T2 T3spin_lock(x) spin_unlock(x); wake_q_add_sleeper(q1, T1) spin_unlock(x) set_state(TASK_INTERRUPTIBLE) if (!condition) schedule() condition = true wake_q_add(q2, T1) // T1 not added, still enqueued wake_up_q(q2) wake_up_q_sleeper(q1) // T1 not woken up, wrong task stateIn order to solve this race this patch adds a
wake_q_nodefor the sleeper case.Reported-by Mike Galbraith <[email protected]>Signed-off-by Sebastian Andrzej Siewior <[email protected]>Signed-off-by Steven Rostedt (VMware) <[email protected]>--- include/linux/sched.h | 17 +++++++++++++++-- kernel/fork.c | 1 + kernel/locking/rtmutex.c | 2 +- kernel/sched/core.c | 20 ++++++++++++++++---- 4 files changed, 33 insertions(+), 7 deletions(-)include/linux/sched.h +15
-2@@ -979,8 +979,20 @@ struct wake_q_head {#define WAKE_Q (name) \ struct wake_q_head name = { WAKE_Q_TAIL, &name.first } -extern void wake_q_add (struct wake_q_head *head, -struct task_struct *task); +extern void __wake_q_add (struct wake_q_head *head, +struct task_struct *task,bool sleeper); +static inline void wake_q_add (struct wake_q_head *head, +struct task_struct *task) +{ +__wake_q_add (head, task, false); +} + +static inline void wake_q_add_sleeper (struct wake_q_head *head, +struct task_struct *task) +{ +__wake_q_add (head, task, true); +} +extern void __wake_up_q (struct wake_q_head *head,bool sleeper);static inline void wake_up_q (struct wake_q_head *head)@@ -1640,6 +1652,7 @@ struct task_struct {raw_spinlock_t pi_lock;struct wake_q_node wake_q; +struct wake_q_node wake_q_sleeper;#ifdef CONFIG_RT_MUTEXES /* PI waiters blocked on a rt_mutex held by this task */ kernel/fork.c +1
-0@@ -395,6 +395,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node) tsk->splice_pipe =NULL ; tsk->task_frag.page =NULL ; tsk->wake_q.next =NULL ; + tsk->wake_q_sleeper.next =NULL ;account_kernel_stack (ti,1 );kernel/locking/rtmutex.c +1
-1@@ -1557,7 +1557,7 @@ static void mark_wakeup_next_waiter(struct wake_q_head *wake_q,raw_spin_unlock (¤t->pi_lock);if (waiter->savestate) -wake_q_add (wake_sleeper_q, waiter->task); +wake_q_add_sleeper (wake_sleeper_q, waiter->task);else wake_q_add (wake_q, waiter->task); }kernel/sched/core.c +16
-4@@ -523,9 +523,15 @@ static bool set_nr_if_polling(struct task_struct *p)#endif #endif -void wake_q_add (struct wake_q_head *head,struct task_struct *task) +void __wake_q_add (struct wake_q_head *head,struct task_struct *task, +bool sleeper) { -struct wake_q_node *node = &task->wake_q; +struct wake_q_node *node; + +if (sleeper) + node = &task->wake_q_sleeper; +else + node = &task->wake_q;/* * Atomically grab the task, if ->wake_q is !nil already it means @@ -554,11 +560,17 @@ void __wake_up_q(struct wake_q_head *head, bool sleeper)while (node !=WAKE_Q_TAIL ) {struct task_struct *task; - task = container_of(node, struct task_struct, wake_q); +if (sleeper) + task = container_of(node, struct task_struct, wake_q_sleeper); +else + task = container_of(node, struct task_struct, wake_q);BUG_ON (!task);/* task can safely be re-inserted now */ node = node->next; - task->wake_q.next =NULL ; +if (sleeper) + task->wake_q_sleeper.next =NULL ; +else + task->wake_q.next =NULL ;/* * wake_up_process() implies a wmb() to pair with the queueing