Skip to contentSkip to conversation

Linux 5.4.74-rt42-rc2

PATCH In review Steven Rostedt 4 participants 12 messages Nov 10, 2020 – Nov 16, 2020

Subsystems (6)
Networking Memory management memcg ptrace support +2 more subsystems
lore
Share & tools
lore

Steven Rostedt

author
#0 lore raw

cover Linux 5.4.74-rt42-rc2

Dear RT Folks,

This is the RT stable review cycle of patch 5.4.74-rt42-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/11/2020.

Changes since -rc1: removed patch:

tcp: Remove superfluous BH-disable around listening_hash

Enjoy,

-- Steve

To build 5.4.74-rt42-rc2 directly, the following patches should be applied:

http://www.kernel.org/pub/linux/kernel/v5.x/linux-5.4.tar.xz

http://www.kernel.org/pub/linux/kernel/v5.x/patch-5.4.74.xz

http://www.kernel.org/pub/linux/kernel/projects/rt/5.4/patch-5.4.74-rt42-rc2.patch.xz

You can also build from 5.4.74-rt41 by applying the incremental patch:

http://www.kernel.org/pub/linux/kernel/projects/rt/5.4/incr/patch-5.4.74-rt41-rt42-rc2.patch.xz

Changes from 5.4.74-rt41:

---

Oleg Nesterov (1):
ptrace: fix ptrace_unfreeze_traced() race with rt-lock

Sebastian Andrzej Siewior (3):
net: Properly annotate the try-lock for the seqlock mm/memcontrol: Disable preemption in __mod_memcg_lruvec_state()
timers: Don't block on ->expiry_lock for TIMER_IRQSAFE

Steven Rostedt (VMware) (1):

      Linux 5.4.74-rt42-rc2

----

 include/linux/seqlock.h   |  9 ---------
 include/net/sch_generic.h | 10 +++++++++-
 kernel/ptrace.c           | 23 +++++++++++++++--------
 kernel/time/timer.c       |  9 ++++++++-
 localversion-rt           |  2 +-
 mm/memcontrol.c           |  2 ++
 6 files changed, 35 insertions(+), 20 deletions(-)
author
#1 lore raw

PATCH 1/5 net: Properly annotate the try-lock for the seqlock

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

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

From: Sebastian Andrzej Siewior <[email protected]>

In patch
("net/Qdisc: use a seqlock instead seqcount")

the seqcount has been replaced with a seqlock to allow to reader to boost the preempted writer.
The try_write_seqlock() acquired the lock with a try-lock but the seqcount annotation was "lock".

Opencode write_seqcount_t_begin() and use the try-lock annotation for lockdep.

Reported-by Mike Galbraith <[email protected]>
Cc [email protected]
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 include/linux/seqlock.h   |  9 ---------
 include/net/sch_generic.h | 10 +++++++++-
 2 files changed, 9 insertions(+), 10 deletions(-)
include/linux/seqlock.h +0 -9
index e5207897c33e..f390293974ea 100644
@@ -489,15 +489,6 @@ static inline void write_seqlock(seqlock_t *sl)
 	__raw_write_seqcount_begin(&sl->seqcount);
 }
 
-static inline int try_write_seqlock(seqlock_t *sl)
-{
-	if (spin_trylock(&sl->lock)) {
-		__raw_write_seqcount_begin(&sl->seqcount);
-		return 1;
-	}
-	return 0;
-}
-
 static inline void write_sequnlock(seqlock_t *sl)
 {
 	__raw_write_seqcount_end(&sl->seqcount);
include/net/sch_generic.h +9 -1
index e6afb4b9cede..112d2dca8b08 100644
@@ -168,8 +168,16 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc)
 		return false;
 	}
 #ifdef CONFIG_PREEMPT_RT
