1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
|
From f9bcdc9de0ced2b99b351e226344eb1c0246dd5c Mon Sep 17 00:00:00 2001
From: Zhang Xiao <xiao.zhang@windriver.com>
Date: Tue, 17 Mar 2020 12:47:43 +0100
Subject: [PATCH 324/347] tasklet: Address a race resulting in double-enqueue
Origin: https://www.kernel.org/pub/linux/kernel/projects/rt/4.19/older/patches-4.19.246-rt110.tar.xz
The kernel bugzilla has the following race condition reported:
CPU0 CPU1 CPU2
------------------------------------------------
test_set SCHED
test_set RUN
if SCHED
add_list
raise
clear RUN
<softirq>
test_set RUN
test_clear SCHED
->func
test_set SCHED
tasklet_try_unlock ->0
test_clear SCHED
test_set SCHED
->func
tasklet_try_unlock ->1
test_set RUN
if SCHED
add list
raise
clear RUN
test_set RUN
if SCHED
add list
raise
clear RUN
As a result the tasklet is enqueued on both CPUs and run on both CPUs. Due
to the nature of the list used here, it is possible that further
(different) tasklets, which are enqueued after this double-enqueued
tasklet, are scheduled on CPU2 but invoked on CPU1. It is also possible
that these tasklets won't be invoked at all, because during the second
enqueue process the t->next pointer is set to NULL - dropping everything
from the list.
This race will trigger one or two of the WARN_ON() in
tasklet_action_common().
The problem is that the tasklet may be invoked multiple times and clear
SCHED bit on each invocation. This makes it possible to enqueue the
very same tasklet on different CPUs.
Current RT-devel is using the upstream implementation which does not
re-run tasklets if they have SCHED set again and so it does not clear
the SCHED bit multiple times on a single invocation.
Introduce the CHAINED flag. The tasklet will only be enqueued if the
CHAINED flag has been set successfully.
If it is possible to exchange the flags (CHAINED | RUN) -> 0 then the
tasklet won't be re-run. Otherwise the possible SCHED flag is removed
and the tasklet is re-run again.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61451
Not-signed-off-by: Zhang Xiao <xiao.zhang@windriver.com>
[bigeasy: patch description]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Tom Zanussi <zanussi@kernel.org>
---
include/linux/interrupt.h | 5 ++++-
kernel/softirq.c | 6 +++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 97d9ba26915e..a3b5edb26bc5 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -579,12 +579,15 @@ enum
{
TASKLET_STATE_SCHED, /* Tasklet is scheduled for execution */
TASKLET_STATE_RUN, /* Tasklet is running (SMP only) */
- TASKLET_STATE_PENDING /* Tasklet is pending */
+ TASKLET_STATE_PENDING, /* Tasklet is pending */
+ TASKLET_STATE_CHAINED /* Tasklet is chained */
};
#define TASKLET_STATEF_SCHED (1 << TASKLET_STATE_SCHED)
#define TASKLET_STATEF_RUN (1 << TASKLET_STATE_RUN)
#define TASKLET_STATEF_PENDING (1 << TASKLET_STATE_PENDING)
+#define TASKLET_STATEF_CHAINED (1 << TASKLET_STATE_CHAINED)
+#define TASKLET_STATEF_RC (TASKLET_STATEF_RUN | TASKLET_STATEF_CHAINED)
#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_RT_FULL)
static inline int tasklet_trylock(struct tasklet_struct *t)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 25bcf2f2714b..73dae64bfc9c 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -947,6 +947,10 @@ static void __tasklet_schedule_common(struct tasklet_struct *t,
* is locked before adding it to the list.
*/
if (test_bit(TASKLET_STATE_SCHED, &t->state)) {
+ if (test_and_set_bit(TASKLET_STATE_CHAINED, &t->state)) {
+ tasklet_unlock(t);
+ return;
+ }
t->next = NULL;
*head->tail = t;
head->tail = &(t->next);
@@ -1040,7 +1044,7 @@ static void tasklet_action_common(struct softirq_action *a,
again:
t->func(t->data);
- while (!tasklet_tryunlock(t)) {
+ while (cmpxchg(&t->state, TASKLET_STATEF_RC, 0) != TASKLET_STATEF_RC) {
/*
* If it got disabled meanwhile, bail out:
*/
--
2.36.1
|
