Skip to contentSkip to conversation

Linux 4.4.97-rt111-rc2

PATCH In review Steven Rostedt 2 participants 12 messages Nov 23, 2017 – Nov 28, 2017

Subsystems (7)
lore
Share & tools
lore

Steven Rostedt

author reviewer
#0 lore raw

cover Linux 4.4.97-rt111-rc2

Dear RT Folks,

This is the RT stable review cycle of patch 4.4.97-rt111-rc2.

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.

Note, I realized I only backported from v4.13.10-rt3 and not v4.13.13-rt5. Added a few more patches.

Enjoy,

-- Steve

To build 4.4.97-rt111-rc2 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-rc2.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-rc2.patch.xz

Changes from 4.4.97-rt110:

---

Peter Zijlstra (1):
sched: Remove TASK_ALL

Sebastian Andrzej Siewior (6):
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
fs: convert two more BH_Uptodate_Lock related bitspinlocks md/raid5: do not disable interrupts

Steven Rostedt (VMware) (1):

      Linux 4.4.97-rt111-rc2

Thomas Gleixner (2):
rtmutex: Make lock_killable work
sched: Prevent task state corruption by spurious lock wakeup

----

 drivers/char/random.c    | 10 +++---
 drivers/md/raid5.c       |  4 +--
 fs/ext4/page-io.c        |  6 ++--
 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 +-
 9 files changed, 113 insertions(+), 39 deletions(-)
author reviewer
#1 lore raw

PATCH 1/10 timer/hrtimer: check properly for a running timer

4.4.97-rt111-rc2 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
index 8fbcdfa5dc77..ff317006d3e8 100644
@@ -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
author reviewer
#2 lore raw

PATCH 2/10 rtmutex: Make lock_killable work

In reply to Steven Rostedt

4.4.97-rt111-rc2 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
index 0e9a6260441d..552dc6dd3a79 100644
@@ -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
author reviewer
#3 lore raw

PATCH 3/10 random: avoid preempt_disable()ed section

In reply to Steven Rostedt

4.4.97-rt111-rc2 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
index fecc40a69df8..b41745c5962c 100644
@@ -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
#4 lore raw

Re: [PATCH RT 03/10] random: avoid preempt_disable()ed section

Hi Steve,

I just build the patches, a build error found here:

drivers/char/random.c: In function ‘get_random_int’:
drivers/char/random.c:1816:7: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]

  hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);
       ^
drivers/char/random.c: In function ‘get_random_long’:
drivers/char/random.c:1838:7: error: assignment from incompatible
pointer type [-Werror=incompatible-pointer-types]
  hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);
       ^

Review

Quoted 2 lines
(quoted diff) +1 -1
-	hash = get_cpu_var(get_random_int_hash);
+	hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);
               ^

Is this a extra '&' which need to remove?

Quoted 10 lines
drivers/char/random.c +1 -1
 	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;
 }

drivers/char/random.c unsigned long get_random_long(void) · line 1838

Quoted 5 lines
drivers/char/random.c +1 -1
@@ -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);
               ^

Ditto

Regards
Alex

author reviewer
#5 lore raw

Re: [PATCH RT 03/10] random: avoid preempt_disable()ed section

Quoted 33 lines · On Fri, 24 Nov 2017 14:26:40 +0800 Alex Shi <[email protected]> wrote:

Hi Steve,

I just build the patches, a build error found here:

drivers/char/random.c: In function ‘get_random_int’:
drivers/char/random.c:1816:7: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]

  hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);
       ^
drivers/char/random.c: In function ‘get_random_long’:
drivers/char/random.c:1838:7: error: assignment from incompatible
pointer type [-Werror=incompatible-pointer-types]
  hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);
       ^

Review

Quoted 2 lines
(quoted diff) +1 -1
-	hash = get_cpu_var(get_random_int_hash);
+	hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);  
               ^

Is this a extra '&' which need to remove?

Quoted 13 lines
(quoted diff) +1 -1
 	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;
 
Quoted 4 lines

Review unsigned long get_random_long(void) · line 1838

Quoted 2 lines
(quoted diff) +1 -1
-	hash = get_cpu_var(get_random_int_hash);
+	hash = &get_locked_var(hash_entropy_int_lock, get_random_int_hash);  
               ^

Ditto

Thanks! I made the following update. I forgot to check for warnings in the build. :-/

-- Steve

drivers/char/random.c +2 -2
index b41745c5962c..46c0e27cf27f 100644
@@ -1813,7 +1813,7 @@ unsigned int get_random_int(void)
 	if (arch_get_random_int(&ret))
 		return ret;
 
-	hash = &get_locked_var(hash_entropy_int_lock, 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);
@@ -1835,7 +1835,7 @@ unsigned long get_random_long(void)
 	if (arch_get_random_long(&ret))
 		return ret;
 
-	hash = &get_locked_var(hash_entropy_int_lock, 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);
author reviewer
#6 lore raw