-	if (try_write_seqlock(&qdisc->running))
+	if (spin_trylock(&qdisc->running.lock)) {
+		seqcount_t *s = &qdisc->running.seqcount;
+		/*
+		 * Variant of write_seqcount_t_begin() telling lockdep that a
+		 * trylock was attempted.
+		 */
+		__raw_write_seqcount_begin(s);
+		seqcount_acquire(&s->dep_map, 0, 1, _RET_IP_);
 		return true;
+	}
 	return false;
 #else
 	/* Variant of write_seqcount_begin() telling lockdep a trylock
Signature · 1 line
2.28.0
#2 lore raw

Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

Hi Steve,

Quoted 62 lines · Steven Rostedt

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

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

From: Sebastian Andrzej Siewior <[email protected]>

In patch
("net/Qdisc: use a seqlock instead seqcount")

the seqcount has been replaced with a seqlock to allow to reader to boost the preempted writer.
The try_write_seqlock() acquired the lock with a try-lock but the seqcount annotation was "lock".

Opencode write_seqcount_t_begin() and use the try-lock annotation for lockdep.

Reported-by Mike Galbraith <[email protected]>
Cc [email protected]
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 include/linux/seqlock.h   |  9 ---------
 include/net/sch_generic.h | 10 +++++++++-
 2 files changed, 9 insertions(+), 10 deletions(-)
include/linux/seqlock.h +0 -9
index e5207897c33e..f390293974ea 100644
@@ -489,15 +489,6 @@ static inline void write_seqlock(seqlock_t *sl)
 	__raw_write_seqcount_begin(&sl->seqcount);
 }
 
-static inline int try_write_seqlock(seqlock_t *sl)
-{
-	if (spin_trylock(&sl->lock)) {
-		__raw_write_seqcount_begin(&sl->seqcount);
-		return 1;
-	}
-	return 0;
-}
-
 static inline void write_sequnlock(seqlock_t *sl)
 {
 	__raw_write_seqcount_end(&sl->seqcount);
include/net/sch_generic.h +4 -1
index e6afb4b9cede..112d2dca8b08 100644
@@ -168,8 +168,16 @@ static inline bool qdisc_run_begin(struct Qdisc
 *qdisc)
 		return false;
 	}
 #ifdef CONFIG_PREEMPT_RT
-	if (try_write_seqlock(&qdisc->running))
+	if (spin_trylock(&qdisc->running.lock)) {
+		seqcount_t *s = &qdisc->running.seqcount;
+		/*
+		 * Variant of write_seqcount_t_begin() telling lockdep

that a

include/net/sch_generic.h static inline bool qdisc_run_begin(struct Qdisc *qdisc) · line 183

Quoted 9 lines
include/net/sch_generic.h +5 -0
+		 * trylock was attempted.
+		 */
+		__raw_write_seqcount_begin(s);
+		seqcount_acquire(&s->dep_map, 0, 1, _RET_IP_);
 		return true;
