Hi,
In v3 we had some discussion concerning the open coded wait loop in
arch/powerpc/kvm/book3s_hv.c. If I understood it correctly, the
current version is okay though it wouldn't hurt to address the open
coded style eventually. Since I can't really test it and it looks
fragile I left it as it is.
swake_up_locked() is now available as non loop version as requested
by Boqun.
The API naming discussion faded out and the decision is on Peter. I
assume since he wrote it initially it stays as it is.
There is now a new patch which tries to address the compile time type
check assertion. I added the checks to a couple of existing macros and
not on the complete API. If we want this we need to add macro for all
function calls I guess.
Obviously, I have rebased it and this time it is against tip/sched/core.
I noticed there are some smaller code refactorings ongoing, so this
version is not going to apply cleanly against mainline. There is
nothing particularly difficult to fix up though. In case someone
is interested I can post a version of it.
These patches are against
tip/sched/core e73e85f0593832aa583b252f9a16cf90ed6d30fa
also available as git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/wagi/linux.git tip-swait
cheers,
daniel
changes since v3
- rebased it on tip/sched/core (KVM bits have changed slightly)
- added compile time type check assertion
- added non lazy version of swake_up_locked()
changes since v2
- rebased again on tip/master. The patches apply
cleanly on v4.3-rc6 too.
- fixed up mips
- reordered patches to avoid lockdep warning when doing bissect.
- remove unnecessary initialization of rsp->rda in rcu_init_one().
changes since v1 (PATCH v0)
- rebased and fixed some typos found by cross building
for S390, ARM and powerpc. For some unknown reason didn't catch
them last time.
- dropped completion patches because it is not clear yet
how to handle complete_all() calls hard-irq/atomic contexts
and swake_up_all.
changes since v0 (RFC v0)
- promoted the series to PATCH state instead of RFC
- fixed a few fallouts with build all and some cross compilers
such ARM, PowerPC, S390.
- Added the simple waitqueue transformation for KVM from -rt
including some numbers requested by Paolo.
- Added a commit message to PeterZ's patch. Hope he likes it.
[I got the numbering wrong in v1, so instead 'PATCH v1' you find it
as 'PATCH v0' series]
v3: https://lwn.net/Articles/661415/
v2: https://lwn.net/Articles/660628/
v1: https://lwn.net/Articles/656942/
v0: https://lwn.net/Articles/653586/
Daniel Wagner (2):
[s]wait: Add compile time type check assertion
rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock
Marcelo Tosatti (1):
KVM: use simple waitqueue for vcpu->wq
Paul Gortmaker (1):
rcu: use simple wait queues where possible in rcutree
Peter Zijlstra (Intel) (1):
wait.[ch]: Introduce the simple waitqueue (swait) implementation
arch/arm/kvm/arm.c | 4 +-
arch/arm/kvm/psci.c | 4 +-
arch/mips/kvm/mips.c | 8 +-
arch/powerpc/include/asm/kvm_host.h | 4 +-
arch/powerpc/kvm/book3s_hv.c | 23 +++--
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kvm/interrupt.c | 8 +-
arch/x86/kvm/lapic.c | 6 +-
include/linux/compiler.h | 4 +
include/linux/kvm_host.h | 5 +-
include/linux/swait.h | 174 ++++++++++++++++++++++++++++++++++++
include/linux/wait.h | 2 +
kernel/rcu/tree.c | 24 ++---
kernel/rcu/tree.h | 12 +--
kernel/rcu/tree_plugin.h | 32 ++++---
kernel/sched/Makefile | 2 +-
kernel/sched/swait.c | 123 +++++++++++++++++++++++++
virt/kvm/async_pf.c | 4 +-
virt/kvm/kvm_main.c | 17 ++--
19 files changed, 387 insertions(+), 71 deletions(-)
create mode 100644 include/linux/swait.h
create mode 100644 kernel/sched/swait.c
--
2.4.3
From: "Peter Zijlstra (Intel)" <[email protected]>
The existing wait queue support has support for custom wake up call
backs, wake flags, wake key (passed to call back) and exclusive
flags that allow wakers to be tagged as exclusive, for limiting
the number of wakers.
In a lot of cases, none of these features are used, and hence we
can benefit from a slimmed down version that lowers memory overhead
and reduces runtime overhead.
The concept originated from -rt, where waitqueues are a constant
source of trouble, as we can't convert the head lock to a raw
spinlock due to fancy and long lasting callbacks.
With the removal of custom callbacks, we can use a raw lock for
queue list manipulations, hence allowing the simple wait support
to be used in -rt.
Signed-off-by: Daniel Wagner <[email protected]>
Mostly-Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Originally-by: Thomas Gleixner <[email protected]>
Cc: Paul Gortmaker <[email protected]>
Cc: [email protected]
[Patch is from PeterZ which is based on Thomas version.
Commit message is written by Paul G.
Daniel:
- And some compile issues fixed.
- Added non-lazy implementation of swake_up_locked as suggested by Boqun Feng.]
---
include/linux/swait.h | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++
kernel/sched/Makefile | 2 +-
kernel/sched/swait.c | 123 ++++++++++++++++++++++++++++++++++++
3 files changed, 296 insertions(+), 1 deletion(-)
create mode 100644 include/linux/swait.h
create mode 100644 kernel/sched/swait.c
diff --git a/include/linux/swait.h b/include/linux/swait.h
new file mode 100644
index 0000000..c1f9c62
--- /dev/null
+++ b/include/linux/swait.h
@@ -0,0 +1,172 @@
+#ifndef _LINUX_SWAIT_H
+#define _LINUX_SWAIT_H
+
+#include <linux/list.h>
+#include <linux/stddef.h>
+#include <linux/spinlock.h>
+#include <asm/current.h>
+
+/*
+ * Simple wait queues
+ *
+ * While these are very similar to the other/complex wait queues (wait.h) the
+ * most important difference is that the simple waitqueue allows for
+ * deterministic behaviour -- IOW it has strictly bounded IRQ and lock hold
+ * times.
+ *
+ * In order to make this so, we had to drop a fair number of features of the
+ * other waitqueue code; notably:
+ *
+ * - mixing INTERRUPTIBLE and UNINTERRUPTIBLE sleeps on the same waitqueue;
+ * all wakeups are TASK_NORMAL in order to avoid O(n) lookups for the right
+ * sleeper state.
+ *
+ * - the exclusive mode; because this requires preserving the list order
+ * and this is hard.
+ *
+ * - custom wake functions; because you cannot give any guarantees about
+ * random code.
+ *
+ * As a side effect of this; the data structures are slimmer.
+ *
+ * One would recommend using this wait queue where possible.
+ */
+
+struct task_struct;
+
+struct swait_queue_head {
+ raw_spinlock_t lock;
+ struct list_head task_list;
+};
+
+struct swait_queue {
+ struct task_struct *task;
+ struct list_head task_list;
+};
+
+#define __SWAITQUEUE_INITIALIZER(name) { \
+ .task = current, \
+ .task_list = LIST_HEAD_INIT((name).task_list), \
+}
+
+#define DECLARE_SWAITQUEUE(name) \
+ struct swait_queue name = __SWAITQUEUE_INITIALIZER(name)
+
+#define __SWAIT_QUEUE_HEAD_INITIALIZER(name) { \
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \
+ .task_list = LIST_HEAD_INIT((name).task_list), \
+}
+
+#define DECLARE_SWAIT_QUEUE_HEAD(name) \
+ struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INITIALIZER(name)
+
+extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+ struct lock_class_key *key);
+
+#define init_swait_queue_head(q) \
+ do { \
+ static struct lock_class_key __key; \
+ __init_swait_queue_head((q), #q, &__key); \
+ } while (0)
+
+#ifdef CONFIG_LOCKDEP
+# define __SWAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
+ ({ init_swait_queue_head(&name); name; })
+# define DECLARE_SWAIT_QUEUE_HEAD_ONSTACK(name) \
+ struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INIT_ONSTACK(name)
+#else
+# define DECLARE_SWAIT_QUEUE_HEAD_ONSTACK(name) \
+ DECLARE_SWAIT_QUEUE_HEAD(name)
+#endif
+
+static inline int swait_active(struct swait_queue_head *q)
+{
+ return !list_empty(&q->task_list);
+}
+
+extern void swake_up(struct swait_queue_head *q);
+extern void swake_up_all(struct swait_queue_head *q);
+extern void swake_up_locked(struct swait_queue_head *q);
+
+extern void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state);
+extern long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state);
+
+extern void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+
+/* as per ___wait_event() but for swait, therefore "exclusive == 0" */
+#define ___swait_event(wq, condition, state, ret, cmd) \
+({ \
+ struct swait_queue __wait; \
+ long __ret = ret; \
+ \
+ INIT_LIST_HEAD(&__wait.task_list); \
+ for (;;) { \
+ long __int = prepare_to_swait_event(&wq, &__wait, state);\
+ \
+ if (condition) \
+ break; \
+ \
+ if (___wait_is_interruptible(state) && __int) { \
+ __ret = __int; \
+ break; \
+ } \
+ \
+ cmd; \
+ } \
+ finish_swait(&wq, &__wait); \
+ __ret; \
+})
+
+#define __swait_event(wq, condition) \
+ (void)___swait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0, \
+ schedule())
+
+#define swait_event(wq, condition) \
+do { \
+ if (condition) \
+ break; \
+ __swait_event(wq, condition); \
+} while (0)
+
+#define __swait_event_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_UNINTERRUPTIBLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_timeout(wq, condition, timeout); \
+ __ret; \
+})
+
+#define __swait_event_interruptible(wq, condition) \
+ ___swait_event(wq, condition, TASK_INTERRUPTIBLE, 0, \
+ schedule())
+
+#define swait_event_interruptible(wq, condition) \
+({ \
+ int __ret = 0; \
+ if (!(condition)) \
+ __ret = __swait_event_interruptible(wq, condition); \
+ __ret; \
+})
+
+#define __swait_event_interruptible_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_INTERRUPTIBLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_interruptible_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_interruptible_timeout(wq, \
+ condition, timeout); \
+ __ret; \
+})
+
+#endif /* _LINUX_SWAIT_H */
diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile
index 6768797..7d4cba2 100644
--- a/kernel/sched/Makefile
+++ b/kernel/sched/Makefile
@@ -13,7 +13,7 @@ endif
obj-y += core.o loadavg.o clock.o cputime.o
obj-y += idle_task.o fair.o rt.o deadline.o stop_task.o
-obj-y += wait.o completion.o idle.o
+obj-y += wait.o swait.o completion.o idle.o
obj-$(CONFIG_SMP) += cpupri.o cpudeadline.o
obj-$(CONFIG_SCHED_AUTOGROUP) += auto_group.o
obj-$(CONFIG_SCHEDSTATS) += stats.o
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
new file mode 100644
index 0000000..82f0dff
--- /dev/null
+++ b/kernel/sched/swait.c
@@ -0,0 +1,123 @@
+#include <linux/sched.h>
+#include <linux/swait.h>
+
+void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+ struct lock_class_key *key)
+{
+ raw_spin_lock_init(&q->lock);
+ lockdep_set_class_and_name(&q->lock, key, name);
+ INIT_LIST_HEAD(&q->task_list);
+}
+EXPORT_SYMBOL(__init_swait_queue_head);
+
+/*
+ * The thing about the wake_up_state() return value; I think we can ignore it.
+ *
+ * If for some reason it would return 0, that means the previously waiting
+ * task is already running, so it will observe condition true (or has already).
+ */
+void swake_up_locked(struct swait_queue_head *q)
+{
+ struct swait_queue *curr;
+
+ if (list_empty(&q->task_list))
+ return;
+
+ curr = list_first_entry(&q->task_list, typeof(*curr), task_list);
+ wake_up_process(curr->task);
+ list_del_init(&curr->task_list);
+}
+EXPORT_SYMBOL(swake_up_locked);
+
+void swake_up(struct swait_queue_head *q)
+{
+ unsigned long flags;
+
+ if (!swait_active(q))
+ return;
+
+ raw_spin_lock_irqsave(&q->lock, flags);
+ swake_up_locked(q);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+}
+EXPORT_SYMBOL(swake_up);
+
+/*
+ * Does not allow usage from IRQ disabled, since we must be able to
+ * release IRQs to guarantee bounded hold time.
+ */
+void swake_up_all(struct swait_queue_head *q)
+{
+ struct swait_queue *curr;
+ LIST_HEAD(tmp);
+
+ if (!swait_active(q))
+ return;
+
+ raw_spin_lock_irq(&q->lock);
+ list_splice_init(&q->task_list, &tmp);
+ while (!list_empty(&tmp)) {
+ curr = list_first_entry(&tmp, typeof(*curr), task_list);
+
+ wake_up_state(curr->task, TASK_NORMAL);
+ list_del_init(&curr->task_list);
+
+ if (list_empty(&tmp))
+ break;
+
+ raw_spin_unlock_irq(&q->lock);
+ raw_spin_lock_irq(&q->lock);
+ }
+ raw_spin_unlock_irq(&q->lock);
+}
+EXPORT_SYMBOL(swake_up_all);
+
+void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ wait->task = current;
+ if (list_empty(&wait->task_list))
+ list_add(&wait->task_list, &q->task_list);
+}
+
+void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&q->lock, flags);
+ __prepare_to_swait(q, wait);
+ set_current_state(state);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+}
+EXPORT_SYMBOL(prepare_to_swait);
+
+long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
+{
+ if (signal_pending_state(state, current))
+ return -ERESTARTSYS;
+
+ prepare_to_swait(q, wait, state);
+
+ return 0;
+}
+EXPORT_SYMBOL(prepare_to_swait_event);
+
+void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ __set_current_state(TASK_RUNNING);
+ if (!list_empty(&wait->task_list))
+ list_del_init(&wait->task_list);
+}
+
+void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ unsigned long flags;
+
+ __set_current_state(TASK_RUNNING);
+
+ if (!list_empty_careful(&wait->task_list)) {
+ raw_spin_lock_irqsave(&q->lock, flags);
+ list_del_init(&wait->task_list);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+ }
+}
+EXPORT_SYMBOL(finish_swait);
--
2.4.3
The API provided by wait.h and swait.h is very similiar. Most of the
time your are only one character away from either of it:
wake_up() vs swake_up()
This is on purpose so that we do not have two nearly identical bits of
infrastructre code with dissimilar names.
A compile time type check assertion ensures that obvious wrong usage
is caught at early stage.
Signed-off-by: Daniel Wagner <[email protected]>
---
include/linux/compiler.h | 4 ++++
include/linux/swait.h | 2 ++
include/linux/wait.h | 2 ++
3 files changed, 8 insertions(+)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c836eb2..ac7afcb 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -455,6 +455,10 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
compiletime_assert(__native_word(t), \
"Need native word sized stores/loads for atomicity.")
+#define compiletime_assert_same_type(a, b) \
+ compiletime_assert(__same_type(a, b), \
+ "Need same type.");
+
/*
* Prevent the compiler from merging or refetching accesses. The compiler
* is also forbidden from reordering successive instances of ACCESS_ONCE(),
diff --git a/include/linux/swait.h b/include/linux/swait.h
index c1f9c62..80e2eb8 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -66,6 +66,7 @@ extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name
#define init_swait_queue_head(q) \
do { \
static struct lock_class_key __key; \
+ compiletime_assert_same_type(struct swait_queue_head *, q); \
__init_swait_queue_head((q), #q, &__key); \
} while (0)
@@ -101,6 +102,7 @@ extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
struct swait_queue __wait; \
long __ret = ret; \
\
+ compiletime_assert_same_type(struct swait_queue_head, wq); \
INIT_LIST_HEAD(&__wait.task_list); \
for (;;) { \
long __int = prepare_to_swait_event(&wq, &__wait, state);\
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1e1bf9f..bc4f829 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -75,6 +75,7 @@ extern void __init_waitqueue_head(wait_queue_head_t *q, const char *name, struct
do { \
static struct lock_class_key __key; \
\
+ compiletime_assert_same_type(wait_queue_head_t *, q); \
__init_waitqueue_head((q), #q, &__key); \
} while (0)
@@ -215,6 +216,7 @@ wait_queue_head_t *bit_waitqueue(void *, int);
wait_queue_t __wait; \
long __ret = ret; /* explicit shadow */ \
\
+ compiletime_assert_same_type(wait_queue_head_t, wq); \
INIT_LIST_HEAD(&__wait.task_list); \
if (exclusive) \
__wait.flags = WQ_FLAG_EXCLUSIVE; \
--
2.4.3
From: Marcelo Tosatti <[email protected]>
The problem:
On -rt, an emulated LAPIC timer instances has the following path:
1) hard interrupt
2) ksoftirqd is scheduled
3) ksoftirqd wakes up vcpu thread
4) vcpu thread is scheduled
This extra context switch introduces unnecessary latency in the
LAPIC path for a KVM guest.
The solution:
Allow waking up vcpu thread from hardirq context,
thus avoiding the need for ksoftirqd to be scheduled.
Normal waitqueues make use of spinlocks, which on -RT
are sleepable locks. Therefore, waking up a waitqueue
waiter involves locking a sleeping lock, which
is not allowed from hard interrupt context.
cyclictest command line:
This patch reduces the average latency in my tests from 14us to 11us.
Daniel writes:
Paolo asked for numbers from kvm-unit-tests/tscdeadline_latency
benchmark on mainline. The test was run 382, respectively 300 times:
./x86-run x86/tscdeadline_latency.flat -cpu host
with idle=poll.
The test seems not to deliver really stable numbers though most of
them are smaller. Paolo write:
"Anything above ~10000 cycles means that the host went to C1 or
lower---the number means more or less nothing in that case.
The mean shows an improvement indeed."
Before:
min max mean std
count 382.000000 382.000000 382.000000 382.000000
mean 6068.552356 269502.528796 8056.016198 3912.128273
std 707.404966 848866.474783 1062.472704 9835.891707
min 2335.000000 29828.000000 7337.426000 445.738750
25% 6004.500000 44237.500000 7471.094250 1078.834837
50% 6372.000000 64175.000000 7663.133700 1783.172446
75% 6465.500000 150384.500000 8210.771900 2759.734524
max 6886.000000 10188451.000000 15466.434000 120469.205668
After
min max mean std
count 300.000000 300.000000 300.000000 300.000000
mean 5618.380000 217464.786667 7745.545114 3258.483272
std 824.719741 516371.888369 847.391685 5632.943904
min 3494.000000 31410.000000 7083.574800 438.445477
25% 4937.000000 45446.000000 7214.102850 1045.536261
50% 6118.000000 67023.000000 7417.330800 1699.574075
75% 6224.000000 134191.500000 7871.625600 2809.536185
max 6654.000000 4570896.000000 13528.788600 52206.226799
[Patch was originaly based on the swait implementation found in the -rt
tree. Daniel ported it to mainline's version and gathered the
benchmark numbers for tscdeadline_latency test.]
Signed-off-by: Daniel Wagner <[email protected]>
Cc: Marcelo Tosatti <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: [email protected]
---
arch/arm/kvm/arm.c | 4 ++--
arch/arm/kvm/psci.c | 4 ++--
arch/mips/kvm/mips.c | 8 ++++----
arch/powerpc/include/asm/kvm_host.h | 4 ++--
arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++------------
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kvm/interrupt.c | 8 ++++----
arch/x86/kvm/lapic.c | 6 +++---
include/linux/kvm_host.h | 5 +++--
virt/kvm/async_pf.c | 4 ++--
virt/kvm/kvm_main.c | 17 ++++++++---------
11 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index dc017ad..97e8336 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -470,9 +470,9 @@ bool kvm_arch_intc_initialized(struct kvm *kvm)
static void vcpu_pause(struct kvm_vcpu *vcpu)
{
- wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
+ struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
- wait_event_interruptible(*wq, !vcpu->arch.pause);
+ swait_event_interruptible(*wq, !vcpu->arch.pause);
}
static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index ad6f642..2b93577 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -70,7 +70,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
{
struct kvm *kvm = source_vcpu->kvm;
struct kvm_vcpu *vcpu = NULL;
- wait_queue_head_t *wq;
+ struct swait_queue_head *wq;
unsigned long cpu_id;
unsigned long context_id;
phys_addr_t target_pc;
@@ -119,7 +119,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
smp_mb(); /* Make sure the above is visible */
wq = kvm_arch_vcpu_wq(vcpu);
- wake_up_interruptible(wq);
+ swake_up(wq);
return PSCI_RET_SUCCESS;
}
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 49ff3bf..290161d 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -442,8 +442,8 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
dvcpu->arch.wait = 0;
- if (waitqueue_active(&dvcpu->wq))
- wake_up_interruptible(&dvcpu->wq);
+ if (swait_active(&dvcpu->wq))
+ swake_up(&dvcpu->wq);
return 0;
}
@@ -1171,8 +1171,8 @@ static void kvm_mips_comparecount_func(unsigned long data)
kvm_mips_callbacks->queue_timer_int(vcpu);
vcpu->arch.wait = 0;
- if (waitqueue_active(&vcpu->wq))
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq))
+ swake_up(&vcpu->wq);
}
/* low level hrtimer wake routine */
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 827a38d..12e9835 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -286,7 +286,7 @@ struct kvmppc_vcore {
struct list_head runnable_threads;
struct list_head preempt_list;
spinlock_t lock;
- wait_queue_head_t wq;
+ struct swait_queue_head wq;
spinlock_t stoltb_lock; /* protects stolen_tb and preempt_tb */
u64 stolen_tb;
u64 preempt_tb;
@@ -628,7 +628,7 @@ struct kvm_vcpu_arch {
u8 prodded;
u32 last_inst;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
struct kvmppc_vcore *vcore;
int ret;
int trap;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2280497..f534e15 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -121,11 +121,11 @@ static bool kvmppc_ipi_thread(int cpu)
static void kvmppc_fast_vcpu_kick_hv(struct kvm_vcpu *vcpu)
{
int cpu;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
wqp = kvm_arch_vcpu_wq(vcpu);
- if (waitqueue_active(wqp)) {
- wake_up_interruptible(wqp);
+ if (swait_active(wqp)) {
+ swake_up(wqp);
++vcpu->stat.halt_wakeup;
}
@@ -708,8 +708,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
tvcpu->arch.prodded = 1;
smp_mb();
if (vcpu->arch.ceded) {
- if (waitqueue_active(&vcpu->wq)) {
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq)) {
+ swake_up(&vcpu->wq);
vcpu->stat.halt_wakeup++;
}
}
@@ -1448,7 +1448,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
INIT_LIST_HEAD(&vcore->runnable_threads);
spin_lock_init(&vcore->lock);
spin_lock_init(&vcore->stoltb_lock);
- init_waitqueue_head(&vcore->wq);
+ init_swait_queue_head(&vcore->wq);
vcore->preempt_tb = TB_NIL;
vcore->lpcr = kvm->arch.lpcr;
vcore->first_vcpuid = core * threads_per_subcore;
@@ -2560,10 +2560,9 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
{
struct kvm_vcpu *vcpu;
int do_sleep = 1;
+ DECLARE_SWAITQUEUE(wait);
- DEFINE_WAIT(wait);
-
- prepare_to_wait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
+ prepare_to_swait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
/*
* Check one last time for pending exceptions and ceded state after
@@ -2577,7 +2576,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
}
if (!do_sleep) {
- finish_wait(&vc->wq, &wait);
+ finish_swait(&vc->wq, &wait);
return;
}
@@ -2585,7 +2584,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
trace_kvmppc_vcore_blocked(vc, 0);
spin_unlock(&vc->lock);
schedule();
- finish_wait(&vc->wq, &wait);
+ finish_swait(&vc->wq, &wait);
spin_lock(&vc->lock);
vc->vcore_state = VCORE_INACTIVE;
trace_kvmppc_vcore_blocked(vc, 1);
@@ -2641,7 +2640,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
kvmppc_start_thread(vcpu, vc);
trace_kvm_guest_enter(vcpu);
} else if (vc->vcore_state == VCORE_SLEEPING) {
- wake_up(&vc->wq);
+ swake_up(&vc->wq);
}
}
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 8ced426..a044ddb 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -427,7 +427,7 @@ struct kvm_s390_irq_payload {
struct kvm_s390_local_interrupt {
spinlock_t lock;
struct kvm_s390_float_interrupt *float_int;
- wait_queue_head_t *wq;
+ struct swait_queue_head *wq;
atomic_t *cpuflags;
DECLARE_BITMAP(sigp_emerg_pending, KVM_MAX_VCPUS);
struct kvm_s390_irq_payload irq;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 5c2c169..78625fa 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -884,13 +884,13 @@ no_timer:
void kvm_s390_vcpu_wakeup(struct kvm_vcpu *vcpu)
{
- if (waitqueue_active(&vcpu->wq)) {
+ if (swait_active(&vcpu->wq)) {
/*
* The vcpu gave up the cpu voluntarily, mark it as a good
* yield-candidate.
*/
vcpu->preempted = true;
- wake_up_interruptible(&vcpu->wq);
+ swake_up(&vcpu->wq);
vcpu->stat.halt_wakeup++;
}
}
@@ -994,7 +994,7 @@ int kvm_s390_inject_program_int(struct kvm_vcpu *vcpu, u16 code)
spin_lock(&li->lock);
irq.u.pgm.code = code;
__inject_prog(vcpu, &irq);
- BUG_ON(waitqueue_active(li->wq));
+ BUG_ON(swait_active(li->wq));
spin_unlock(&li->lock);
return 0;
}
@@ -1009,7 +1009,7 @@ int kvm_s390_inject_prog_irq(struct kvm_vcpu *vcpu,
spin_lock(&li->lock);
irq.u.pgm = *pgm_info;
rc = __inject_prog(vcpu, &irq);
- BUG_ON(waitqueue_active(li->wq));
+ BUG_ON(swait_active(li->wq));
spin_unlock(&li->lock);
return rc;
}
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 8d9013c..a59aead 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1117,7 +1117,7 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
static void apic_timer_expired(struct kvm_lapic *apic)
{
struct kvm_vcpu *vcpu = apic->vcpu;
- wait_queue_head_t *q = &vcpu->wq;
+ struct swait_queue_head *q = &vcpu->wq;
struct kvm_timer *ktimer = &apic->lapic_timer;
if (atomic_read(&apic->lapic_timer.pending))
@@ -1126,8 +1126,8 @@ static void apic_timer_expired(struct kvm_lapic *apic)
atomic_inc(&apic->lapic_timer.pending);
kvm_set_pending_timer(vcpu);
- if (waitqueue_active(q))
- wake_up_interruptible(q);
+ if (swait_active(q))
+ swake_up(q);
if (apic_lvtt_tscdeadline(apic))
ktimer->expired_tscdeadline = ktimer->tscdeadline;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1bef9e2..7b6231e 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -24,6 +24,7 @@
#include <linux/err.h>
#include <linux/irqflags.h>
#include <linux/context_tracking.h>
+#include <linux/swait.h>
#include <asm/signal.h>
#include <linux/kvm.h>
@@ -237,7 +238,7 @@ struct kvm_vcpu {
int fpu_active;
int guest_fpu_loaded, guest_xcr0_loaded;
unsigned char fpu_counter;
- wait_queue_head_t wq;
+ struct swait_queue_head wq;
struct pid *pid;
int sigset_active;
sigset_t sigset;
@@ -759,7 +760,7 @@ static inline bool kvm_arch_has_assigned_device(struct kvm *kvm)
}
#endif
-static inline wait_queue_head_t *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
+static inline struct swait_queue_head *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
{
#ifdef __KVM_HAVE_ARCH_WQP
return vcpu->arch.wqp;
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 44660ae..ff4891c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -94,8 +94,8 @@ static void async_pf_execute(struct work_struct *work)
trace_kvm_async_pf_completed(addr, gva);
- if (waitqueue_active(&vcpu->wq))
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq))
+ swake_up(&vcpu->wq);
mmput(mm);
kvm_put_kvm(vcpu->kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8db1d93..45ab55f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -226,8 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
vcpu->kvm = kvm;
vcpu->vcpu_id = id;
vcpu->pid = NULL;
- vcpu->halt_poll_ns = 0;
- init_waitqueue_head(&vcpu->wq);
+ init_swait_queue_head(&vcpu->wq);
kvm_async_pf_vcpu_init(vcpu);
page = alloc_page(GFP_KERNEL | __GFP_ZERO);
@@ -1996,7 +1995,7 @@ static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
void kvm_vcpu_block(struct kvm_vcpu *vcpu)
{
ktime_t start, cur;
- DEFINE_WAIT(wait);
+ DECLARE_SWAITQUEUE(wait);
bool waited = false;
u64 block_ns;
@@ -2019,7 +2018,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
}
for (;;) {
- prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+ prepare_to_swait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
if (kvm_vcpu_check_block(vcpu) < 0)
break;
@@ -2028,7 +2027,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
schedule();
}
- finish_wait(&vcpu->wq, &wait);
+ finish_swait(&vcpu->wq, &wait);
cur = ktime_get();
out:
@@ -2059,11 +2058,11 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
{
int me;
int cpu = vcpu->cpu;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
wqp = kvm_arch_vcpu_wq(vcpu);
- if (waitqueue_active(wqp)) {
- wake_up_interruptible(wqp);
+ if (swait_active(wqp)) {
+ swake_up(wqp);
++vcpu->stat.halt_wakeup;
}
@@ -2164,7 +2163,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
continue;
if (vcpu == me)
continue;
- if (waitqueue_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
+ if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
continue;
if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
continue;
--
2.4.3
rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
not enable the IRQs. lockdep is happy.
By switching over using swait this is not true anymore. swake_up_all()
enables the IRQs while processing the waiters. __do_softirq() can now
run and will eventually call rcu_process_callbacks() which wants to
grap nrp->lock.
Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
switch over to swait.
If we would hold the rnp->lock and use swait, lockdep reports
following:
=================================
[ INFO: inconsistent lock state ]
4.2.0-rc5-00025-g9a73ba0 #136 Not tainted
---------------------------------
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
rcu_preempt/8 [HC0[0]:SC0[0]:HE1:SE1] takes:
(rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
{IN-SOFTIRQ-W} state was registered at:
[<ffffffff81109b9f>] __lock_acquire+0xd5f/0x21e0
[<ffffffff8110be0f>] lock_acquire+0xdf/0x2b0
[<ffffffff81841cc9>] _raw_spin_lock_irqsave+0x59/0xa0
[<ffffffff81136991>] rcu_process_callbacks+0x141/0x3c0
[<ffffffff810b1a9d>] __do_softirq+0x14d/0x670
[<ffffffff810b2214>] irq_exit+0x104/0x110
[<ffffffff81844e96>] smp_apic_timer_interrupt+0x46/0x60
[<ffffffff81842e70>] apic_timer_interrupt+0x70/0x80
[<ffffffff810dba66>] rq_attach_root+0xa6/0x100
[<ffffffff810dbc2d>] cpu_attach_domain+0x16d/0x650
[<ffffffff810e4b42>] build_sched_domains+0x942/0xb00
[<ffffffff821777c2>] sched_init_smp+0x509/0x5c1
[<ffffffff821551e3>] kernel_init_freeable+0x172/0x28f
[<ffffffff8182cdce>] kernel_init+0xe/0xe0
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
irq event stamp: 76
hardirqs last enabled at (75): [<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
hardirqs last disabled at (76): [<ffffffff8184116f>] _raw_spin_lock_irq+0x1f/0x90
softirqs last enabled at (0): [<ffffffff810a8df2>] copy_process.part.26+0x602/0x1cf0
softirqs last disabled at (0): [< (null)>] (null)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(rcu_node_1);
<Interrupt>
lock(rcu_node_1);
*** DEADLOCK ***
1 lock held by rcu_preempt/8:
#0: (rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
stack backtrace:
CPU: 0 PID: 8 Comm: rcu_preempt Not tainted 4.2.0-rc5-00025-g9a73ba0 #136
Hardware name: Dell Inc. PowerEdge R820/066N7P, BIOS 2.0.20 01/16/2014
0000000000000000 000000006d7e67d8 ffff881fb081fbd8 ffffffff818379e0
0000000000000000 ffff881fb0812a00 ffff881fb081fc38 ffffffff8110813b
0000000000000000 0000000000000001 ffff881f00000001 ffffffff8102fa4f
Call Trace:
[<ffffffff818379e0>] dump_stack+0x4f/0x7b
[<ffffffff8110813b>] print_usage_bug+0x1db/0x1e0
[<ffffffff8102fa4f>] ? save_stack_trace+0x2f/0x50
[<ffffffff811087ad>] mark_lock+0x66d/0x6e0
[<ffffffff81107790>] ? check_usage_forwards+0x150/0x150
[<ffffffff81108898>] mark_held_locks+0x78/0xa0
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff81108a28>] trace_hardirqs_on_caller+0x168/0x220
[<ffffffff81108aed>] trace_hardirqs_on+0xd/0x10
[<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810fd1c7>] swake_up_all+0xb7/0xe0
[<ffffffff811386e1>] rcu_gp_kthread+0xab1/0xeb0
[<ffffffff811089bf>] ? trace_hardirqs_on_caller+0xff/0x220
[<ffffffff81841341>] ? _raw_spin_unlock_irq+0x41/0x60
[<ffffffff81137c30>] ? rcu_barrier+0x20/0x20
[<ffffffff810d2014>] kthread+0x104/0x120
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
Signed-off-by: Daniel Wagner <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
---
kernel/rcu/tree.c | 4 +++-
kernel/rcu/tree.h | 3 ++-
kernel/rcu/tree_plugin.h | 16 +++++++++++++---
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 775d36c..952536d 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1568,7 +1568,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
int needmore;
struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
- rcu_nocb_gp_cleanup(rsp, rnp);
rnp->need_future_gp[c & 0x1] = 0;
needmore = rnp->need_future_gp[(c + 1) & 0x1];
trace_rcu_future_gp(rnp, rdp, c,
@@ -1972,6 +1971,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
int nocb = 0;
struct rcu_data *rdp;
struct rcu_node *rnp = rcu_get_root(rsp);
+ struct swait_queue_head *sq;
WRITE_ONCE(rsp->gp_activity, jiffies);
raw_spin_lock_irq(&rnp->lock);
@@ -2010,7 +2010,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
/* smp_mb() provided by prior unlock-lock pair. */
nocb += rcu_future_gp_cleanup(rsp, rnp);
+ sq = rcu_nocb_gp_get(rnp);
raw_spin_unlock_irq(&rnp->lock);
+ rcu_nocb_gp_cleanup(sq);
cond_resched_rcu_qs();
WRITE_ONCE(rsp->gp_activity, jiffies);
rcu_gp_slow(rsp, gp_cleanup_delay);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 2e991f8..3dcf6368 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -608,7 +608,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
static void increment_cpu_stall_ticks(void);
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp);
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq);
static void rcu_init_one_nocb(struct rcu_node *rnp);
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
bool lazy, unsigned long flags);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index b2bf396..db4f357 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1777,9 +1777,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
* grace period.
*/
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
- wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
+ wake_up_all(sq);
}
/*
@@ -1795,6 +1795,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
}
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return &rnp->nocb_gp_wq[rnp->completed & 0x1];
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
init_waitqueue_head(&rnp->nocb_gp_wq[0]);
@@ -2469,7 +2474,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
return false;
}
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
}
@@ -2477,6 +2482,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
{
}
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return NULL;
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
}
--
2.4.3
From: Paul Gortmaker <[email protected]>
As of commit dae6e64d2bcfd4b06304ab864c7e3a4f6b5fedf4 ("rcu: Introduce
proper blocking to no-CBs kthreads GP waits") the RCU subsystem started
making use of wait queues.
Here we convert all additions of RCU wait queues to use simple wait queues,
since they don't need the extra overhead of the full wait queue features.
Originally this was done for RT kernels[1], since we would get things like...
BUG: sleeping function called from invalid context at kernel/rtmutex.c:659
in_atomic(): 1, irqs_disabled(): 1, pid: 8, name: rcu_preempt
Pid: 8, comm: rcu_preempt Not tainted
Call Trace:
[<ffffffff8106c8d0>] __might_sleep+0xd0/0xf0
[<ffffffff817d77b4>] rt_spin_lock+0x24/0x50
[<ffffffff8106fcf6>] __wake_up+0x36/0x70
[<ffffffff810c4542>] rcu_gp_kthread+0x4d2/0x680
[<ffffffff8105f910>] ? __init_waitqueue_head+0x50/0x50
[<ffffffff810c4070>] ? rcu_gp_fqs+0x80/0x80
[<ffffffff8105eabb>] kthread+0xdb/0xe0
[<ffffffff8106b912>] ? finish_task_switch+0x52/0x100
[<ffffffff817e0754>] kernel_thread_helper+0x4/0x10
[<ffffffff8105e9e0>] ? __init_kthread_worker+0x60/0x60
[<ffffffff817e0750>] ? gs_change+0xb/0xb
...and hence simple wait queues were deployed on RT out of necessity
(as simple wait uses a raw lock), but mainline might as well take
advantage of the more streamline support as well.
[1] This is a carry forward of work from v3.10-rt; the original conversion
was by Thomas on an earlier -rt version, and Sebastian extended it to
additional post-3.10 added RCU waiters; here I've added a commit log and
unified the RCU changes into one, and uprev'd it to match mainline RCU.
Signed-off-by: Daniel Wagner <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Paul Gortmaker <[email protected]>
Cc: Peter Zijlstra (Intel) <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
---
kernel/rcu/tree.c | 20 ++++++++++----------
kernel/rcu/tree.h | 9 +++++----
kernel/rcu/tree_plugin.h | 18 +++++++++---------
3 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 952536d..628ffb5 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1588,7 +1588,7 @@ static void rcu_gp_kthread_wake(struct rcu_state *rsp)
!READ_ONCE(rsp->gp_flags) ||
!rsp->gp_kthread)
return;
- wake_up(&rsp->gp_wq);
+ swake_up(&rsp->gp_wq);
}
/*
@@ -2059,7 +2059,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("reqwait"));
rsp->gp_state = RCU_GP_WAIT_GPS;
- wait_event_interruptible(rsp->gp_wq,
+ swait_event_interruptible(rsp->gp_wq,
READ_ONCE(rsp->gp_flags) &
RCU_GP_FLAG_INIT);
rsp->gp_state = RCU_GP_DONE_GPS;
@@ -2089,7 +2089,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("fqswait"));
rsp->gp_state = RCU_GP_WAIT_FQS;
- ret = wait_event_interruptible_timeout(rsp->gp_wq,
+ ret = swait_event_interruptible_timeout(rsp->gp_wq,
rcu_gp_fqs_check_wake(rsp, &gf), j);
rsp->gp_state = RCU_GP_DOING_FQS;
/* Locking provides needed memory barriers. */
@@ -2212,7 +2212,7 @@ static void rcu_report_qs_rsp(struct rcu_state *rsp, unsigned long flags)
WARN_ON_ONCE(!rcu_gp_in_progress(rsp));
WRITE_ONCE(rsp->gp_flags, READ_ONCE(rsp->gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore(&rcu_get_root(rsp)->lock, flags);
- rcu_gp_kthread_wake(rsp);
+ swake_up(&rsp->gp_wq); /* Memory barrier implied by swake_up() path. */
}
/*
@@ -2873,7 +2873,7 @@ static void force_quiescent_state(struct rcu_state *rsp)
}
WRITE_ONCE(rsp->gp_flags, READ_ONCE(rsp->gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore(&rnp_old->lock, flags);
- rcu_gp_kthread_wake(rsp);
+ swake_up(&rsp->gp_wq); /* Memory barrier implied by swake_up() path. */
}
/*
@@ -3465,7 +3465,7 @@ static int synchronize_sched_expedited_cpu_stop(void *data)
/* We are here: If we are last, do the wakeup. */
rdp->exp_done = true;
if (atomic_dec_and_test(&rsp->expedited_need_qs))
- wake_up(&rsp->expedited_wq);
+ swake_up(&rsp->expedited_wq);
return 0;
}
@@ -3481,7 +3481,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
jiffies_start = jiffies;
for (;;) {
- ret = wait_event_interruptible_timeout(
+ ret = swait_event_timeout(
rsp->expedited_wq,
!atomic_read(&rsp->expedited_need_qs),
jiffies_stall);
@@ -3489,7 +3489,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
return;
if (ret < 0) {
/* Hit a signal, disable CPU stall warnings. */
- wait_event(rsp->expedited_wq,
+ swait_event(rsp->expedited_wq,
!atomic_read(&rsp->expedited_need_qs));
return;
}
@@ -3558,7 +3558,7 @@ void synchronize_sched_expedited(void)
rcu_exp_gp_seq_start(rsp);
/* Stop each CPU that is online, non-idle, and not us. */
- init_waitqueue_head(&rsp->expedited_wq);
+ init_swait_queue_head(&rsp->expedited_wq);
atomic_set(&rsp->expedited_need_qs, 1); /* Extra count avoids race. */
for_each_online_cpu(cpu) {
struct rcu_data *rdp = per_cpu_ptr(rsp->rda, cpu);
@@ -4180,7 +4180,7 @@ static void __init rcu_init_one(struct rcu_state *rsp,
}
}
- init_waitqueue_head(&rsp->gp_wq);
+ init_swait_queue_head(&rsp->gp_wq);
rnp = rsp->level[rcu_num_lvls - 1];
for_each_possible_cpu(i) {
while (i > rnp->grphi)
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 3dcf6368..02c29d3 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -27,6 +27,7 @@
#include <linux/threads.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
+#include <linux/swait.h>
#include <linux/stop_machine.h>
/*
@@ -244,7 +245,7 @@ struct rcu_node {
/* Refused to boost: not sure why, though. */
/* This can happen due to race conditions. */
#ifdef CONFIG_RCU_NOCB_CPU
- wait_queue_head_t nocb_gp_wq[2];
+ struct swait_queue_head nocb_gp_wq[2];
/* Place for rcu_nocb_kthread() to wait GP. */
#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
int need_future_gp[2];
@@ -388,7 +389,7 @@ struct rcu_data {
atomic_long_t nocb_q_count_lazy; /* invocation (all stages). */
struct rcu_head *nocb_follower_head; /* CBs ready to invoke. */
struct rcu_head **nocb_follower_tail;
- wait_queue_head_t nocb_wq; /* For nocb kthreads to sleep on. */
+ struct swait_queue_head nocb_wq; /* For nocb kthreads to sleep on. */
struct task_struct *nocb_kthread;
int nocb_defer_wakeup; /* Defer wakeup of nocb_kthread. */
@@ -475,7 +476,7 @@ struct rcu_state {
unsigned long gpnum; /* Current gp number. */
unsigned long completed; /* # of last completed gp. */
struct task_struct *gp_kthread; /* Task for grace periods. */
- wait_queue_head_t gp_wq; /* Where GP task waits. */
+ struct swait_queue_head gp_wq; /* Where GP task waits. */
short gp_flags; /* Commands for GP task. */
short gp_state; /* GP kthread sleep state. */
@@ -507,7 +508,7 @@ struct rcu_state {
atomic_long_t expedited_workdone3; /* # done by others #3. */
atomic_long_t expedited_normal; /* # fallbacks to normal. */
atomic_t expedited_need_qs; /* # CPUs left to check in. */
- wait_queue_head_t expedited_wq; /* Wait for check-ins. */
+ struct swait_queue_head expedited_wq; /* Wait for check-ins. */
unsigned long jiffies_force_qs; /* Time at which to invoke */
/* force_quiescent_state(). */
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index db4f357..0c69868 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1779,7 +1779,7 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
*/
static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
- wake_up_all(sq);
+ swake_up_all(sq);
}
/*
@@ -1802,8 +1802,8 @@ static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
- init_waitqueue_head(&rnp->nocb_gp_wq[0]);
- init_waitqueue_head(&rnp->nocb_gp_wq[1]);
+ init_swait_queue_head(&rnp->nocb_gp_wq[0]);
+ init_swait_queue_head(&rnp->nocb_gp_wq[1]);
}
#ifndef CONFIG_RCU_NOCB_CPU_ALL
@@ -1828,7 +1828,7 @@ static void wake_nocb_leader(struct rcu_data *rdp, bool force)
if (READ_ONCE(rdp_leader->nocb_leader_sleep) || force) {
/* Prior smp_mb__after_atomic() orders against prior enqueue. */
WRITE_ONCE(rdp_leader->nocb_leader_sleep, false);
- wake_up(&rdp_leader->nocb_wq);
+ swake_up(&rdp_leader->nocb_wq);
}
}
@@ -2041,7 +2041,7 @@ static void rcu_nocb_wait_gp(struct rcu_data *rdp)
*/
trace_rcu_future_gp(rnp, rdp, c, TPS("StartWait"));
for (;;) {
- wait_event_interruptible(
+ swait_event_interruptible(
rnp->nocb_gp_wq[c & 0x1],
(d = ULONG_CMP_GE(READ_ONCE(rnp->completed), c)));
if (likely(d))
@@ -2069,7 +2069,7 @@ wait_again:
/* Wait for callbacks to appear. */
if (!rcu_nocb_poll) {
trace_rcu_nocb_wake(my_rdp->rsp->name, my_rdp->cpu, "Sleep");
- wait_event_interruptible(my_rdp->nocb_wq,
+ swait_event_interruptible(my_rdp->nocb_wq,
!READ_ONCE(my_rdp->nocb_leader_sleep));
/* Memory barrier handled by smp_mb() calls below and repoll. */
} else if (firsttime) {
@@ -2144,7 +2144,7 @@ wait_again:
* List was empty, wake up the follower.
* Memory barriers supplied by atomic_long_add().
*/
- wake_up(&rdp->nocb_wq);
+ swake_up(&rdp->nocb_wq);
}
}
@@ -2165,7 +2165,7 @@ static void nocb_follower_wait(struct rcu_data *rdp)
if (!rcu_nocb_poll) {
trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
"FollowerSleep");
- wait_event_interruptible(rdp->nocb_wq,
+ swait_event_interruptible(rdp->nocb_wq,
READ_ONCE(rdp->nocb_follower_head));
} else if (firsttime) {
/* Don't drown trace log with "Poll"! */
@@ -2324,7 +2324,7 @@ void __init rcu_init_nohz(void)
static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
{
rdp->nocb_tail = &rdp->nocb_head;
- init_waitqueue_head(&rdp->nocb_wq);
+ init_swait_queue_head(&rdp->nocb_wq);
rdp->nocb_follower_tail = &rdp->nocb_follower_head;
}
--
2.4.3
Hi Daniel,
On Tue, Nov 24, 2015 at 02:03:06PM +0100, Daniel Wagner wrote:
> rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
> this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
> not enable the IRQs. lockdep is happy.
>
> By switching over using swait this is not true anymore. swake_up_all()
> enables the IRQs while processing the waiters. __do_softirq() can now
> run and will eventually call rcu_process_callbacks() which wants to
> grap nrp->lock.
>
> Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
> switch over to swait.
>
But you did introduce swait in this patch ;-)
[snip]
>
> Signed-off-by: Daniel Wagner <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> ---
> kernel/rcu/tree.c | 4 +++-
> kernel/rcu/tree.h | 3 ++-
> kernel/rcu/tree_plugin.h | 16 +++++++++++++---
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
So I tried to build this patch with a config having RCU_EXPERT=y and
RCU_NOCB_CPU=y, but I got:
In file included from include/linux/completion.h:11:0,
from include/linux/rcupdate.h:43,
from include/linux/sysctl.h:25,
from include/linux/timer.h:242,
from include/linux/workqueue.h:8,
from include/linux/pm.h:25,
from ./arch/x86/include/asm/apic.h:5,
from ./arch/x86/include/asm/smp.h:12,
from include/linux/smp.h:59,
from kernel/rcu/tree.c:34:
kernel/rcu/tree_plugin.h: In function ‘rcu_nocb_gp_cleanup’:
kernel/rcu/tree_plugin.h:1782:14: warning: passing argument 1 of ‘__wake_up’ from incompatible pointer type [-Wincompatible-pointer-types]
wake_up_all(sq);
^
include/linux/wait.h:168:36: note: in definition of macro ‘wake_up_all’
#define wake_up_all(x) __wake_up(x, TASK_NORMAL, 0, NULL)
(I also attach the configure file in case you need it)
I think the reason is that you introduced swait in this patch, but
you didn't use the proper function, e.g. swake_up_all(), and also you
didn't #include <linux/swait.h> ;-)
Regards,
Boqun
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 775d36c..952536d 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1568,7 +1568,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
> int needmore;
> struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
>
> - rcu_nocb_gp_cleanup(rsp, rnp);
> rnp->need_future_gp[c & 0x1] = 0;
> needmore = rnp->need_future_gp[(c + 1) & 0x1];
> trace_rcu_future_gp(rnp, rdp, c,
> @@ -1972,6 +1971,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
> int nocb = 0;
> struct rcu_data *rdp;
> struct rcu_node *rnp = rcu_get_root(rsp);
> + struct swait_queue_head *sq;
>
> WRITE_ONCE(rsp->gp_activity, jiffies);
> raw_spin_lock_irq(&rnp->lock);
> @@ -2010,7 +2010,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
> needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
> /* smp_mb() provided by prior unlock-lock pair. */
> nocb += rcu_future_gp_cleanup(rsp, rnp);
> + sq = rcu_nocb_gp_get(rnp);
> raw_spin_unlock_irq(&rnp->lock);
> + rcu_nocb_gp_cleanup(sq);
> cond_resched_rcu_qs();
> WRITE_ONCE(rsp->gp_activity, jiffies);
> rcu_gp_slow(rsp, gp_cleanup_delay);
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index 2e991f8..3dcf6368 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -608,7 +608,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
> static void increment_cpu_stall_ticks(void);
> static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
> static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
> -static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
> +static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp);
> +static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq);
> static void rcu_init_one_nocb(struct rcu_node *rnp);
> static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
> bool lazy, unsigned long flags);
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index b2bf396..db4f357 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1777,9 +1777,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
> * Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
> * grace period.
> */
> -static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
> +static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
> {
> - wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
> + wake_up_all(sq);
> }
>
> /*
> @@ -1795,6 +1795,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
> rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
> }
>
> +static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
> +{
> + return &rnp->nocb_gp_wq[rnp->completed & 0x1];
> +}
> +
> static void rcu_init_one_nocb(struct rcu_node *rnp)
> {
> init_waitqueue_head(&rnp->nocb_gp_wq[0]);
> @@ -2469,7 +2474,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
> return false;
> }
>
> -static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
> +static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
> {
> }
>
> @@ -2477,6 +2482,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
> {
> }
>
> +static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
> +{
> + return NULL;
> +}
> +
> static void rcu_init_one_nocb(struct rcu_node *rnp)
> {
> }
> --
> 2.4.3
>
On Tue, Nov 24, 2015 at 11:52:12PM +0800, Boqun Feng wrote:
> Hi Daniel,
>
> On Tue, Nov 24, 2015 at 02:03:06PM +0100, Daniel Wagner wrote:
> > rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
> > this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
> > not enable the IRQs. lockdep is happy.
> >
> > By switching over using swait this is not true anymore. swake_up_all()
> > enables the IRQs while processing the waiters. __do_softirq() can now
> > run and will eventually call rcu_process_callbacks() which wants to
> > grap nrp->lock.
> >
> > Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
> > switch over to swait.
> >
>
> But you did introduce swait in this patch ;-)
>
> [snip]
>
> >
> > Signed-off-by: Daniel Wagner <[email protected]>
> > Cc: "Paul E. McKenney" <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: [email protected]
> > ---
> > kernel/rcu/tree.c | 4 +++-
> > kernel/rcu/tree.h | 3 ++-
> > kernel/rcu/tree_plugin.h | 16 +++++++++++++---
> > 3 files changed, 18 insertions(+), 5 deletions(-)
> >
>
> So I tried to build this patch with a config having RCU_EXPERT=y and
> RCU_NOCB_CPU=y, but I got:
>
> In file included from include/linux/completion.h:11:0,
> from include/linux/rcupdate.h:43,
> from include/linux/sysctl.h:25,
> from include/linux/timer.h:242,
> from include/linux/workqueue.h:8,
> from include/linux/pm.h:25,
> from ./arch/x86/include/asm/apic.h:5,
> from ./arch/x86/include/asm/smp.h:12,
> from include/linux/smp.h:59,
> from kernel/rcu/tree.c:34:
> kernel/rcu/tree_plugin.h: In function ‘rcu_nocb_gp_cleanup’:
> kernel/rcu/tree_plugin.h:1782:14: warning: passing argument 1 of ‘__wake_up’ from incompatible pointer type [-Wincompatible-pointer-types]
> wake_up_all(sq);
> ^
> include/linux/wait.h:168:36: note: in definition of macro ‘wake_up_all’
> #define wake_up_all(x) __wake_up(x, TASK_NORMAL, 0, NULL)
>
>
Just to be clear, I saw this build error when I applied only the first
four patches of this series. When I applied the whole series, I didn't
see any build error.
Regards,
Boqun
Hi Boqun,
On 11/25/2015 02:01 AM, Boqun Feng wrote:
> On Tue, Nov 24, 2015 at 11:52:12PM +0800, Boqun Feng wrote:
>> Hi Daniel,
>>
>> On Tue, Nov 24, 2015 at 02:03:06PM +0100, Daniel Wagner wrote:
>>> rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
>>> this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
>>> not enable the IRQs. lockdep is happy.
>>>
>>> By switching over using swait this is not true anymore. swake_up_all()
>>> enables the IRQs while processing the waiters. __do_softirq() can now
>>> run and will eventually call rcu_process_callbacks() which wants to
>>> grap nrp->lock.
>>>
>>> Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
>>> switch over to swait.
>>>
>>
>> But you did introduce swait in this patch ;-)
Argh, that is a fail. I did build all patches individual but seems like
I haven't got the right configuration.
>> [snip]
>>
>>>
>>> Signed-off-by: Daniel Wagner <[email protected]>
>>> Cc: "Paul E. McKenney" <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: [email protected]
>>> ---
>>> kernel/rcu/tree.c | 4 +++-
>>> kernel/rcu/tree.h | 3 ++-
>>> kernel/rcu/tree_plugin.h | 16 +++++++++++++---
>>> 3 files changed, 18 insertions(+), 5 deletions(-)
>>>
>>
>> So I tried to build this patch with a config having RCU_EXPERT=y and
>> RCU_NOCB_CPU=y, but I got:
Will update my config accordingly.
Thanks,
Daniel
Hi Thomas,
On 11/24/2015 02:03 PM, Daniel Wagner wrote:
> The API provided by wait.h and swait.h is very similiar. Most of the
> time your are only one character away from either of it:
>
> wake_up() vs swake_up()
>
> This is on purpose so that we do not have two nearly identical bits of
> infrastructre code with dissimilar names.
>
> A compile time type check assertion ensures that obvious wrong usage
> is caught at early stage.
Obviously, this didn't really work as one can see with patch #4. That
one just compiled. So I wrapped almost all functions to get a better
check coverage. woken_wake_function(), autoremove_wake_function() and
wake_bit_function() can't be wrapped easily because DEFINE_WAIT and
friends. I just left them out.
The result looks pretty bad in my opinion. Probably it would be
better do add -Werror=incompatible-pointer-types to the CFLAGS.
Is that what you had in mind?
cheers,
daniel
>From 3a84d2eed35e3acb76bf2f7557bb4c3763a3a433 Mon Sep 17 00:00:00 2001
From: Daniel Wagner <[email protected]>
Date: Thu, 26 Nov 2015 07:53:03 +0100
Subject: [PATCH] wait: Add compile time type check assertion
The API provided by wait.h and swait.h is very similiar. Most of the
time your are only one character away from either of it:
wake_up() vs swake_up()
This is on purpose so that we do not have two nearly identical bits of
infrastructre code with dissimilar names.
A compile time type check assertion ensures that obvious wrong usage
is caught at early stage.
---
include/linux/compiler.h | 4 +
include/linux/swait.h | 72 ++++++++++++++---
include/linux/wait.h | 200 ++++++++++++++++++++++++++++++++++++-----------
kernel/sched/swait.c | 42 +++++-----
kernel/sched/wait.c | 108 ++++++++++++-------------
5 files changed, 294 insertions(+), 132 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c836eb2..ac7afcb 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -455,6 +455,10 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
compiletime_assert(__native_word(t), \
"Need native word sized stores/loads for atomicity.")
+#define compiletime_assert_same_type(a, b) \
+ compiletime_assert(__same_type(a, b), \
+ "Need same type.");
+
/*
* Prevent the compiler from merging or refetching accesses. The compiler
* is also forbidden from reordering successive instances of ACCESS_ONCE(),
diff --git a/include/linux/swait.h b/include/linux/swait.h
index c1f9c62..ebc6f9a 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -44,6 +44,43 @@ struct swait_queue {
struct list_head task_list;
};
+
+/*
+ * Macros for type checks
+ */
+
+#define swait_tchk_q(fn, q, ...) \
+ do { \
+ compiletime_assert_same_type(struct swait_queue_head *, q); \
+ fn(q, ##__VA_ARGS__); \
+ } while (0)
+
+#define swait_tchk_ret_q(fn, q, ...) \
+ ({ \
+ compiletime_assert_same_type(struct swait_queue_head *, q); \
+ fn(q, ##__VA_ARGS__); \
+ })
+
+#define swait_tchk_w(fn, w, ...) \
+ do { \
+ compiletime_assert_same_type(struct swait_queue *, w); \
+ fn(w, ...); \
+ } while (0)
+
+#define swait_tchk_qw(fn, q, w, ...) \
+ do { \
+ compiletime_assert_same_type(struct swait_queue_head *, q); \
+ compiletime_assert_same_type(struct swait_queue *, w); \
+ fn(q, w, ##__VA_ARGS__); \
+ } while (0)
+
+#define swait_tchk_ret_qw(fn, q, w, ...) \
+ ({ \
+ compiletime_assert_same_type(struct swait_queue_head *, q); \
+ compiletime_assert_same_type(struct swait_queue *, w); \
+ fn(q, w, ##__VA_ARGS__); \
+ })
+
#define __SWAITQUEUE_INITIALIZER(name) { \
.task = current, \
.task_list = LIST_HEAD_INIT((name).task_list), \
@@ -60,8 +97,10 @@ struct swait_queue {
#define DECLARE_SWAIT_QUEUE_HEAD(name) \
struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INITIALIZER(name)
-extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+extern void ___init_swait_queue_head(struct swait_queue_head *q, const char *name,
struct lock_class_key *key);
+#define __init_swait_queue_head(q, s, k) \
+ swait_tchk_q(___init_swait_queue_head, q, s, k)
#define init_swait_queue_head(q) \
do { \
@@ -79,21 +118,34 @@ extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name
DECLARE_SWAIT_QUEUE_HEAD(name)
#endif
-static inline int swait_active(struct swait_queue_head *q)
+static inline int _swait_active(struct swait_queue_head *q)
{
return !list_empty(&q->task_list);
}
-extern void swake_up(struct swait_queue_head *q);
-extern void swake_up_all(struct swait_queue_head *q);
-extern void swake_up_locked(struct swait_queue_head *q);
+#define swait_active(q) swait_tchk_ret_q(_swait_active, q)
-extern void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
-extern void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state);
-extern long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state);
+extern void _swake_up(struct swait_queue_head *q);
+extern void _swake_up_all(struct swait_queue_head *q);
+extern void _swake_up_locked(struct swait_queue_head *q);
+extern void ___prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state);
+extern long _prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state);
+
+extern void ___finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
extern void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
-extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+
+#define swake_up(q) swait_tchk_q(_swake_up, q)
+#define swake_up_all(q) swait_tchk_q(_swake_up_all, q)
+#define swake_up_locked(q) swait_tchk_q(_swake_up_locked, q)
+
+#define _prepare_to_swait(q, w) swait_tchk_qw(___prepare_to_swait, q, w)
+#define prepare_to_swait(q, w, s) swait_tchk_qw(__prepare_to_swait, q, w, s)
+#define prepare_to_swait_event(q, w, s) swait_tchk_ret_qw(_prepare_to_swait_event, q, w, s)
+
+#define _finish_swait(q, w) swait_tchk_qw(___finish_swait, q, w)
+#define finish_swait(q, w) swait_tchk_qw(__finish_swait, q, w)
/* as per ___wait_event() but for swait, therefore "exclusive == 0" */
#define ___swait_event(wq, condition, state, ret, cmd) \
@@ -103,7 +155,7 @@ extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
\
INIT_LIST_HEAD(&__wait.task_list); \
for (;;) { \
- long __int = prepare_to_swait_event(&wq, &__wait, state);\
+ long __int = _prepare_to_swait_event(&wq, &__wait, state);\
\
if (condition) \
break; \
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 1e1bf9f..9186497 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -45,6 +45,48 @@ typedef struct __wait_queue_head wait_queue_head_t;
struct task_struct;
/*
+ * Macros for type checks
+ */
+
+#define wait_tchk_q(fn, q, ...) \
+ do { \
+ compiletime_assert_same_type(wait_queue_head_t *, q); \
+ fn(q, ##__VA_ARGS__); \
+ } while (0)
+
+#define wait_tchk_ret_q(fn, q, ...) \
+ ({ \
+ compiletime_assert_same_type(wait_queue_head_t *, q); \
+ fn(q, ##__VA_ARGS__); \
+ })
+
+#define wait_tchk_w(fn, w, ...) \
+ do { \
+ compiletime_assert_same_type(wait_queue_t *, w); \
+ fn(w, ##__VA_ARGS__); \
+ } while (0)
+
+#define wait_tchk_ret_w(fn, w, ...) \
+ ({ \
+ compiletime_assert_same_type(wait_queue_t *, w); \
+ fn(w, ##__VA_ARGS__); \
+ })
+
+#define wait_tchk_qw(fn, q, w, ...) \
+ do { \
+ compiletime_assert_same_type(wait_queue_head_t *, q); \
+ compiletime_assert_same_type(wait_queue_t *, w); \
+ fn(q, w, ##__VA_ARGS__); \
+ } while (0)
+
+#define wait_tchk_ret_qw(fn, q, w, ...) \
+ ({ \
+ compiletime_assert_same_type(wait_queue_head_t *, q); \
+ compiletime_assert_same_type(wait_queue_t *, w); \
+ fn(q, w, ##__VA_ARGS__); \
+ })
+
+/*
* Macros for declaration and initialisaton of the datatypes
*/
@@ -69,13 +111,15 @@ struct task_struct;
#define __WAIT_ATOMIC_T_KEY_INITIALIZER(p) \
{ .flags = p, .bit_nr = WAIT_ATOMIC_T_BIT_NR, }
-extern void __init_waitqueue_head(wait_queue_head_t *q, const char *name, struct lock_class_key *);
+extern void ___init_waitqueue_head(wait_queue_head_t *q, const char *name, struct lock_class_key *);
+#define __init_waitqueue_head(q, s, l) \
+ wait_tchk_q(___init_waitqueue_head, q, s, l)
#define init_waitqueue_head(q) \
do { \
static struct lock_class_key __key; \
\
- __init_waitqueue_head((q), #q, &__key); \
+ __init_waitqueue_head((q), #q, &__key); \
} while (0)
#ifdef CONFIG_LOCKDEP
@@ -87,7 +131,7 @@ extern void __init_waitqueue_head(wait_queue_head_t *q, const char *name, struct
# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) DECLARE_WAIT_QUEUE_HEAD(name)
#endif
-static inline void init_waitqueue_entry(wait_queue_t *q, struct task_struct *p)
+static inline void _init_waitqueue_entry(wait_queue_t *q, struct task_struct *p)
{
q->flags = 0;
q->private = p;
@@ -95,23 +139,37 @@ static inline void init_waitqueue_entry(wait_queue_t *q, struct task_struct *p)
}
static inline void
-init_waitqueue_func_entry(wait_queue_t *q, wait_queue_func_t func)
+_init_waitqueue_func_entry(wait_queue_t *q, wait_queue_func_t func)
{
q->flags = 0;
q->private = NULL;
q->func = func;
}
-static inline int waitqueue_active(wait_queue_head_t *q)
+static inline int _waitqueue_active(wait_queue_head_t *q)
{
return !list_empty(&q->task_list);
}
-extern void add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
-extern void add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait);
-extern void remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
-
-static inline void __add_wait_queue(wait_queue_head_t *head, wait_queue_t *new)
+#define init_waitqueue_entry(w, p) \
+ wait_tchk_w(_init_waitqueue_entry, w, p)
+#define init_waitqueue_func_entry(w, f) \
+ wait_tchk_w(_init_waitqueue_func_entry, w, f)
+#define waitqueue_active(q) \
+ wait_tchk_ret_q(_waitqueue_active, q)
+
+extern void _add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
+extern void _add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait);
+extern void _remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait);
+
+#define add_wait_queue(q, w) \
+ wait_tchk_qw(_add_wait_queue, q, w)
+#define add_wait_queue_exclusive(q, w) \
+ wait_tchk_qw(_add_wait_queue_exclusive, q, w)
+#define remove_wait_queue(q, w) \
+ wait_tchk_qw(_remove_wait_queue, q, w)
+
+static inline void ___add_wait_queue(wait_queue_head_t *head, wait_queue_t *new)
{
list_add(&new->task_list, &head->task_list);
}
@@ -120,40 +178,51 @@ static inline void __add_wait_queue(wait_queue_head_t *head, wait_queue_t *new)
* Used for wake-one threads:
*/
static inline void
-__add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait)
+___add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait)
{
wait->flags |= WQ_FLAG_EXCLUSIVE;
- __add_wait_queue(q, wait);
+ ___add_wait_queue(q, wait);
}
-static inline void __add_wait_queue_tail(wait_queue_head_t *head,
+static inline void ___add_wait_queue_tail(wait_queue_head_t *head,
wait_queue_t *new)
{
list_add_tail(&new->task_list, &head->task_list);
}
static inline void
-__add_wait_queue_tail_exclusive(wait_queue_head_t *q, wait_queue_t *wait)
+___add_wait_queue_tail_exclusive(wait_queue_head_t *q, wait_queue_t *wait)
{
wait->flags |= WQ_FLAG_EXCLUSIVE;
- __add_wait_queue_tail(q, wait);
+ ___add_wait_queue_tail(q, wait);
}
static inline void
-__remove_wait_queue(wait_queue_head_t *head, wait_queue_t *old)
+___remove_wait_queue(wait_queue_head_t *head, wait_queue_t *old)
{
list_del(&old->task_list);
}
+#define __add_wait_queue(q, w) \
+ wait_tchk_qw(___add_wait_queue, q, w)
+#define __add_wait_queue_exclusive(q, w) \
+ wait_tchk_qw(___add_wait_queue_exclusive, q, w)
+#define __add_wait_queue_tail(q, w) \
+ wait_tchk_qw(___add_wait_queue_tail, q, w)
+#define __add_wait_queue_tail_exclusive(q, w) \
+ wait_tchk_qw(___add_wait_queue_tail_exclusive, q, w)
+#define __remove_wait_queue(q, w) \
+ wait_tchk_qw(___remove_wait_queue, q, w)
+
typedef int wait_bit_action_f(struct wait_bit_key *);
-void __wake_up(wait_queue_head_t *q, unsigned int mode, int nr, void *key);
-void __wake_up_locked_key(wait_queue_head_t *q, unsigned int mode, void *key);
-void __wake_up_sync_key(wait_queue_head_t *q, unsigned int mode, int nr, void *key);
-void __wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr);
-void __wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr);
-void __wake_up_bit(wait_queue_head_t *, void *, int);
-int __wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, wait_bit_action_f *, unsigned);
-int __wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, wait_bit_action_f *, unsigned);
+void ___wake_up(wait_queue_head_t *q, unsigned int mode, int nr, void *key);
+void ___wake_up_locked_key(wait_queue_head_t *q, unsigned int mode, void *key);
+void ___wake_up_sync_key(wait_queue_head_t *q, unsigned int mode, int nr, void *key);
+void ___wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr);
+void ___wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr);
+void ___wake_up_bit(wait_queue_head_t *, void *, int);
+int ___wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, wait_bit_action_f *, unsigned);
+int ___wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, wait_bit_action_f *, unsigned);
void wake_up_bit(void *, int);
void wake_up_atomic_t(atomic_t *);
int out_of_line_wait_on_bit(void *, int, wait_bit_action_f *, unsigned);
@@ -162,16 +231,42 @@ int out_of_line_wait_on_bit_lock(void *, int, wait_bit_action_f *, unsigned);
int out_of_line_wait_on_atomic_t(atomic_t *, int (*)(atomic_t *), unsigned);
wait_queue_head_t *bit_waitqueue(void *, int);
-#define wake_up(x) __wake_up(x, TASK_NORMAL, 1, NULL)
-#define wake_up_nr(x, nr) __wake_up(x, TASK_NORMAL, nr, NULL)
-#define wake_up_all(x) __wake_up(x, TASK_NORMAL, 0, NULL)
-#define wake_up_locked(x) __wake_up_locked((x), TASK_NORMAL, 1)
-#define wake_up_all_locked(x) __wake_up_locked((x), TASK_NORMAL, 0)
-
-#define wake_up_interruptible(x) __wake_up(x, TASK_INTERRUPTIBLE, 1, NULL)
-#define wake_up_interruptible_nr(x, nr) __wake_up(x, TASK_INTERRUPTIBLE, nr, NULL)
-#define wake_up_interruptible_all(x) __wake_up(x, TASK_INTERRUPTIBLE, 0, NULL)
-#define wake_up_interruptible_sync(x) __wake_up_sync((x), TASK_INTERRUPTIBLE, 1)
+#define __wake_up(q, m, n, k) \
+ wait_tchk_q(___wake_up, q, m, n, k)
+#define __wake_up_locked_key(q, m, k) \
+ wait_tchk_q(___wake_up_locked_key, q, m, k)
+#define __wake_up_sync_key(q, m, n, k) \
+ wait_tchk_q(___wake_up_sync_key, q, m, n, k)
+#define __wake_up_locked(q, m, n) \
+ wait_tchk_q(___wake_up_locked, q, m, n)
+#define __wake_up_sync(q, m, n) \
+ wait_tchk_q(___wake_up_sync, q, m, n)
+#define __wake_up_bit(q, w, b) \
+ wait_tchk_q(___wake_up_bit, q, w, b)
+#define __wait_on_bit(q, w, a, m) \
+ wait_tchk_ret_q(___wait_on_bit, q, w, a, m)
+#define __wait_on_bit_lock(q, w, a, m) \
+ wait_tchk_ret_q(___wait_on_bit_lock, q, w, a, m)
+
+#define wake_up(q) \
+ wait_tchk_q(___wake_up, q, TASK_NORMAL, 1, NULL)
+#define wake_up_nr(q, n) \
+ wait_tchk_q(___wake_up, q, TASK_NORMAL, n, NULL)
+#define wake_up_all(q) \
+ wait_tchk_q(___wake_up, q, TASK_NORMAL, 0, NULL)
+#define wake_up_locked(q) \
+ wait_tchk_q(___wake_up_locked, q, TASK_NORMAL, 1)
+#define wake_up_all_locked(q) \
+ wait_tchk_q(___wake_up_locked, q, TASK_NORMAL, 0)
+
+#define wake_up_interruptible(q) \
+ wait_tchk_q(___wake_up, q, TASK_INTERRUPTIBLE, 1, NULL)
+#define wake_up_interruptible_nr(q, n) \
+ wait_tchk_q(___wake_up, q, TASK_INTERRUPTIBLE, n, NULL)
+#define wake_up_interruptible_all(q) \
+ wait_tchk_q(___wake_up, q, TASK_INTERRUPTIBLE, 0, NULL)
+#define wake_up_interruptible_sync(q) \
+ wait_tchk_q(___wake_up_sync, q, TASK_INTERRUPTIBLE, 1)
/*
* Wakeup macros to be used to report events to the targets.
@@ -198,6 +293,30 @@ wait_queue_head_t *bit_waitqueue(void *, int);
state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE) \
/*
+ * Waitqueues which are removed from the waitqueue_head at wakeup time
+ */
+void _prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state);
+void _prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state);
+long _prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state);
+void _finish_wait(wait_queue_head_t *q, wait_queue_t *wait);
+void _abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait, unsigned int mode, void *key);
+long wait_woken(wait_queue_t *wait, unsigned mode, long timeout);
+int woken_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
+int autoremove_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
+int wake_bit_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
+
+#define prepare_to_wait(q, w, s) \
+ wait_tchk_qw(_prepare_to_wait, q, w, s)
+#define prepare_to_wait_exclusive(q, w, s) \
+ wait_tchk_qw(_prepare_to_wait_exclusive, q, w, s)
+#define prepare_to_wait_event(q, w, s) \
+ wait_tchk_ret_qw(_prepare_to_wait_event, q, w, s)
+#define finish_wait(q, w) \
+ wait_tchk_qw(_finish_wait, q, w)
+#define abort_exclusive_wait(q, w, m, k) \
+ wait_tchk_qw(_abort_exclusive_wait, q, w, m, k)
+
+/*
* The below macro ___wait_event() has an explicit shadow of the __ret
* variable when used from the wait_event_*() macros.
*
@@ -918,19 +1037,6 @@ do { \
__ret; \
})
-/*
- * Waitqueues which are removed from the waitqueue_head at wakeup time
- */
-void prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state);
-void prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state);
-long prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state);
-void finish_wait(wait_queue_head_t *q, wait_queue_t *wait);
-void abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait, unsigned int mode, void *key);
-long wait_woken(wait_queue_t *wait, unsigned mode, long timeout);
-int woken_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
-int autoremove_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
-int wake_bit_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
-
#define DEFINE_WAIT_FUNC(name, function) \
wait_queue_t name = { \
.private = current, \
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
index 82f0dff..ca91043 100644
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -1,14 +1,14 @@
#include <linux/sched.h>
#include <linux/swait.h>
-void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+void ___init_swait_queue_head(struct swait_queue_head *q, const char *name,
struct lock_class_key *key)
{
raw_spin_lock_init(&q->lock);
lockdep_set_class_and_name(&q->lock, key, name);
INIT_LIST_HEAD(&q->task_list);
}
-EXPORT_SYMBOL(__init_swait_queue_head);
+EXPORT_SYMBOL(___init_swait_queue_head);
/*
* The thing about the wake_up_state() return value; I think we can ignore it.
@@ -16,7 +16,7 @@ EXPORT_SYMBOL(__init_swait_queue_head);
* If for some reason it would return 0, that means the previously waiting
* task is already running, so it will observe condition true (or has already).
*/
-void swake_up_locked(struct swait_queue_head *q)
+void _swake_up_locked(struct swait_queue_head *q)
{
struct swait_queue *curr;
@@ -27,31 +27,31 @@ void swake_up_locked(struct swait_queue_head *q)
wake_up_process(curr->task);
list_del_init(&curr->task_list);
}
-EXPORT_SYMBOL(swake_up_locked);
+EXPORT_SYMBOL(_swake_up_locked);
-void swake_up(struct swait_queue_head *q)
+void _swake_up(struct swait_queue_head *q)
{
unsigned long flags;
- if (!swait_active(q))
+ if (!_swait_active(q))
return;
raw_spin_lock_irqsave(&q->lock, flags);
- swake_up_locked(q);
+ _swake_up_locked(q);
raw_spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(swake_up);
+EXPORT_SYMBOL(_swake_up);
/*
* Does not allow usage from IRQ disabled, since we must be able to
* release IRQs to guarantee bounded hold time.
*/
-void swake_up_all(struct swait_queue_head *q)
+void _swake_up_all(struct swait_queue_head *q)
{
struct swait_queue *curr;
LIST_HEAD(tmp);
- if (!swait_active(q))
+ if (!_swait_active(q))
return;
raw_spin_lock_irq(&q->lock);
@@ -70,45 +70,45 @@ void swake_up_all(struct swait_queue_head *q)
}
raw_spin_unlock_irq(&q->lock);
}
-EXPORT_SYMBOL(swake_up_all);
+EXPORT_SYMBOL(_swake_up_all);
-void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+void ___prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
{
wait->task = current;
if (list_empty(&wait->task_list))
list_add(&wait->task_list, &q->task_list);
}
-void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
+void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
{
unsigned long flags;
raw_spin_lock_irqsave(&q->lock, flags);
- __prepare_to_swait(q, wait);
+ ___prepare_to_swait(q, wait);
set_current_state(state);
raw_spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(prepare_to_swait);
+EXPORT_SYMBOL(__prepare_to_swait);
-long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
+long _prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
{
if (signal_pending_state(state, current))
return -ERESTARTSYS;
- prepare_to_swait(q, wait, state);
+ __prepare_to_swait(q, wait, state);
return 0;
}
-EXPORT_SYMBOL(prepare_to_swait_event);
+EXPORT_SYMBOL(_prepare_to_swait_event);
-void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+void ___finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
{
__set_current_state(TASK_RUNNING);
if (!list_empty(&wait->task_list))
list_del_init(&wait->task_list);
}
-void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
{
unsigned long flags;
@@ -120,4 +120,4 @@ void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
raw_spin_unlock_irqrestore(&q->lock, flags);
}
}
-EXPORT_SYMBOL(finish_swait);
+EXPORT_SYMBOL(__finish_swait);
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 052e026..02c69dc 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -11,46 +11,46 @@
#include <linux/hash.h>
#include <linux/kthread.h>
-void __init_waitqueue_head(wait_queue_head_t *q, const char *name, struct lock_class_key *key)
+void ___init_waitqueue_head(wait_queue_head_t *q, const char *name, struct lock_class_key *key)
{
spin_lock_init(&q->lock);
lockdep_set_class_and_name(&q->lock, key, name);
INIT_LIST_HEAD(&q->task_list);
}
-EXPORT_SYMBOL(__init_waitqueue_head);
+EXPORT_SYMBOL(___init_waitqueue_head);
-void add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait)
+void _add_wait_queue(wait_queue_head_t *q, wait_queue_t *wait)
{
unsigned long flags;
wait->flags &= ~WQ_FLAG_EXCLUSIVE;
spin_lock_irqsave(&q->lock, flags);
- __add_wait_queue(q, wait);
+ ___add_wait_queue(q, wait);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(add_wait_queue);
+EXPORT_SYMBOL(_add_wait_queue);
-void add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait)
+void _add_wait_queue_exclusive(wait_queue_head_t *q, wait_queue_t *wait)
{
unsigned long flags;
wait->flags |= WQ_FLAG_EXCLUSIVE;
spin_lock_irqsave(&q->lock, flags);
- __add_wait_queue_tail(q, wait);
+ ___add_wait_queue_tail(q, wait);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(add_wait_queue_exclusive);
+EXPORT_SYMBOL(_add_wait_queue_exclusive);
-void remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait)
+void _remove_wait_queue(wait_queue_head_t *q, wait_queue_t *wait)
{
unsigned long flags;
spin_lock_irqsave(&q->lock, flags);
- __remove_wait_queue(q, wait);
+ ___remove_wait_queue(q, wait);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(remove_wait_queue);
+EXPORT_SYMBOL(_remove_wait_queue);
/*
@@ -86,7 +86,7 @@ static void __wake_up_common(wait_queue_head_t *q, unsigned int mode,
* It may be assumed that this function implies a write memory barrier before
* changing the task state if and only if any tasks are woken up.
*/
-void __wake_up(wait_queue_head_t *q, unsigned int mode,
+void ___wake_up(wait_queue_head_t *q, unsigned int mode,
int nr_exclusive, void *key)
{
unsigned long flags;
@@ -95,22 +95,22 @@ void __wake_up(wait_queue_head_t *q, unsigned int mode,
__wake_up_common(q, mode, nr_exclusive, 0, key);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(__wake_up);
+EXPORT_SYMBOL(___wake_up);
/*
* Same as __wake_up but called with the spinlock in wait_queue_head_t held.
*/
-void __wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr)
+void ___wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr)
{
__wake_up_common(q, mode, nr, 0, NULL);
}
-EXPORT_SYMBOL_GPL(__wake_up_locked);
+EXPORT_SYMBOL_GPL(___wake_up_locked);
-void __wake_up_locked_key(wait_queue_head_t *q, unsigned int mode, void *key)
+void ___wake_up_locked_key(wait_queue_head_t *q, unsigned int mode, void *key)
{
__wake_up_common(q, mode, 1, 0, key);
}
-EXPORT_SYMBOL_GPL(__wake_up_locked_key);
+EXPORT_SYMBOL_GPL(___wake_up_locked_key);
/**
* __wake_up_sync_key - wake up threads blocked on a waitqueue.
@@ -129,7 +129,7 @@ EXPORT_SYMBOL_GPL(__wake_up_locked_key);
* It may be assumed that this function implies a write memory barrier before
* changing the task state if and only if any tasks are woken up.
*/
-void __wake_up_sync_key(wait_queue_head_t *q, unsigned int mode,
+void ___wake_up_sync_key(wait_queue_head_t *q, unsigned int mode,
int nr_exclusive, void *key)
{
unsigned long flags;
@@ -145,16 +145,16 @@ void __wake_up_sync_key(wait_queue_head_t *q, unsigned int mode,
__wake_up_common(q, mode, nr_exclusive, wake_flags, key);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL_GPL(__wake_up_sync_key);
+EXPORT_SYMBOL_GPL(___wake_up_sync_key);
/*
- * __wake_up_sync - see __wake_up_sync_key()
+ * ___wake_up_sync - see __wake_up_sync_key()
*/
-void __wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr_exclusive)
+void ___wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr_exclusive)
{
- __wake_up_sync_key(q, mode, nr_exclusive, NULL);
+ ___wake_up_sync_key(q, mode, nr_exclusive, NULL);
}
-EXPORT_SYMBOL_GPL(__wake_up_sync); /* For internal use only */
+EXPORT_SYMBOL_GPL(___wake_up_sync); /* For internal use only */
/*
* Note: we use "set_current_state()" _after_ the wait-queue add,
@@ -169,34 +169,34 @@ EXPORT_SYMBOL_GPL(__wake_up_sync); /* For internal use only */
* loads to move into the critical region).
*/
void
-prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state)
+_prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state)
{
unsigned long flags;
wait->flags &= ~WQ_FLAG_EXCLUSIVE;
spin_lock_irqsave(&q->lock, flags);
if (list_empty(&wait->task_list))
- __add_wait_queue(q, wait);
+ ___add_wait_queue(q, wait);
set_current_state(state);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(prepare_to_wait);
+EXPORT_SYMBOL(_prepare_to_wait);
void
-prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state)
+_prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state)
{
unsigned long flags;
wait->flags |= WQ_FLAG_EXCLUSIVE;
spin_lock_irqsave(&q->lock, flags);
if (list_empty(&wait->task_list))
- __add_wait_queue_tail(q, wait);
+ ___add_wait_queue_tail(q, wait);
set_current_state(state);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(prepare_to_wait_exclusive);
+EXPORT_SYMBOL(_prepare_to_wait_exclusive);
-long prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state)
+long _prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state)
{
unsigned long flags;
@@ -209,19 +209,19 @@ long prepare_to_wait_event(wait_queue_head_t *q, wait_queue_t *wait, int state)
spin_lock_irqsave(&q->lock, flags);
if (list_empty(&wait->task_list)) {
if (wait->flags & WQ_FLAG_EXCLUSIVE)
- __add_wait_queue_tail(q, wait);
+ ___add_wait_queue_tail(q, wait);
else
- __add_wait_queue(q, wait);
+ ___add_wait_queue(q, wait);
}
set_current_state(state);
spin_unlock_irqrestore(&q->lock, flags);
return 0;
}
-EXPORT_SYMBOL(prepare_to_wait_event);
+EXPORT_SYMBOL(_prepare_to_wait_event);
/**
- * finish_wait - clean up after waiting in a queue
+ * _finish_wait - clean up after waiting in a queue
* @q: waitqueue waited on
* @wait: wait descriptor
*
@@ -229,7 +229,7 @@ EXPORT_SYMBOL(prepare_to_wait_event);
* the wait descriptor from the given waitqueue if still
* queued.
*/
-void finish_wait(wait_queue_head_t *q, wait_queue_t *wait)
+void _finish_wait(wait_queue_head_t *q, wait_queue_t *wait)
{
unsigned long flags;
@@ -253,10 +253,10 @@ void finish_wait(wait_queue_head_t *q, wait_queue_t *wait)
spin_unlock_irqrestore(&q->lock, flags);
}
}
-EXPORT_SYMBOL(finish_wait);
+EXPORT_SYMBOL(_finish_wait);
/**
- * abort_exclusive_wait - abort exclusive waiting in a queue
+ * _abort_exclusive_wait - abort exclusive waiting in a queue
* @q: waitqueue waited on
* @wait: wait descriptor
* @mode: runstate of the waiter to be woken
@@ -273,7 +273,7 @@ EXPORT_SYMBOL(finish_wait);
* aborts and is woken up concurrently and no one wakes up
* the next waiter.
*/
-void abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait,
+void _abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait,
unsigned int mode, void *key)
{
unsigned long flags;
@@ -286,7 +286,7 @@ void abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait,
__wake_up_locked_key(q, mode, key);
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(abort_exclusive_wait);
+EXPORT_SYMBOL(_abort_exclusive_wait);
int autoremove_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key)
{
@@ -384,7 +384,7 @@ EXPORT_SYMBOL(wake_bit_function);
* permitted return codes. Nonzero return codes halt waiting and return.
*/
int __sched
-__wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
+___wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
wait_bit_action_f *action, unsigned mode)
{
int ret = 0;
@@ -394,10 +394,10 @@ __wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
if (test_bit(q->key.bit_nr, q->key.flags))
ret = (*action)(&q->key);
} while (test_bit(q->key.bit_nr, q->key.flags) && !ret);
- finish_wait(wq, &q->wait);
+ _finish_wait(wq, &q->wait);
return ret;
}
-EXPORT_SYMBOL(__wait_on_bit);
+EXPORT_SYMBOL(___wait_on_bit);
int __sched out_of_line_wait_on_bit(void *word, int bit,
wait_bit_action_f *action, unsigned mode)
@@ -405,7 +405,7 @@ int __sched out_of_line_wait_on_bit(void *word, int bit,
wait_queue_head_t *wq = bit_waitqueue(word, bit);
DEFINE_WAIT_BIT(wait, word, bit);
- return __wait_on_bit(wq, &wait, action, mode);
+ return ___wait_on_bit(wq, &wait, action, mode);
}
EXPORT_SYMBOL(out_of_line_wait_on_bit);
@@ -417,30 +417,30 @@ int __sched out_of_line_wait_on_bit_timeout(
DEFINE_WAIT_BIT(wait, word, bit);
wait.key.timeout = jiffies + timeout;
- return __wait_on_bit(wq, &wait, action, mode);
+ return ___wait_on_bit(wq, &wait, action, mode);
}
EXPORT_SYMBOL_GPL(out_of_line_wait_on_bit_timeout);
int __sched
-__wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
+___wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
wait_bit_action_f *action, unsigned mode)
{
do {
int ret;
- prepare_to_wait_exclusive(wq, &q->wait, mode);
+ _prepare_to_wait_exclusive(wq, &q->wait, mode);
if (!test_bit(q->key.bit_nr, q->key.flags))
continue;
ret = action(&q->key);
if (!ret)
continue;
- abort_exclusive_wait(wq, &q->wait, mode, &q->key);
+ _abort_exclusive_wait(wq, &q->wait, mode, &q->key);
return ret;
} while (test_and_set_bit(q->key.bit_nr, q->key.flags));
- finish_wait(wq, &q->wait);
+ _finish_wait(wq, &q->wait);
return 0;
}
-EXPORT_SYMBOL(__wait_on_bit_lock);
+EXPORT_SYMBOL(___wait_on_bit_lock);
int __sched out_of_line_wait_on_bit_lock(void *word, int bit,
wait_bit_action_f *action, unsigned mode)
@@ -452,13 +452,13 @@ int __sched out_of_line_wait_on_bit_lock(void *word, int bit,
}
EXPORT_SYMBOL(out_of_line_wait_on_bit_lock);
-void __wake_up_bit(wait_queue_head_t *wq, void *word, int bit)
+void ___wake_up_bit(wait_queue_head_t *wq, void *word, int bit)
{
struct wait_bit_key key = __WAIT_BIT_KEY_INITIALIZER(word, bit);
if (waitqueue_active(wq))
__wake_up(wq, TASK_NORMAL, 1, &key);
}
-EXPORT_SYMBOL(__wake_up_bit);
+EXPORT_SYMBOL(___wake_up_bit);
/**
* wake_up_bit - wake up a waiter on a bit
@@ -479,7 +479,7 @@ EXPORT_SYMBOL(__wake_up_bit);
*/
void wake_up_bit(void *word, int bit)
{
- __wake_up_bit(bit_waitqueue(word, bit), word, bit);
+ ___wake_up_bit(bit_waitqueue(word, bit), word, bit);
}
EXPORT_SYMBOL(wake_up_bit);
@@ -541,7 +541,7 @@ int __wait_on_atomic_t(wait_queue_head_t *wq, struct wait_bit_queue *q,
break;
ret = (*action)(val);
} while (!ret && atomic_read(val) != 0);
- finish_wait(wq, &q->wait);
+ _finish_wait(wq, &q->wait);
return ret;
}
@@ -577,7 +577,7 @@ EXPORT_SYMBOL(out_of_line_wait_on_atomic_t);
*/
void wake_up_atomic_t(atomic_t *p)
{
- __wake_up_bit(atomic_t_waitqueue(p), p, WAIT_ATOMIC_T_BIT_NR);
+ ___wake_up_bit(atomic_t_waitqueue(p), p, WAIT_ATOMIC_T_BIT_NR);
}
EXPORT_SYMBOL(wake_up_atomic_t);
--
2.4.3
On Thu, 26 Nov 2015, Daniel Wagner wrote:
> On 11/24/2015 02:03 PM, Daniel Wagner wrote:
> > The API provided by wait.h and swait.h is very similiar. Most of the
> > time your are only one character away from either of it:
> >
> > wake_up() vs swake_up()
> >
> > This is on purpose so that we do not have two nearly identical bits of
> > infrastructre code with dissimilar names.
> >
> > A compile time type check assertion ensures that obvious wrong usage
> > is caught at early stage.
>
> Obviously, this didn't really work as one can see with patch #4. That
> one just compiled. So I wrapped almost all functions to get a better
> check coverage. woken_wake_function(), autoremove_wake_function() and
> wake_bit_function() can't be wrapped easily because DEFINE_WAIT and
> friends. I just left them out.
>
> The result looks pretty bad in my opinion. Probably it would be
> better do add -Werror=incompatible-pointer-types to the CFLAGS.
That's really bad.
If we can pull off the -Werror=incompatible-pointer-types trick, that
would solve it nicely.
Thanks,
tglx