PATCH 4/10 sched: Prevent task state corruption by spurious lock wakeup

In reply to Steven Rostedt

4.4.97-rt111-rc2 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
index ed9550c87f66..970b893a1d15 100644
@@ -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
author reviewer
#7 lore raw

PATCH 5/10 sched: Remove TASK_ALL

In reply to Steven Rostedt

4.4.97-rt111-rc2 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
index b7b001e26509..56ccd0a3dd49 100644
@@ -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
author reviewer
#8 lore raw

PATCH 6/10 sched/migrate disable: handle updated task-mask mg-dis section

In reply to Steven Rostedt

4.4.97-rt111-rc2 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
index 56ccd0a3dd49..331cdbfc6431 100644
@@ -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
author reviewer
#9 lore raw

PATCH 8/10 fs: convert two more BH_Uptodate_Lock related bitspinlocks

In reply to Steven Rostedt

4.4.97-rt111-rc2 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <[email protected]>

We convert all BH_Uptodate_Lock based bit-spinlocks to use bh_uptodate_lock_irqsave() instead. Those two were introduced after the initial change in -RT and were not noticed before.

---
 fs/ext4/page-io.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
fs/ext4/page-io.c +2 -4
index 6ca56f5f72b5..9e145fe7cae0 100644
@@ -97,8 +97,7 @@ static void ext4_finish_bio(struct bio *bio)
 		 * We check all buffers in the page under BH_Uptodate_Lock
 		 * to avoid races with other end io clearing async_write flags
 		 */
-		local_irq_save(flags);
-		bit_spin_lock(BH_Uptodate_Lock, &head->b_state);
+		flags = bh_uptodate_lock_irqsave(head);
 		do {
 			if (bh_offset(bh) < bio_start ||
 			    bh_offset(bh) + bh->b_size > bio_end) {
@@ -110,8 +109,7 @@ static void ext4_finish_bio(struct bio *bio)
 			if (bio->bi_error)
 				buffer_io_error(bh);
 		} while ((bh = bh->b_this_page) != head);
-		bit_spin_unlock(BH_Uptodate_Lock, &head->b_state);
-		local_irq_restore(flags);
+		bh_uptodate_unlock_irqrestore(head, flags);
 		if (!under_io) {
 #ifdef CONFIG_EXT4_FS_ENCRYPTION
 			if (ctx)
Signature · 1 line
2.13.2
author reviewer
#10 lore raw

PATCH 9/10 md/raid5: do not disable interrupts

In reply to Steven Rostedt

4.4.97-rt111-rc2 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <[email protected]>

|BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:974
|in_atomic(): 0, irqs_disabled(): 1, pid: 2992, name: lvm
|CPU: 2 PID: 2992 Comm: lvm Not tainted 4.13.10-rt3+ #54
|Call Trace:
| dump_stack+0x4f/0x65
| ___might_sleep+0xfc/0x150
| atomic_dec_and_spin_lock+0x3c/0x80
| raid5_release_stripe+0x73/0x110
| grow_one_stripe+0xce/0xf0
| setup_conf+0x841/0xaa0
| raid5_run+0x7e7/0xa40
| md_run+0x515/0xaf0
| raid_ctr+0x147d/0x25e0
| dm_table_add_target+0x155/0x320
| table_load+0x103/0x320
| ctl_ioctl+0x1d9/0x510
| dm_ctl_ioctl+0x9/0x10
| do_vfs_ioctl+0x8e/0x670
| SyS_ioctl+0x3c/0x70
| entry_SYSCALL_64_fastpath+0x17/0x98

The interrupts were disabled because ->device_lock is taken with interrupts disabled.

---
 drivers/md/raid5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/md/raid5.c +2 -2
index 9b1aedb8e5df..8b236d622889 100644
@@ -429,7 +429,7 @@ void raid5_release_stripe(struct stripe_head *sh)
 		md_wakeup_thread(conf->mddev->thread);
 	return;
 slow_path:
-	local_irq_save(flags);
+	local_irq_save_nort(flags);
 	/* we are ok here if STRIPE_ON_RELEASE_LIST is set or not */
 	if (atomic_dec_and_lock(&sh->count, &conf->device_lock)) {
 		INIT_LIST_HEAD(&list);
@@ -438,7 +438,7 @@ slow_path:
 		spin_unlock(&conf->device_lock);
 		release_inactive_stripe_list(conf, &list, hash);
 	}
-	local_irq_restore(flags);
+	local_irq_restore_nort(flags);
 }
 
 static inline void remove_hash(struct stripe_head *sh)
Signature · 1 line
2.13.2
author reviewer
#11 lore raw

PATCH 10/10 Linux 4.4.97-rt111-rc2

In reply to Steven Rostedt

4.4.97-rt111-rc2 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
index b3e668a8fb94..757d33fb65a3 100644
@@ -1 +1 @@
--rt110
+-rt111-rc2
Signature · 1 line
2.13.2