+	}
 	return false;
 #else
 	/* Variant of write_seqcount_begin() telling lockdep a trylock

I applied this to 4.19 and saw some splat with my 'debug-full' configuration, so tried 5.4 and saw the same thing:

[   30.573698] BUG: workqueue leaked lock or atomic: kworker/1:4/0x00000000/143
                    last function: wireless_nlevent_process
[   30.573707] 1 lock held by kworker/1:4/143:
[   30.573708]  #0: ffffffff8e981d80 (noop_qdisc.running#2){+.+.}, at: __do_softirq+0xca/0x561
[   30.573715] CPU: 1 PID: 143 Comm: kworker/1:4 Not tainted 5.4.74-rt42-rt-test-full-debug #1
[   30.573716] Hardware name: LENOVO 4236L51/4236L51, BIOS 83ET59WW (1.29 ) 06/01/2011
[   30.573720] Workqueue: events wireless_nlevent_process
[   30.573721] Call Trace:
[   30.573724]  dump_stack+0x71/0x9b
[   30.573728]  process_one_work+0x533/0x760
[   30.573731]  worker_thread+0x39/0x3f0
[   30.573733]  ? process_one_work+0x760/0x760
[   30.573734]  kthread+0x165/0x180
[   30.573736]  ? __kthread_create_on_node+0x180/0x180
[   30.573737]  ret_from_fork+0x3a/0x50
[   30.629329] wlp3s0: authenticate with 84:1b:5e:41:5e:4d
[   30.634864] wlp3s0: send auth to 84:1b:5e:41:5e:4d (try 1/3)
[   30.638433] wlp3s0: authenticated
[   30.642250] wlp3s0: associate with 84:1b:5e:41:5e:4d (try 1/3)
[   30.645704] wlp3s0: RX AssocResp from 84:1b:5e:41:5e:4d (capab=0x411 status=0 aid=6)
[   30.650803] wlp3s0: associated
[   30.656764] ================================================
[   30.656765] WARNING: lock held when returning to user space!
[   30.656766] 5.4.74-rt42-rt-test-full-debug #1 Not tainted
[   30.656767] ------------------------------------------------
[   30.656768] wpa_supplicant/836 is leaving the kernel with locks still held!
[   30.656769] 1 lock held by wpa_supplicant/836:
[   30.656770]  #0: ffff98f1c9622280 (&dev->qdisc_running_key){+.+.}, at: packet_sendmsg+0xec1/0x1ad0

Let me know if you want me to send you my .config or the full output (a bunch more of the above).

Thanks,

Tom

#3 lore raw

Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

Hi Steve,

Quoted 116 lines · Tom Zanussi

Hi Steve,

Quoted 65 lines · Steven Rostedt

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

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

From: Sebastian Andrzej Siewior <[email protected]>

In patch
("net/Qdisc: use a seqlock instead seqcount")

the seqcount has been replaced with a seqlock to allow to reader to boost the preempted writer.
The try_write_seqlock() acquired the lock with a try-lock but the seqcount annotation was "lock".

Opencode write_seqcount_t_begin() and use the try-lock annotation for
lockdep.

Reported-by Mike Galbraith <[email protected]>
Cc [email protected]
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 include/linux/seqlock.h   |  9 ---------
 include/net/sch_generic.h | 10 +++++++++-
 2 files changed, 9 insertions(+), 10 deletions(-)
include/linux/seqlock.h +0 -9
index e5207897c33e..f390293974ea 100644
@@ -489,15 +489,6 @@ static inline void write_seqlock(seqlock_t
 *sl)
 	__raw_write_seqcount_begin(&sl->seqcount);
 }
 
-static inline int try_write_seqlock(seqlock_t *sl)
-{
-	if (spin_trylock(&sl->lock)) {
-		__raw_write_seqcount_begin(&sl->seqcount);
-		return 1;
-	}
-	return 0;
-}
-
 static inline void write_sequnlock(seqlock_t *sl)
 {
include/linux/seqlock.h +0 -0
 	__raw_write_seqcount_end(&sl->seqcount);
include/net/sch_generic.h +0 -0
index e6afb4b9cede..112d2dca8b08 100644
@@ -168,8 +168,16 @@ static inline bool qdisc_run_begin(struct

Qdisc
*qdisc)

include/net/sch_generic.h +4 -1
 		return false;
 	}
 #ifdef CONFIG_PREEMPT_RT
-	if (try_write_seqlock(&qdisc->running))
+	if (spin_trylock(&qdisc->running.lock)) {
+		seqcount_t *s = &qdisc->running.seqcount;
+		/*
+		 * Variant of write_seqcount_t_begin() telling lockdep

that a

include/net/sch_generic.h

Quoted 9 lines
include/net/sch_generic.h +5 -0
+		 * trylock was attempted.
+		 */
+		__raw_write_seqcount_begin(s);
+		seqcount_acquire(&s->dep_map, 0, 1, _RET_IP_);
 		return true;
+	}
 	return false;
 #else
 	/* Variant of write_seqcount_begin() telling lockdep a trylock

I applied this to 4.19 and saw some splat with my 'debug-full' configuration, so tried 5.4 and saw the same thing:

[   30.573698] BUG: workqueue leaked lock or atomic:
kworker/1:4/0x00000000/143
                    last function: wireless_nlevent_process
[   30.573707] 1 lock held by kworker/1:4/143:
[   30.573708]  #0: ffffffff8e981d80 (noop_qdisc.running#2){+.+.},
at: __do_softirq+0xca/0x561
[   30.573715] CPU: 1 PID: 143 Comm: kworker/1:4 Not tainted 5.4.74-
rt42-rt-test-full-debug #1
[   30.573716] Hardware name: LENOVO 4236L51/4236L51, BIOS 83ET59WW
(1.29 ) 06/01/2011
[   30.573720] Workqueue: events wireless_nlevent_process
[   30.573721] Call Trace:
[   30.573724]  dump_stack+0x71/0x9b
[   30.573728]  process_one_work+0x533/0x760
[   30.573731]  worker_thread+0x39/0x3f0
[   30.573733]  ? process_one_work+0x760/0x760
[   30.573734]  kthread+0x165/0x180
[   30.573736]  ? __kthread_create_on_node+0x180/0x180
[   30.573737]  ret_from_fork+0x3a/0x50
[   30.629329] wlp3s0: authenticate with 84:1b:5e:41:5e:4d
[   30.634864] wlp3s0: send auth to 84:1b:5e:41:5e:4d (try 1/3)
[   30.638433] wlp3s0: authenticated
[   30.642250] wlp3s0: associate with 84:1b:5e:41:5e:4d (try 1/3)
[   30.645704] wlp3s0: RX AssocResp from 84:1b:5e:41:5e:4d
(capab=0x411 status=0 aid=6)
[   30.650803] wlp3s0: associated
[   30.656764] ================================================
[   30.656765] WARNING: lock held when returning to user space!
[   30.656766] 5.4.74-rt42-rt-test-full-debug #1 Not tainted
[   30.656767] ------------------------------------------------
[   30.656768] wpa_supplicant/836 is leaving the kernel with locks
still held!
[   30.656769] 1 lock held by wpa_supplicant/836:
[   30.656770]  #0: ffff98f1c9622280 (&dev->qdisc_running_key){+.+.},
Quoted 7 lines

at: packet_sendmsg+0xec1/0x1ad0

Let me know if you want me to send you my .config or the full output (a
bunch more of the above).

Thanks,

This patch seems to fix it for me:

From 4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e Mon Sep 17 00:00:00 2001
Message-Id: <4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e.1605297603.git.zanussi@kernel.org>
From: Tom Zanussi <[email protected]>
Date: Fri, 13 Nov 2020 13:04:15 -0600
Subject: [PATCH] net: Add missing __raw_write_seqcount_end() and
seqcount_release()

The patch ('net: Properly annotate the try-lock for the seqlock") adds __raw_write_seqcount_begin() in qdisc_run_begin() but omits the corresponding __raw_write_seqcount_end() and seqcount_release() in qdisc_run_end().

Add it unconditionally, since qdisc_run_end() is never called unless qdisc_run_begin() succeeds, and if it succeeds, __raw_write_seqcount_begin() seqcount_acquire() will have been called.

Signed-off-by Tom Zanussi <[email protected]>
---
 include/net/sch_generic.h | 4 ++++
 1 file changed, 4 insertions(+)
include/net/sch_generic.h +4 -0
index 112d2dca8b08..c5ccce4f8f62 100644
@@ -192,7 +192,11 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc)
 static inline void qdisc_run_end(struct Qdisc *qdisc)
 {
 #ifdef CONFIG_PREEMPT_RT
+	seqcount_t *s = &qdisc->running.seqcount;
+
 	write_sequnlock(&qdisc->running);
+	__raw_write_seqcount_end(s);
+	seqcount_release(&s->dep_map, 1, _RET_IP_);
 #else
 	write_seqcount_end(&qdisc->running);
 #endif
Signature · 1 line
2.17.1
#4 lore raw

Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

Quoted 2 lines · Tom Zanussi

This patch seems to fix it for me:

If there was any discussion about this patch, I missed it.

Quoted 27 lines

From 4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e Mon Sep 17 00:00:00 2001
Message-Id: <4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e.1605297603.git.zanussi@kernel.org>
From: Tom Zanussi <[email protected]>
Date: Fri, 13 Nov 2020 13:04:15 -0600
Subject: [PATCH] net: Add missing __raw_write_seqcount_end() and
seqcount_release()

The patch ('net: Properly annotate the try-lock for the seqlock") adds __raw_write_seqcount_begin() in qdisc_run_begin() but omits the corresponding __raw_write_seqcount_end() and seqcount_release() in qdisc_run_end().

Add it unconditionally, since qdisc_run_end() is never called unless qdisc_run_begin() succeeds, and if it succeeds, __raw_write_seqcount_begin() seqcount_acquire() will have been called.

Signed-off-by Tom Zanussi <[email protected]>
---
 include/net/sch_generic.h | 4 ++++
 1 file changed, 4 insertions(+)
include/net/sch_generic.h +0 -0
index 112d2dca8b08..c5ccce4f8f62 100644
@@ -192,7 +192,11 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc)
 static inline void qdisc_run_end(struct Qdisc *qdisc)

include/net/sch_generic.h static inline bool qdisc_run_begin(struct Qdisc *qdisc) · line 202

Quoted 10 lines
include/net/sch_generic.h +4 -0
 {
 #ifdef CONFIG_PREEMPT_RT
+	seqcount_t *s = &qdisc->running.seqcount;
+
 	write_sequnlock(&qdisc->running);
+	__raw_write_seqcount_end(s);
+	seqcount_release(&s->dep_map, 1, _RET_IP_);
 #else
 	write_seqcount_end(&qdisc->running);
 #endif

__raw_write_seqcount_end() is an integral part of write_sequnlock(), but we do seem to be missing a seqcount_release() in 5.4-rt.

-Mike

#5 lore raw

Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

Quoted 5 lines · Mike Galbraith
Quoted 2 lines · Tom Zanussi

This patch seems to fix it for me:

If there was any discussion about this patch, I missed it.

There wasn't, just this thread.

Quoted 40 lines
Quoted 40 lines

From 4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e Mon Sep 17 00:00:00 2001
Message-Id: < 4855377d0cb34b1b67a5c6d84cc8609c9da0bc3e.1605297603.git.zanussi@kernel.org

Quoted 1 line

From: Tom Zanussi <[email protected]>
Date: Fri, 13 Nov 2020 13:04:15 -0600
Subject: [PATCH] net: Add missing __raw_write_seqcount_end() and
seqcount_release()

The patch ('net: Properly annotate the try-lock for the seqlock") adds
__raw_write_seqcount_begin() in qdisc_run_begin() but omits the corresponding __raw_write_seqcount_end() and seqcount_release() in qdisc_run_end().

Add it unconditionally, since qdisc_run_end() is never called unless
qdisc_run_begin() succeeds, and if it succeeds, __raw_write_seqcount_begin() seqcount_acquire() will have been called.

Signed-off-by Tom Zanussi <[email protected]>
---
 include/net/sch_generic.h | 4 ++++
 1 file changed, 4 insertions(+)
include/net/sch_generic.h +2 -0
index 112d2dca8b08..c5ccce4f8f62 100644
@@ -192,7 +192,11 @@ static inline bool qdisc_run_begin(struct
 Qdisc *qdisc)
 static inline void qdisc_run_end(struct Qdisc *qdisc)
 {
 #ifdef CONFIG_PREEMPT_RT
+	seqcount_t *s = &qdisc->running.seqcount;
+
 	write_sequnlock(&qdisc->running);
Quoted 9 lines
Quoted 5 lines
include/net/sch_generic.h +2 -0
+	__raw_write_seqcount_end(s);
+	seqcount_release(&s->dep_map, 1, _RET_IP_);
 #else
 	write_seqcount_end(&qdisc->running);

#endif

__raw_write_seqcount_end() is an integral part of write_sequnlock(), but we do seem to be missing a seqcount_release() in 5.4-rt.

Yep, you're right, it's just the missing seqcount_release() - I'll resubmit with just that.

Thanks,

Tom

Quoted 2 lines

-Mike

#6 lore raw

Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

Quoted 8 lines · Tom Zanussi
Quoted 3 lines · Mike Galbraith

__raw_write_seqcount_end() is an integral part of write_sequnlock(), but we do seem to be missing a seqcount_release() in 5.4-rt.

Yep, you're right, it's just the missing seqcount_release() - I'll resubmit with just that.

Or just drop the backport, since it adds annotation, while the original was fixing existing annotation.

__raw_write_seqcount_begin() called in 5.4-rt try_write_seqlock() is not annotated, while write_seqcount_begin() called by the 5.9-rt version leads to the broken annotation that the original then fixed.

-Mike

#7 lore raw

Re: [PATCH RT 1/5] net: Properly annotate the try-lock for the seqlock

Quoted 10 lines · Mike Galbraith
Quoted 8 lines · Tom Zanussi
Quoted 3 lines · Mike Galbraith

__raw_write_seqcount_end() is an integral part of write_sequnlock(), but we do seem to be missing a seqcount_release() in 5.4-rt.

Yep, you're right, it's just the missing seqcount_release() - I'll resubmit with just that.

Quoted 6 lines

Or just drop the backport, since it adds annotation, while the original was fixing existing annotation.

__raw_write_seqcount_begin() called in 5.4-rt try_write_seqlock() is not annotated, while write_seqcount_begin() called by the 5.9-rt version leads to the broken annotation that the original then fixed.

That is correct.
I was looking at the 5.4-RT series Steven posted and I was under the impression that this patch was correctly missing in previous RT since I even added the stable tag.
As Mike said, the previous RT implementation did not use seqlock annotation, they used a spin-lock instead. So the "try_write_seqlock()" did the try-lock annotation.
With the reworked seqcount implementation (v5.6-RT time frame) this was solved differently (closer to what upstream does) and the now the annotation was wrong and fixed.

Sorry for that.

Quoted 1 line

-Mike

Sebastian

author
#8 lore raw

PATCH 2/5 mm/memcontrol: Disable preemption in __mod_memcg_lruvec_state()

In reply to Steven Rostedt

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

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

From: Sebastian Andrzej Siewior <[email protected]>

The callers expect disabled preemption/interrupts while invoking __mod_memcg_lruvec_state(). This works mainline because a lock of somekind is acquired.

Use preempt_disable_rt() where per-CPU variables are accessed and a stable pointer is expected. This is also done in __mod_zone_page_state() for the same reason.

Cc [email protected]
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 mm/memcontrol.c | 2 ++
 1 file changed, 2 insertions(+)
mm/memcontrol.c +2 -0
index 9bdb75ef6d62..c9d02e2272e1 100644
@@ -752,6 +752,7 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
 	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
 	memcg = pn->memcg;
 
+	preempt_disable_rt();
 	/* Update memcg */
 	__mod_memcg_state(memcg, idx, val);
 

@@ -767,6 +768,7 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
 		x = 0;
 	}
 	__this_cpu_write(pn->lruvec_stat_cpu->count[idx], x);
+	preempt_enable_rt();
 }
 
 void __mod_lruvec_slab_state(void *p, enum node_stat_item idx, int val)
Signature · 1 line
2.28.0
author
#9 lore raw

PATCH 3/5 ptrace: fix ptrace_unfreeze_traced() race with rt-lock

In reply to Steven Rostedt

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

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

From: Oleg Nesterov <[email protected]>

The patch "ptrace: fix ptrace vs tasklist_lock race" changed ptrace_freeze_traced() to take task->saved_state into account, but ptrace_unfreeze_traced() has the same problem and needs a similar fix:
it should check/update both ->state and ->saved_state.

Reported-by Luis Claudio R. Goncalves <[email protected]>
Fixes "ptrace: fix ptrace vs tasklist_lock race"
Signed-off-by Oleg Nesterov <[email protected]>
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
Cc [email protected]
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 kernel/ptrace.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)
kernel/ptrace.c +15 -8
index 3075006d720e..3f7156f06b6c 100644
@@ -197,8 +197,8 @@ static bool ptrace_freeze_traced(struct task_struct *task)
 
 static void ptrace_unfreeze_traced(struct task_struct *task)
 {
-	if (task->state != __TASK_TRACED)
-		return;
+	unsigned long flags;
+	bool frozen = true;
 
 	WARN_ON(!task->ptrace || task->parent != current);
 

@@ -207,12 +207,19 @@ static void ptrace_unfreeze_traced(struct task_struct *task)
 	 * Recheck state under the lock to close this race.
 	 */
 	spin_lock_irq(&task->sighand->siglock);
-	if (task->state == __TASK_TRACED) {
-		if (__fatal_signal_pending(task))
-			wake_up_state(task, __TASK_TRACED);
-		else
-			task->state = TASK_TRACED;
-	}
+
+	raw_spin_lock_irqsave(&task->pi_lock, flags);
+	if (task->state == __TASK_TRACED)
+		task->state = TASK_TRACED;
+	else if (task->saved_state == __TASK_TRACED)
+		task->saved_state = TASK_TRACED;
+	else
+		frozen = false;
+	raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+
+	if (frozen && __fatal_signal_pending(task))
+		wake_up_state(task, __TASK_TRACED);
+
 	spin_unlock_irq(&task->sighand->siglock);
 }
Signature · 1 line
2.28.0
author
#10 lore raw

PATCH 4/5 timers: Dont block on ->expiry_lock for TIMER_IRQSAFE

In reply to Steven Rostedt

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

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

From: Sebastian Andrzej Siewior <[email protected]>

PREEMPT_RT does not spin and wait until a running timer completes its callback but instead it blocks on a sleeping lock to prevent a deadlock.

This blocking can not be done for workqueue's IRQ_SAFE timer which will be canceled in an IRQ-off region. It has to happen to in IRQ-off region because changing the PENDING bit and clearing the timer must not be interrupted to avoid a busy-loop.

The callback invocation of IRQSAFE timer is not preempted on PREEMPT_RT so there is no need to synchronize on timer_base::expiry_lock.

Don't acquire the timer_base::expiry_lock for TIMER_IRQSAFE flagged timer.
Add a lockdep annotation to ensure that this function is always invoked in preemptible context on PREEMPT_RT.

Reported-by Mike Galbraith <[email protected]>
Signed-off-by Sebastian Andrzej Siewior <[email protected]>
Cc [email protected]
Signed-off-by Steven Rostedt (VMware) <[email protected]>
---
 kernel/time/timer.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
kernel/time/timer.c +8 -1
index 89078fd848b9..3e9d7f227a5c 100644
@@ -1289,7 +1289,7 @@ static void del_timer_wait_running(struct timer_list *timer)
 	u32 tf;
 
 	tf = READ_ONCE(timer->flags);
-	if (!(tf & TIMER_MIGRATING)) {
+	if (!(tf & (TIMER_MIGRATING | TIMER_IRQSAFE))) {
 		struct timer_base *base = get_timer_base(tf);
 
 		/*

@@ -1373,6 +1373,13 @@ int del_timer_sync(struct timer_list *timer)
 	 */
 	WARN_ON(in_irq() && !(timer->flags & TIMER_IRQSAFE));
 
+	/*
+	 * Must be able to sleep on PREEMPT_RT because of the slowpath in
+	 * del_timer_wait_running().
+	 */
+	if (IS_ENABLED(CONFIG_PREEMPT_RT) && !(timer->flags & TIMER_IRQSAFE))
+		might_sleep();
+
 	do {
 		ret = try_to_del_timer_sync(timer);
Signature · 1 line
2.28.0
author
#11 lore raw

PATCH 5/5 Linux 5.4.74-rt42-rc2

In reply to Steven Rostedt

5.4.74-rt42-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 629e0b4384b8..9338de9953e3 100644
@@ -1 +1 @@
--rt41
+-rt42-rc2
Signature · 1 line
2.28.0