All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] sched/swait: Convert to full exclusive mode
@ 2018-06-12  8:34 Peter Zijlstra
  2018-06-12  8:34 ` [PATCH 1/3] sched/swait: Remove __prepare_to_swait Peter Zijlstra
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Peter Zijlstra @ 2018-06-12  8:34 UTC (permalink / raw)
  To: mingo, linux-kernel
  Cc: peterz, torvalds, tglx, bigeasy, oleg, paulmck, pbonzini

As mentioned by Linus, swait is exclusive mode and had better behave like it
and be named like it.

Make it so.

---
 arch/mips/kvm/mips.c         |  4 ++--
 arch/powerpc/kvm/book3s_hv.c |  6 +++---
 arch/s390/kvm/interrupt.c    |  2 +-
 arch/x86/kernel/kvm.c        |  4 ++--
 arch/x86/kvm/lapic.c         |  2 +-
 include/linux/swait.h        | 36 ++++++++++++++++++------------------
 kernel/power/suspend.c       |  4 ++--
 kernel/rcu/srcutiny.c        |  4 ++--
 kernel/rcu/tree.c            |  8 ++++----
 kernel/rcu/tree_exp.h        |  4 ++--
 kernel/rcu/tree_plugin.h     | 12 ++++++------
 kernel/sched/swait.c         | 32 ++++++++++++++++++++++----------
 virt/kvm/arm/arm.c           |  4 ++--
 virt/kvm/arm/psci.c          |  2 +-
 virt/kvm/async_pf.c          |  2 +-
 virt/kvm/kvm_main.c          |  4 ++--
 16 files changed, 71 insertions(+), 59 deletions(-)



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/3] sched/swait: Remove __prepare_to_swait
  2018-06-12  8:34 [PATCH 0/3] sched/swait: Convert to full exclusive mode Peter Zijlstra
@ 2018-06-12  8:34 ` Peter Zijlstra
  2018-06-20  9:39   ` [tip:sched/core] " tip-bot for Peter Zijlstra
  2018-06-12  8:34 ` [PATCH 2/3] sched/swait: Switch to full exclusive mode Peter Zijlstra
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Peter Zijlstra @ 2018-06-12  8:34 UTC (permalink / raw)
  To: mingo, linux-kernel
  Cc: peterz, torvalds, tglx, bigeasy, oleg, paulmck, pbonzini

[-- Attachment #1: peterz-swait-0.patch --]
[-- Type: text/plain, Size: 1126 bytes --]

There is no public user of this API, remove it.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/swait.h |    1 -
 kernel/sched/swait.c  |    2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -161,7 +161,6 @@ extern void swake_up(struct swait_queue_
 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);
 
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -69,7 +69,7 @@ void swake_up_all(struct swait_queue_hea
 }
 EXPORT_SYMBOL(swake_up_all);
 
-void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+static void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
 {
 	wait->task = current;
 	if (list_empty(&wait->task_list))



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/3] sched/swait: Switch to full exclusive mode
  2018-06-12  8:34 [PATCH 0/3] sched/swait: Convert to full exclusive mode Peter Zijlstra
  2018-06-12  8:34 ` [PATCH 1/3] sched/swait: Remove __prepare_to_swait Peter Zijlstra
@ 2018-06-12  8:34 ` Peter Zijlstra
  2018-06-20  9:40   ` [tip:sched/core] " tip-bot for Peter Zijlstra
  2018-06-12  8:34 ` [PATCH 3/3] sched/swait: Rename to exclusive Peter Zijlstra
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Peter Zijlstra @ 2018-06-12  8:34 UTC (permalink / raw)
  To: mingo, linux-kernel
  Cc: peterz, torvalds, tglx, bigeasy, oleg, paulmck, pbonzini

[-- Attachment #1: peterz-swait-1.patch --]
[-- Type: text/plain, Size: 3397 bytes --]

Linus noted that swait basically implements exclusive mode -- because
swake_up() only wakes a single waiter. And because of that it should
take care to properly deal with the interruptible case.

In short, the problem is that swake_up() can race with a signal. In
this this case it is possible the swake_up() 'wakes' the waiter that
is already on the way out because it just got a signal and the wakeup
gets lost.

The normal wait code is very careful and avoids this situation, make
sure we do too.

Copy the exact exclusive semantics from wait.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/swait.h |   11 ++++++-----
 kernel/sched/swait.c  |   22 +++++++++++++++++-----
 2 files changed, 23 insertions(+), 10 deletions(-)

--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -38,8 +38,8 @@
  *    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.
+ *  - the !exclusive mode; because that leads to O(n) wakeups, everything is
+ *    exclusive.
  *
  *  - custom wake callback functions; because you cannot give any guarantees
  *    about random code. This also allows swait to be used in RT, such that
@@ -167,9 +167,10 @@ extern long prepare_to_swait_event(struc
 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" */
+/* as per ___wait_event() but for swait, therefore "exclusive == 1" */
 #define ___swait_event(wq, condition, state, ret, cmd)			\
 ({									\
+	__label__ __out;						\
 	struct swait_queue __wait;					\
 	long __ret = ret;						\
 									\
@@ -182,13 +183,13 @@ extern void finish_swait(struct swait_qu
 									\
 		if (___wait_is_interruptible(state) && __int) {		\
 			__ret = __int;					\
-			break;						\
+			goto __out;					\
 		}							\
 									\
 		cmd;							\
 	}								\
 	finish_swait(&wq, &__wait);					\
-	__ret;								\
+__out:	__ret;								\
 })
 
 #define __swait_event(wq, condition)					\
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -73,7 +73,7 @@ static void __prepare_to_swait(struct sw
 {
 	wait->task = current;
 	if (list_empty(&wait->task_list))
-		list_add(&wait->task_list, &q->task_list);
+		list_add_tail(&wait->task_list, &q->task_list);
 }
 
 void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
@@ -89,12 +89,24 @@ 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;
+	unsigned long flags;
+	long ret = 0;
 
-	prepare_to_swait(q, wait, state);
+	raw_spin_lock_irqsave(&q->lock, flags);
+	if (unlikely(signal_pending_state(state, current))) {
+		/*
+		 * See prepare_to_wait_event(). TL;DR, subsequent swake_up()
+		 * must not see us.
+		 */
+		list_del_init(&wait->task_list);
+		ret = -ERESTARTSYS;
+	} else {
+		__prepare_to_swait(q, wait);
+		set_current_state(state);
+	}
+	raw_spin_unlock_irqrestore(&q->lock, flags);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(prepare_to_swait_event);
 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/3] sched/swait: Rename to exclusive
  2018-06-12  8:34 [PATCH 0/3] sched/swait: Convert to full exclusive mode Peter Zijlstra
  2018-06-12  8:34 ` [PATCH 1/3] sched/swait: Remove __prepare_to_swait Peter Zijlstra
  2018-06-12  8:34 ` [PATCH 2/3] sched/swait: Switch to full exclusive mode Peter Zijlstra
@ 2018-06-12  8:34 ` Peter Zijlstra
  2018-06-20  9:40   ` [tip:sched/core] " tip-bot for Peter Zijlstra
  2018-06-12 16:47 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Linus Torvalds
  2018-06-14  1:27 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Paul E. McKenney
  4 siblings, 1 reply; 22+ messages in thread
From: Peter Zijlstra @ 2018-06-12  8:34 UTC (permalink / raw)
  To: mingo, linux-kernel
  Cc: peterz, torvalds, tglx, bigeasy, oleg, paulmck, pbonzini

[-- Attachment #1: peterz-swait-2.patch --]
[-- Type: text/plain, Size: 16811 bytes --]

Since swait basically implemented exclusive waits only, make sure
the API reflects that.

  $ git grep -l -e "\<swake_up\>"
		-e "\<swait_event[^ (]*"
		-e "\<prepare_to_swait\>" | while read file;
    do
	sed -i -e 's/\<swake_up\>/&_one/g'
	       -e 's/\<swait_event[^ (]*/&_exclusive/g'
	       -e 's/\<prepare_to_swait\>/&_exclusive/g' $file;
    done

With a few manual touch-ups.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/mips/kvm/mips.c         |    4 ++--
 arch/powerpc/kvm/book3s_hv.c |    6 +++---
 arch/s390/kvm/interrupt.c    |    2 +-
 arch/x86/kernel/kvm.c        |    4 ++--
 arch/x86/kvm/lapic.c         |    2 +-
 include/linux/swait.h        |   24 ++++++++++++------------
 kernel/power/suspend.c       |    4 ++--
 kernel/rcu/srcutiny.c        |    4 ++--
 kernel/rcu/tree.c            |    8 ++++----
 kernel/rcu/tree_exp.h        |    4 ++--
 kernel/rcu/tree_plugin.h     |   12 ++++++------
 kernel/sched/swait.c         |   10 +++++-----
 virt/kvm/arm/arm.c           |    4 ++--
 virt/kvm/arm/psci.c          |    2 +-
 virt/kvm/async_pf.c          |    2 +-
 virt/kvm/kvm_main.c          |    4 ++--
 16 files changed, 48 insertions(+), 48 deletions(-)

--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -515,7 +515,7 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_
 	dvcpu->arch.wait = 0;
 
 	if (swq_has_sleeper(&dvcpu->wq))
-		swake_up(&dvcpu->wq);
+		swake_up_one(&dvcpu->wq);
 
 	return 0;
 }
@@ -1204,7 +1204,7 @@ static void kvm_mips_comparecount_func(u
 
 	vcpu->arch.wait = 0;
 	if (swq_has_sleeper(&vcpu->wq))
-		swake_up(&vcpu->wq);
+		swake_up_one(&vcpu->wq);
 }
 
 /* low level hrtimer wake routine */
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -190,7 +190,7 @@ static void kvmppc_fast_vcpu_kick_hv(str
 
 	wqp = kvm_arch_vcpu_wq(vcpu);
 	if (swq_has_sleeper(wqp)) {
-		swake_up(wqp);
+		swake_up_one(wqp);
 		++vcpu->stat.halt_wakeup;
 	}
 
@@ -3106,7 +3106,7 @@ static void kvmppc_vcore_blocked(struct
 		}
 	}
 
-	prepare_to_swait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
+	prepare_to_swait_exclusive(&vc->wq, &wait, TASK_INTERRUPTIBLE);
 
 	if (kvmppc_vcore_check_block(vc)) {
 		finish_swait(&vc->wq, &wait);
@@ -3229,7 +3229,7 @@ static int kvmppc_run_vcpu(struct kvm_ru
 			kvmppc_start_thread(vcpu, vc);
 			trace_kvm_guest_enter(vcpu);
 		} else if (vc->vcore_state == VCORE_SLEEPING) {
-			swake_up(&vc->wq);
+			swake_up_one(&vc->wq);
 		}
 
 	}
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1145,7 +1145,7 @@ void kvm_s390_vcpu_wakeup(struct kvm_vcp
 		 * yield-candidate.
 		 */
 		vcpu->preempted = true;
-		swake_up(&vcpu->wq);
+		swake_up_one(&vcpu->wq);
 		vcpu->stat.halt_wakeup++;
 	}
 	/*
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -154,7 +154,7 @@ void kvm_async_pf_task_wait(u32 token, i
 
 	for (;;) {
 		if (!n.halted)
-			prepare_to_swait(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
+			prepare_to_swait_exclusive(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
 		if (hlist_unhashed(&n.link))
 			break;
 
@@ -188,7 +188,7 @@ static void apf_task_wake_one(struct kvm
 	if (n->halted)
 		smp_send_reschedule(n->cpu);
 	else if (swq_has_sleeper(&n->wq))
-		swake_up(&n->wq);
+		swake_up_one(&n->wq);
 }
 
 static void apf_task_wake_all(void)
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1379,7 +1379,7 @@ static void apic_timer_expired(struct kv
 	 * using swait_active() is safe.
 	 */
 	if (swait_active(q))
-		swake_up(q);
+		swake_up_one(q);
 
 	if (apic_lvtt_tscdeadline(apic))
 		ktimer->expired_tscdeadline = ktimer->tscdeadline;
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -16,7 +16,7 @@
  * wait-queues, but the semantics are actually completely different, and
  * every single user we have ever had has been buggy (or pointless).
  *
- * A "swake_up()" only wakes up _one_ waiter, which is not at all what
+ * A "swake_up_one()" only wakes up _one_ waiter, which is not at all what
  * "wake_up()" does, and has led to problems. In other cases, it has
  * been fine, because there's only ever one waiter (kvm), but in that
  * case gthe whole "simple" wait-queue is just pointless to begin with,
@@ -115,7 +115,7 @@ extern void __init_swait_queue_head(stru
  *      CPU0 - waker                    CPU1 - waiter
  *
  *                                      for (;;) {
- *      @cond = true;                     prepare_to_swait(&wq_head, &wait, state);
+ *      @cond = true;                     prepare_to_swait_exclusive(&wq_head, &wait, state);
  *      smp_mb();                         // smp_mb() from set_current_state()
  *      if (swait_active(wq_head))        if (@cond)
  *        wake_up(wq_head);                      break;
@@ -157,11 +157,11 @@ static inline bool swq_has_sleeper(struc
 	return swait_active(wq);
 }
 
-extern void swake_up(struct swait_queue_head *q);
+extern void swake_up_one(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, int state);
+extern void prepare_to_swait_exclusive(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);
@@ -196,7 +196,7 @@ __out:	__ret;								\
 	(void)___swait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0,	\
 			    schedule())
 
-#define swait_event(wq, condition)					\
+#define swait_event_exclusive(wq, condition)				\
 do {									\
 	if (condition)							\
 		break;							\
@@ -208,7 +208,7 @@ do {									\
 		      TASK_UNINTERRUPTIBLE, timeout,			\
 		      __ret = schedule_timeout(__ret))
 
-#define swait_event_timeout(wq, condition, timeout)			\
+#define swait_event_timeout_exclusive(wq, condition, timeout)		\
 ({									\
 	long __ret = timeout;						\
 	if (!___wait_cond_timeout(condition))				\
@@ -220,7 +220,7 @@ do {									\
 	___swait_event(wq, condition, TASK_INTERRUPTIBLE, 0,		\
 		      schedule())
 
-#define swait_event_interruptible(wq, condition)			\
+#define swait_event_interruptible_exclusive(wq, condition)		\
 ({									\
 	int __ret = 0;							\
 	if (!(condition))						\
@@ -233,7 +233,7 @@ do {									\
 		      TASK_INTERRUPTIBLE, timeout,			\
 		      __ret = schedule_timeout(__ret))
 
-#define swait_event_interruptible_timeout(wq, condition, timeout)	\
+#define swait_event_interruptible_timeout_exclusive(wq, condition, timeout)\
 ({									\
 	long __ret = timeout;						\
 	if (!___wait_cond_timeout(condition))				\
@@ -246,7 +246,7 @@ do {									\
 	(void)___swait_event(wq, condition, TASK_IDLE, 0, schedule())
 
 /**
- * swait_event_idle - wait without system load contribution
+ * swait_event_idle_exclusive - wait without system load contribution
  * @wq: the waitqueue to wait on
  * @condition: a C expression for the event to wait for
  *
@@ -257,7 +257,7 @@ do {									\
  * condition and doesn't want to contribute to system load. Signals are
  * ignored.
  */
-#define swait_event_idle(wq, condition)					\
+#define swait_event_idle_exclusive(wq, condition)			\
 do {									\
 	if (condition)							\
 		break;							\
@@ -270,7 +270,7 @@ do {									\
 		       __ret = schedule_timeout(__ret))
 
 /**
- * swait_event_idle_timeout - wait up to timeout without load contribution
+ * swait_event_idle_timeout_exclusive - wait up to timeout without load contribution
  * @wq: the waitqueue to wait on
  * @condition: a C expression for the event to wait for
  * @timeout: timeout at which we'll give up in jiffies
@@ -288,7 +288,7 @@ do {									\
  * or the remaining jiffies (at least 1) if the @condition evaluated
  * to %true before the @timeout elapsed.
  */
-#define swait_event_idle_timeout(wq, condition, timeout)		\
+#define swait_event_idle_timeout_exclusive(wq, condition, timeout)	\
 ({									\
 	long __ret = timeout;						\
 	if (!___wait_cond_timeout(condition))				\
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -92,7 +92,7 @@ static void s2idle_enter(void)
 	/* Push all the CPUs into the idle loop. */
 	wake_up_all_idle_cpus();
 	/* Make the current CPU wait so it can enter the idle loop too. */
-	swait_event(s2idle_wait_head,
+	swait_event_exclusive(s2idle_wait_head,
 		    s2idle_state == S2IDLE_STATE_WAKE);
 
 	cpuidle_pause();
@@ -160,7 +160,7 @@ void s2idle_wake(void)
 	raw_spin_lock_irqsave(&s2idle_lock, flags);
 	if (s2idle_state > S2IDLE_STATE_NONE) {
 		s2idle_state = S2IDLE_STATE_WAKE;
-		swake_up(&s2idle_wait_head);
+		swake_up_one(&s2idle_wait_head);
 	}
 	raw_spin_unlock_irqrestore(&s2idle_lock, flags);
 }
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -110,7 +110,7 @@ void __srcu_read_unlock(struct srcu_stru
 
 	WRITE_ONCE(sp->srcu_lock_nesting[idx], newval);
 	if (!newval && READ_ONCE(sp->srcu_gp_waiting))
-		swake_up(&sp->srcu_wq);
+		swake_up_one(&sp->srcu_wq);
 }
 EXPORT_SYMBOL_GPL(__srcu_read_unlock);
 
@@ -140,7 +140,7 @@ void srcu_drive_gp(struct work_struct *w
 	idx = sp->srcu_idx;
 	WRITE_ONCE(sp->srcu_idx, !sp->srcu_idx);
 	WRITE_ONCE(sp->srcu_gp_waiting, true);  /* srcu_read_unlock() wakes! */
-	swait_event(sp->srcu_wq, !READ_ONCE(sp->srcu_lock_nesting[idx]));
+	swait_event_exclusive(sp->srcu_wq, !READ_ONCE(sp->srcu_lock_nesting[idx]));
 	WRITE_ONCE(sp->srcu_gp_waiting, false); /* srcu_read_unlock() cheap. */
 
 	/* Invoke the callbacks we removed above. */
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1727,7 +1727,7 @@ static void rcu_gp_kthread_wake(struct r
 	    !READ_ONCE(rsp->gp_flags) ||
 	    !rsp->gp_kthread)
 		return;
-	swake_up(&rsp->gp_wq);
+	swake_up_one(&rsp->gp_wq);
 }
 
 /*
@@ -2002,7 +2002,7 @@ static bool rcu_gp_init(struct rcu_state
 }
 
 /*
- * Helper function for swait_event_idle() wakeup at force-quiescent-state
+ * Helper function for swait_event_idle_exclusive() wakeup at force-quiescent-state
  * time.
  */
 static bool rcu_gp_fqs_check_wake(struct rcu_state *rsp, int *gfp)
@@ -2144,7 +2144,7 @@ static int __noreturn rcu_gp_kthread(voi
 					       READ_ONCE(rsp->gpnum),
 					       TPS("reqwait"));
 			rsp->gp_state = RCU_GP_WAIT_GPS;
-			swait_event_idle(rsp->gp_wq, READ_ONCE(rsp->gp_flags) &
+			swait_event_idle_exclusive(rsp->gp_wq, READ_ONCE(rsp->gp_flags) &
 						     RCU_GP_FLAG_INIT);
 			rsp->gp_state = RCU_GP_DONE_GPS;
 			/* Locking provides needed memory barrier. */
@@ -2176,7 +2176,7 @@ static int __noreturn rcu_gp_kthread(voi
 					       READ_ONCE(rsp->gpnum),
 					       TPS("fqswait"));
 			rsp->gp_state = RCU_GP_WAIT_FQS;
-			ret = swait_event_idle_timeout(rsp->gp_wq,
+			ret = swait_event_idle_timeout_exclusive(rsp->gp_wq,
 					rcu_gp_fqs_check_wake(rsp, &gf), j);
 			rsp->gp_state = RCU_GP_DOING_FQS;
 			/* Locking provides needed memory barriers. */
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -212,7 +212,7 @@ static void __rcu_report_exp_rnp(struct
 			raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 			if (wake) {
 				smp_mb(); /* EGP done before wake_up(). */
-				swake_up(&rsp->expedited_wq);
+				swake_up_one(&rsp->expedited_wq);
 			}
 			break;
 		}
@@ -518,7 +518,7 @@ static void synchronize_sched_expedited_
 	jiffies_start = jiffies;
 
 	for (;;) {
-		ret = swait_event_timeout(
+		ret = swait_event_timeout_exclusive(
 				rsp->expedited_wq,
 				sync_rcu_preempt_exp_done_unlocked(rnp_root),
 				jiffies_stall);
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1854,8 +1854,8 @@ static void __wake_nocb_leader(struct rc
 		WRITE_ONCE(rdp_leader->nocb_leader_sleep, false);
 		del_timer(&rdp->nocb_timer);
 		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
-		smp_mb(); /* ->nocb_leader_sleep before swake_up(). */
-		swake_up(&rdp_leader->nocb_wq);
+		smp_mb(); /* ->nocb_leader_sleep before swake_up_one(). */
+		swake_up_one(&rdp_leader->nocb_wq);
 	} else {
 		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 	}
@@ -2082,7 +2082,7 @@ static void rcu_nocb_wait_gp(struct rcu_
 	 */
 	trace_rcu_this_gp(rnp, rdp, c, TPS("StartWait"));
 	for (;;) {
-		swait_event_interruptible(
+		swait_event_interruptible_exclusive(
 			rnp->nocb_gp_wq[c & 0x1],
 			(d = ULONG_CMP_GE(READ_ONCE(rnp->completed), c)));
 		if (likely(d))
@@ -2111,7 +2111,7 @@ static void nocb_leader_wait(struct rcu_
 	/* Wait for callbacks to appear. */
 	if (!rcu_nocb_poll) {
 		trace_rcu_nocb_wake(my_rdp->rsp->name, my_rdp->cpu, TPS("Sleep"));
-		swait_event_interruptible(my_rdp->nocb_wq,
+		swait_event_interruptible_exclusive(my_rdp->nocb_wq,
 				!READ_ONCE(my_rdp->nocb_leader_sleep));
 		raw_spin_lock_irqsave(&my_rdp->nocb_lock, flags);
 		my_rdp->nocb_leader_sleep = true;
@@ -2176,7 +2176,7 @@ static void nocb_leader_wait(struct rcu_
 		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 		if (rdp != my_rdp && tail == &rdp->nocb_follower_head) {
 			/* List was empty, so wake up the follower.  */
-			swake_up(&rdp->nocb_wq);
+			swake_up_one(&rdp->nocb_wq);
 		}
 	}
 
@@ -2193,7 +2193,7 @@ static void nocb_follower_wait(struct rc
 {
 	for (;;) {
 		trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu, TPS("FollowerSleep"));
-		swait_event_interruptible(rdp->nocb_wq,
+		swait_event_interruptible_exclusive(rdp->nocb_wq,
 					 READ_ONCE(rdp->nocb_follower_head));
 		if (smp_load_acquire(&rdp->nocb_follower_head)) {
 			/* ^^^ Ensure CB invocation follows _head test. */
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -32,7 +32,7 @@ void swake_up_locked(struct swait_queue_
 }
 EXPORT_SYMBOL(swake_up_locked);
 
-void swake_up(struct swait_queue_head *q)
+void swake_up_one(struct swait_queue_head *q)
 {
 	unsigned long flags;
 
@@ -40,7 +40,7 @@ void swake_up(struct swait_queue_head *q
 	swake_up_locked(q);
 	raw_spin_unlock_irqrestore(&q->lock, flags);
 }
-EXPORT_SYMBOL(swake_up);
+EXPORT_SYMBOL(swake_up_one);
 
 /*
  * Does not allow usage from IRQ disabled, since we must be able to
@@ -76,7 +76,7 @@ static void __prepare_to_swait(struct sw
 		list_add_tail(&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_exclusive(struct swait_queue_head *q, struct swait_queue *wait, int state)
 {
 	unsigned long flags;
 
@@ -85,7 +85,7 @@ void prepare_to_swait(struct swait_queue
 	set_current_state(state);
 	raw_spin_unlock_irqrestore(&q->lock, flags);
 }
-EXPORT_SYMBOL(prepare_to_swait);
+EXPORT_SYMBOL(prepare_to_swait_exclusive);
 
 long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
 {
@@ -95,7 +95,7 @@ long prepare_to_swait_event(struct swait
 	raw_spin_lock_irqsave(&q->lock, flags);
 	if (unlikely(signal_pending_state(state, current))) {
 		/*
-		 * See prepare_to_wait_event(). TL;DR, subsequent swake_up()
+		 * See prepare_to_wait_event(). TL;DR, subsequent swake_up_one()
 		 * must not see us.
 		 */
 		list_del_init(&wait->task_list);
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -586,7 +586,7 @@ void kvm_arm_resume_guest(struct kvm *kv
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		vcpu->arch.pause = false;
-		swake_up(kvm_arch_vcpu_wq(vcpu));
+		swake_up_one(kvm_arch_vcpu_wq(vcpu));
 	}
 }
 
@@ -594,7 +594,7 @@ static void vcpu_req_sleep(struct kvm_vc
 {
 	struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
 
-	swait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
+	swait_event_interruptible_exclusive(*wq, ((!vcpu->arch.power_off) &&
 				       (!vcpu->arch.pause)));
 
 	if (vcpu->arch.power_off || vcpu->arch.pause) {
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -155,7 +155,7 @@ static unsigned long kvm_psci_vcpu_on(st
 	smp_mb();		/* Make sure the above is visible */
 
 	wq = kvm_arch_vcpu_wq(vcpu);
-	swake_up(wq);
+	swake_up_one(wq);
 
 	return PSCI_RET_SUCCESS;
 }
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -107,7 +107,7 @@ static void async_pf_execute(struct work
 	trace_kvm_async_pf_completed(addr, gva);
 
 	if (swq_has_sleeper(&vcpu->wq))
-		swake_up(&vcpu->wq);
+		swake_up_one(&vcpu->wq);
 
 	mmput(mm);
 	kvm_put_kvm(vcpu->kvm);
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2155,7 +2155,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcp
 	kvm_arch_vcpu_blocking(vcpu);
 
 	for (;;) {
-		prepare_to_swait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+		prepare_to_swait_exclusive(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
 		if (kvm_vcpu_check_block(vcpu) < 0)
 			break;
@@ -2197,7 +2197,7 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *v
 
 	wqp = kvm_arch_vcpu_wq(vcpu);
 	if (swq_has_sleeper(wqp)) {
-		swake_up(wqp);
+		swake_up_one(wqp);
 		++vcpu->stat.halt_wakeup;
 		return true;
 	}



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-12  8:34 [PATCH 0/3] sched/swait: Convert to full exclusive mode Peter Zijlstra
                   ` (2 preceding siblings ...)
  2018-06-12  8:34 ` [PATCH 3/3] sched/swait: Rename to exclusive Peter Zijlstra
@ 2018-06-12 16:47 ` Linus Torvalds
  2018-06-12 17:14   ` Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode) Peter Zijlstra
  2018-06-12 18:52   ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Andreas Grünbacher
  2018-06-14  1:27 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Paul E. McKenney
  4 siblings, 2 replies; 22+ messages in thread
From: Linus Torvalds @ 2018-06-12 16:47 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Linux Kernel Mailing List, Thomas Gleixner,
	Sebastian Andrzej Siewior, Oleg Nesterov, Paul McKenney,
	Paolo Bonzini

"
On Tue, Jun 12, 2018 at 1:39 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> As mentioned by Linus, swait is exclusive mode and had better behave like it
> and be named like it.

Ack on the patches.

I do note how quilt emails are really hard to read, because that:

    Content-Disposition: inline

makes gmail think it's flowed.

Which works horribly badly for patches, surprise surprise.

So I really wish quilt wouldn't do that. It does smell like a gmail
bug, but at the same time, why would you use "Content-Disposition:
inline" when you don't have an actual multi-part email? So I do blame
quilt too for sending nonsensical headers.

(Yes, yes, I see the "It is permissible to use Content-Disposition on
the main body" in the RFC. But the RFC also makes it clear that it
actually matters for how things are presented, so saying "ok, I'll do
flowed" seems equally insane and equally technically RFC-compliant)

              Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode)
  2018-06-12 16:47 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Linus Torvalds
@ 2018-06-12 17:14   ` Peter Zijlstra
  2018-06-13 12:32     ` Jean Delvare
  2018-06-12 18:52   ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Andreas Grünbacher
  1 sibling, 1 reply; 22+ messages in thread
From: Peter Zijlstra @ 2018-06-12 17:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ingo Molnar, Linux Kernel Mailing List, Thomas Gleixner,
	Sebastian Andrzej Siewior, Oleg Nesterov, Paul McKenney,
	Paolo Bonzini, Jean Delvare, Andreas Gruenbacher

On Tue, Jun 12, 2018 at 09:47:34AM -0700, Linus Torvalds wrote:

> I do note how quilt emails are really hard to read, because that:
> 
>     Content-Disposition: inline
> 
> makes gmail think it's flowed.
> 
> Which works horribly badly for patches, surprise surprise.
> 
> So I really wish quilt wouldn't do that. It does smell like a gmail
> bug, but at the same time, why would you use "Content-Disposition:
> inline" when you don't have an actual multi-part email? So I do blame
> quilt too for sending nonsensical headers.
> 
> (Yes, yes, I see the "It is permissible to use Content-Disposition on
> the main body" in the RFC. But the RFC also makes it clear that it
> actually matters for how things are presented, so saying "ok, I'll do
> flowed" seems equally insane and equally technically RFC-compliant)

Quilt people, anything that can be done about that?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-12 16:47 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Linus Torvalds
  2018-06-12 17:14   ` Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode) Peter Zijlstra
@ 2018-06-12 18:52   ` Andreas Grünbacher
       [not found]     ` <CA+55aFx81igOjFZcvO03mvDFd3=pxsq2QuNrWrPW+4pvJy780A@mail.gmail.com>
  1 sibling, 1 reply; 22+ messages in thread
From: Andreas Grünbacher @ 2018-06-12 18:52 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Zijlstra, Ingo Molnar, Linux Kernel Mailing List,
	Thomas Gleixner, Sebastian Andrzej Siewior, Oleg Nesterov,
	Paul McKenney, Paolo Bonzini, quilt-dev

2018-06-12 18:47 GMT+02:00 Linus Torvalds <torvalds@linux-foundation.org>:
> "
> On Tue, Jun 12, 2018 at 1:39 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> As mentioned by Linus, swait is exclusive mode and had better behave like it
>> and be named like it.
>
> Ack on the patches.
>
> I do note how quilt emails are really hard to read, because that:
>
>     Content-Disposition: inline
>
> makes gmail think it's flowed.
>
> Which works horribly badly for patches, surprise surprise.
>
> So I really wish quilt wouldn't do that. It does smell like a gmail
> bug, but at the same time, why would you use "Content-Disposition:
> inline" when you don't have an actual multi-part email? So I do blame
> quilt too for sending nonsensical headers.
>
> (Yes, yes, I see the "It is permissible to use Content-Disposition on
> the main body" in the RFC. But the RFC also makes it clear that it
> actually matters for how things are presented, so saying "ok, I'll do
> flowed" seems equally insane and equally technically RFC-compliant)

Quilt uses those Content-Disposition headers to preserve the patch
filenames; the full headers look like:

  Content-Disposition: inline; filename=foo.patch

Various mail clients use that as the default filename when saving
emails, but there's been at least one other mail client that didn't
handle those headers very well.

Is this a new problem with gmail?

I'm not sure whose workflows would break if we kill those headers
altogether, but maybe we can omit them by default.

Andreas

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
       [not found]     ` <CA+55aFx81igOjFZcvO03mvDFd3=pxsq2QuNrWrPW+4pvJy780A@mail.gmail.com>
@ 2018-06-12 19:43       ` Thomas Gleixner
  2018-06-12 21:54         ` Sebastian Andrzej Siewior
  2018-06-13 13:00       ` [Quilt-dev] Quilt vs gmail Jean Delvare
  1 sibling, 1 reply; 22+ messages in thread
From: Thomas Gleixner @ 2018-06-12 19:43 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andreas Grünbacher, Peter Zijlstra, Ingo Molnar,
	Linux Kernel Mailing List, Sebastian Andrzej Siewior,
	Oleg Nesterov, Paul McKenney, Paolo Bonzini, quilt-dev

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

On Tue, 12 Jun 2018, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 11:52 AM Andreas Grünbacher
> <andreas.gruenbacher@gmail.com> wrote:
> >
> > Quilt uses those Content-Disposition headers to preserve the patch
> > filenames;
> 
> That' what I was assuming, but does anybody really care?

People might have scripts using it but I doubt anyone still relies on that
because git mail never did that inline/filename dance so scripts which
convert mboxes to quilt need to have a mechanism to create filenames
anyway. My personal script uses the subject line to create the filenames
and I never tried to use the inline filename as those filenames are often
enough complete garbage.

> (Even if they use quilt to manage patches, maybe they don't _send_
> them that way?)

Many quilt users still do.

> How long has quilt been doing it?

Forever.

> Also, note that I'm not at all sure that it's _just_ the
> 
>     Content-Disposition: inline
> 
> that triggers this. There might be something else in those emails that
> triggers it but that's the thing that stands out.

It is. There is nothing else in the headers which could cause that. quilt
mail format is pretty simplistic.

> > I'm not sure whose workflows would break if we kill those headers
> > altogether, but maybe we can omit them by default.
> 
> That would be lovely at least for my case.

No objections.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-12 19:43       ` Thomas Gleixner
@ 2018-06-12 21:54         ` Sebastian Andrzej Siewior
  2018-06-12 22:03           ` Linus Torvalds
  0 siblings, 1 reply; 22+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-06-12 21:54 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Linus Torvalds, Andreas Grünbacher, Peter Zijlstra,
	Ingo Molnar, Linux Kernel Mailing List, Oleg Nesterov,
	Paul McKenney, Paolo Bonzini, quilt-dev

On 2018-06-12 21:43:48 [+0200], Thomas Gleixner wrote:
> > Also, note that I'm not at all sure that it's _just_ the
> > 
> >     Content-Disposition: inline
> > 
> > that triggers this. There might be something else in those emails that
> > triggers it but that's the thing that stands out.
> 
> It is. There is nothing else in the headers which could cause that. quilt
> mail format is pretty simplistic.

mutt sets this and the tip-bot, too. The difference is that quilt also
sets the filename parameter. Do the mutt or tip-bot mails look flowed?

> Thanks,
> 
> 	tglx

Sebastian

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-12 21:54         ` Sebastian Andrzej Siewior
@ 2018-06-12 22:03           ` Linus Torvalds
  2018-06-12 22:55             ` Randy Dunlap
  0 siblings, 1 reply; 22+ messages in thread
From: Linus Torvalds @ 2018-06-12 22:03 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Thomas Gleixner, Andreas Grünbacher, Peter Zijlstra,
	Ingo Molnar, Linux Kernel Mailing List, Oleg Nesterov,
	Paul McKenney, Paolo Bonzini, quilt-dev

On Tue, Jun 12, 2018 at 2:54 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> mutt sets this and the tip-bot, too. The difference is that quilt also
> sets the filename parameter. Do the mutt or tip-bot mails look flowed?

No, they look fine.

And yes, the difference looks to be that filename. Looking at the
tipbot emails, I see:

  Content-Transfer-Encoding: 8bit
  Content-Type: text/plain; charset=UTF-8
  Content-Disposition: inline

and the email looks perfectly normal.

So it's not Content-Disposition itself that triggers it, it is indeed
that *together* with filename information, so

  Content-Type: text/plain; charset=UTF-8
  Content-Disposition: inline; filename=peterz-swait-1.patch

makes gmail go "Oh, I want to help you download this thing", and makes
that "download attachment" widget, and makes the text itself act
flowed.

Yeah, that's just stupid of gmail.

I've sent feedback, but judging by my last attempt to get something
fixed (making the mobile application have a "Plain text" mode), I
expect to be laughed at by some coked-up QA person who thinks "flowed"
is obviously better.

                   Linus

                  Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-12 22:03           ` Linus Torvalds
@ 2018-06-12 22:55             ` Randy Dunlap
  0 siblings, 0 replies; 22+ messages in thread
From: Randy Dunlap @ 2018-06-12 22:55 UTC (permalink / raw)
  To: Linus Torvalds, Sebastian Andrzej Siewior
  Cc: Thomas Gleixner, Andreas Grünbacher, Peter Zijlstra,
	Ingo Molnar, Linux Kernel Mailing List, Oleg Nesterov,
	Paul McKenney, Paolo Bonzini, quilt-dev

On 06/12/2018 03:03 PM, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 2:54 PM Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
>>
>> mutt sets this and the tip-bot, too. The difference is that quilt also
>> sets the filename parameter. Do the mutt or tip-bot mails look flowed?
> 
> No, they look fine.
> 
> And yes, the difference looks to be that filename. Looking at the
> tipbot emails, I see:
> 
>   Content-Transfer-Encoding: 8bit
>   Content-Type: text/plain; charset=UTF-8
>   Content-Disposition: inline
> 
> and the email looks perfectly normal.
> 
> So it's not Content-Disposition itself that triggers it, it is indeed
> that *together* with filename information, so
> 
>   Content-Type: text/plain; charset=UTF-8
>   Content-Disposition: inline; filename=peterz-swait-1.patch
> 
> makes gmail go "Oh, I want to help you download this thing", and makes
> that "download attachment" widget, and makes the text itself act
> flowed.
> 
> Yeah, that's just stupid of gmail.
> 
> I've sent feedback, but judging by my last attempt to get something
> fixed (making the mobile application have a "Plain text" mode), I
> expect to be laughed at by some coked-up QA person who thinks "flowed"
> is obviously better.


FWIW, Peter's patches also show up in Thunderbird as attachments (at least
for me they do).

-- 
~Randy

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode)
  2018-06-12 17:14   ` Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode) Peter Zijlstra
@ 2018-06-13 12:32     ` Jean Delvare
  2018-06-13 13:27       ` Andreas Grünbacher
  0 siblings, 1 reply; 22+ messages in thread
From: Jean Delvare @ 2018-06-13 12:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Ingo Molnar, Linux Kernel Mailing List,
	Thomas Gleixner, Sebastian Andrzej Siewior, Oleg Nesterov,
	Paul McKenney, Paolo Bonzini, Andreas Gruenbacher

Hi Peter, Linus, Andreas,

On Tue, 12 Jun 2018 19:14:20 +0200, Peter Zijlstra wrote:
> On Tue, Jun 12, 2018 at 09:47:34AM -0700, Linus Torvalds wrote:
> 
> > I do note how quilt emails are really hard to read, because that:
> > 
> >     Content-Disposition: inline
> > 
> > makes gmail think it's flowed.
> > 
> > Which works horribly badly for patches, surprise surprise.
> > 
> > So I really wish quilt wouldn't do that. It does smell like a gmail
> > bug, but at the same time, why would you use "Content-Disposition:
> > inline" when you don't have an actual multi-part email? So I do blame
> > quilt too for sending nonsensical headers.
> > 
> > (Yes, yes, I see the "It is permissible to use Content-Disposition on
> > the main body" in the RFC. But the RFC also makes it clear that it
> > actually matters for how things are presented, so saying "ok, I'll do
> > flowed" seems equally insane and equally technically RFC-compliant)  
> 
> Quilt people, anything that can be done about that?

The purpose of the Content-Disposition header is to let quilt store the
original patch file name, so that the recipient can save the email as a
patch file having the exact same name as the sender was using, to make
communication between developers easier. This is the reason why the
header is being added despite the email not being multi-part. As Linus
found out already, RFC 2183 allows that. The RFC also explicitly allows
the use of a filename parameter for inline parts (see section 2.3.)

Using "attachment" instead of "inline" would presumably force the user
to save the patch to a file before being able to read it, or, at least,
to take additional actions in the MUA to convince it to display the
contents inline regardless of what the Content-Disposition header
says. This is clearly not desirable.

We could try specifying the filename directly, without the "inline"
keyword, however that would no longer comply with the RFC
("disposition-parm" is optional, but "disposition-type" is mandatory)
and I am afraid that some MUA implementations would either default to
disposition-type "attachment" in this case, or ignore the header
altogether.

I'm not sure I understand what "flowed" means in this context. If you
mean that gmail breaks the formatting of the patch, I would say that
gmail is infringing the RFC, which clearly stipulates at the beginning
that the Content-Disposition header field is only about telling the MUA
which parts should be displayed immediately and which parts should not,
and not about presentation issues.

Considering that "inline" is the default for a non-multi-part message,
any MUA which changes its behavior in the presence of
"Content-Disposition: inline" is bugged in my opinion.
-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Quilt-dev] Quilt vs gmail
       [not found]     ` <CA+55aFx81igOjFZcvO03mvDFd3=pxsq2QuNrWrPW+4pvJy780A@mail.gmail.com>
  2018-06-12 19:43       ` Thomas Gleixner
@ 2018-06-13 13:00       ` Jean Delvare
  2018-06-13 13:35         ` Greg KH
  1 sibling, 1 reply; 22+ messages in thread
From: Jean Delvare @ 2018-06-13 13:00 UTC (permalink / raw)
  To: Linus Torvalds, Greg KH
  Cc: Andreas Grünbacher, Peter Zijlstra,
	Sebastian Andrzej Siewior, Oleg Nesterov,
	Linux Kernel Mailing List, quilt-dev, Paolo Bonzini,
	Thomas Gleixner, Paul McKenney, Ingo Molnar

On Tue, 12 Jun 2018 12:23:26 -0700, Linus Torvalds wrote:
> On Tue, Jun 12, 2018 at 11:52 AM Andreas Grünbacher
> <andreas.gruenbacher@gmail.com> wrote:
> >
> > Quilt uses those Content-Disposition headers to preserve the patch
> > filenames;  
> 
> That' what I was assuming, but does anybody really care?

Long ago (probably a decade by now, literally) I wrote a shell script
named "rename-patch" for Greg KH which suggests a file name for a patch
received by e-mail. The script first looks for a "filename" attribute
in the Content-Disposition header, and only if not found, falls back to
a heuristic which attempts to generate a good-looking file name based
on the e-mail's subject.

The script used to be published on my kernel.org personal web space,
but went away when kernel.org got hacked, and I never bothered
publishing my few scripts again, sorry about that.

I'm still using that script myself, to name patches generated with "git
show --pretty=email", however there is no Content-Disposition header
there, so the subject heuristic is always used. I don't know if Greg is
still using rename-patch in combination with quilt. Greg?

> If you do things one patch at a time, maybe it's convenient, but then
> it doesn't sound like a huge win either.
> 
> And if you do a patch-series, then it won't work anyway, and you'd be
> saving to an mbox or something. Unless people save patch-series things
> one by one, but at that point "convenient" is no longer an issue.

I'm not sure why it wouldn't work with a series. The name information
is available in each patch of the series, and I know that some kernel
developers have all sorts of shortcuts and macros implemented on top of
their MUA to automate queuing of patches for various testing or
publishing purposes.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode)
  2018-06-13 12:32     ` Jean Delvare
@ 2018-06-13 13:27       ` Andreas Grünbacher
  2018-06-13 13:48         ` Linus Torvalds
  2018-06-13 14:40         ` Jean Delvare
  0 siblings, 2 replies; 22+ messages in thread
From: Andreas Grünbacher @ 2018-06-13 13:27 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Peter Zijlstra, Linus Torvalds, Ingo Molnar,
	Linux Kernel Mailing List, Thomas Gleixner,
	Sebastian Andrzej Siewior, Oleg Nesterov, Paul McKenney,
	Paolo Bonzini, Andreas Gruenbacher

Jean,

2018-06-13 14:32 GMT+02:00 Jean Delvare <jdelvare@suse.de>:
> Hi Peter, Linus, Andreas,
>
> On Tue, 12 Jun 2018 19:14:20 +0200, Peter Zijlstra wrote:
>> On Tue, Jun 12, 2018 at 09:47:34AM -0700, Linus Torvalds wrote:
>>
>> > I do note how quilt emails are really hard to read, because that:
>> >
>> >     Content-Disposition: inline
>> >
>> > makes gmail think it's flowed.
>> >
>> > Which works horribly badly for patches, surprise surprise.
>> >
>> > So I really wish quilt wouldn't do that. It does smell like a gmail
>> > bug, but at the same time, why would you use "Content-Disposition:
>> > inline" when you don't have an actual multi-part email? So I do blame
>> > quilt too for sending nonsensical headers.
>> >
>> > (Yes, yes, I see the "It is permissible to use Content-Disposition on
>> > the main body" in the RFC. But the RFC also makes it clear that it
>> > actually matters for how things are presented, so saying "ok, I'll do
>> > flowed" seems equally insane and equally technically RFC-compliant)
>>
>> Quilt people, anything that can be done about that?
>
> The purpose of the Content-Disposition header is to let quilt store the
> original patch file name, so that the recipient can save the email as a
> patch file having the exact same name as the sender was using, to make
> communication between developers easier. This is the reason why the
> header is being added despite the email not being multi-part. As Linus
> found out already, RFC 2183 allows that. The RFC also explicitly allows
> the use of a filename parameter for inline parts (see section 2.3.)
>
> Using "attachment" instead of "inline" would presumably force the user
> to save the patch to a file before being able to read it, or, at least,
> to take additional actions in the MUA to convince it to display the
> contents inline regardless of what the Content-Disposition header
> says. This is clearly not desirable.
>
> We could try specifying the filename directly, without the "inline"
> keyword, however that would no longer comply with the RFC
> ("disposition-parm" is optional, but "disposition-type" is mandatory)
> and I am afraid that some MUA implementations would either default to
> disposition-type "attachment" in this case, or ignore the header
> altogether.
>
> I'm not sure I understand what "flowed" means in this context. If you
> mean that gmail breaks the formatting of the patch, I would say that
> gmail is infringing the RFC, which clearly stipulates at the beginning
> that the Content-Disposition header field is only about telling the MUA
> which parts should be displayed immediately and which parts should not,
> and not about presentation issues.
>
> Considering that "inline" is the default for a non-multi-part message,
> any MUA which changes its behavior in the presence of
> "Content-Disposition: inline" is bugged in my opinion.

All of that may be correct, but those headers apparently do break
email based patch reviewing on Thunderbird and Gmail now, and that's
not very likely to change. If we continue with our current practice,
we'll end up frustrating users. On top of that, i we make this an
optional feature, quilt users may think that using that option is a
good idea when they will actually break their recipients' workflows.
As Thomas Gleixner wrote in the other thread, most recipients will
already have a way to deal with messages from other sources that don't
include patch filenames, so let's just get rid of Content-Disposition
headers in quilt for good.

Andreas

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Quilt-dev] Quilt vs gmail
  2018-06-13 13:00       ` [Quilt-dev] Quilt vs gmail Jean Delvare
@ 2018-06-13 13:35         ` Greg KH
  0 siblings, 0 replies; 22+ messages in thread
From: Greg KH @ 2018-06-13 13:35 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Linus Torvalds, Andreas Grünbacher, Peter Zijlstra,
	Sebastian Andrzej Siewior, Oleg Nesterov,
	Linux Kernel Mailing List, quilt-dev, Paolo Bonzini,
	Thomas Gleixner, Paul McKenney, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]

On Wed, Jun 13, 2018 at 03:00:25PM +0200, Jean Delvare wrote:
> On Tue, 12 Jun 2018 12:23:26 -0700, Linus Torvalds wrote:
> > On Tue, Jun 12, 2018 at 11:52 AM Andreas Grünbacher
> > <andreas.gruenbacher@gmail.com> wrote:
> > >
> > > Quilt uses those Content-Disposition headers to preserve the patch
> > > filenames;  
> > 
> > That' what I was assuming, but does anybody really care?
> 
> Long ago (probably a decade by now, literally) I wrote a shell script
> named "rename-patch" for Greg KH which suggests a file name for a patch
> received by e-mail. The script first looks for a "filename" attribute
> in the Content-Disposition header, and only if not found, falls back to
> a heuristic which attempts to generate a good-looking file name based
> on the e-mail's subject.
> 
> The script used to be published on my kernel.org personal web space,
> but went away when kernel.org got hacked, and I never bothered
> publishing my few scripts again, sorry about that.
> 
> I'm still using that script myself, to name patches generated with "git
> show --pretty=email", however there is no Content-Disposition header
> there, so the subject heuristic is always used. I don't know if Greg is
> still using rename-patch in combination with quilt. Greg?

Yes I am, I also use it for other things, it's quite useful to me.  It's
attached below if anyone else wants it.

But I don't really use the Content-Disposition portion of the logic in
that script much, if any, anymore as I handle most of my normal kernel
work using git.  I only use quilt these days for local work before using
git, and for all of my stable kernel work.

So if that logic goes away in quilt, I'm not going to miss it :)

thanks,

greg k-h

[-- Attachment #2: rename-patch --]
[-- Type: text/plain, Size: 2076 bytes --]

#!/bin/bash

# Rename patch files according to the Content-Disposition header line
# if they do have one, or Subject if they don't.
#
# Copyright (C) 2005, 2006, 2008  Jean Delvare <khali@linux-fr.org>
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
# 
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details:
# http://www.gnu.org/copyleft/gpl.html
#
# The latest official version of this script can be found at:
# ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/scripts/rename-patch

# Avoid nasty locale effects
export LC_ALL=C

findname()
{
	local file="$1" pattern;

	# Try Content-Disposition first
	pattern='^Content-Disposition:[[:space:]]*inline[[:space:]]*;[[:space:]]*filename[[:space:]]*=';
	if grep -q $pattern "$file"
	then
		sed -ne "/$pattern/{s/$pattern[[:space:]]*//p;q}" "$file"
		return
	fi

	# Fallback to Subject
	pattern='^Subject:';
	if grep -q $pattern "$file"
	then
		sed -ne "/$pattern/{
			s/$pattern[[:space:]]*//;
			s/\[[^]]*\]//g;
			s/[^a-z0-9._-]/-/ig;
			s/--*/-/g;
			s/^-//;
			s/^re-//i;
			s/[.-]*$/.patch/;
			p;q}" "$file" | \
		tr A-Z a-z
		return
	fi
}

if [ $# -eq 0 ]
then
	echo "Usage: rename-patch file [file...]" >&2
	exit 1
fi

for file in "$@"
do
	name=$(findname "$file")
	if echo "$file" | grep -q '/'
	then
		path=$(echo "$file" | sed -e 's/\/[^/]*$/\//')
	else
		path=''
	fi

	if [ -z "$name" ]
	then
		echo "No name found for $file" >&2
		continue
	fi

	if [ "$path$name" == "$file" ]
	then
		# It's already OK
		continue
	fi

	if [ -e "$path$name" ]
	then
		echo "Can't rename $file to $path$name which already exists" >&2
		continue
	fi

	mv -f "$file" "$path$name"
#	echo "$file renamed to $path$name"
	echo "$path$name"
done

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode)
  2018-06-13 13:27       ` Andreas Grünbacher
@ 2018-06-13 13:48         ` Linus Torvalds
  2018-06-13 14:40         ` Jean Delvare
  1 sibling, 0 replies; 22+ messages in thread
From: Linus Torvalds @ 2018-06-13 13:48 UTC (permalink / raw)
  To: Andreas Grünbacher
  Cc: Jean Delvare, Peter Zijlstra, Ingo Molnar,
	Linux Kernel Mailing List, Thomas Gleixner,
	Sebastian Andrzej Siewior, Oleg Nesterov, Paul McKenney,
	Paolo Bonzini, Andreas Gruenbacher

On Wed, Jun 13, 2018 at 6:27 AM Andreas Grünbacher
<andreas.gruenbacher@gmail.com> wrote:
>
> All of that may be correct, but those headers apparently do break
> email based patch reviewing on Thunderbird and Gmail now, and that's
> not very likely to change.

I do want to point out that at least for me, it's not a very big
annoyance. It would be sad if people who actually_use_ quilt lose a
feature they use for this.

I've seen it twice in the last few months, so it's not  like it's all
that noticeable. It's a bit annoying when it happens, but it's not a
show-stopper.

Either I don't interact with a lot of quilt users, or (quite likely)
people don't always use quilt itself to then send the patches (the
same way I don't use git send-email to send patches even though I
obviously use git).

                  Linus

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode)
  2018-06-13 13:27       ` Andreas Grünbacher
  2018-06-13 13:48         ` Linus Torvalds
@ 2018-06-13 14:40         ` Jean Delvare
  1 sibling, 0 replies; 22+ messages in thread
From: Jean Delvare @ 2018-06-13 14:40 UTC (permalink / raw)
  To: Andreas Grünbacher
  Cc: Peter Zijlstra, Linus Torvalds, Ingo Molnar,
	Linux Kernel Mailing List, Thomas Gleixner,
	Sebastian Andrzej Siewior, Oleg Nesterov, Paul McKenney,
	Paolo Bonzini, Andreas Gruenbacher

On Wed, 13 Jun 2018 15:27:31 +0200, Andreas Grünbacher wrote:
> All of that may be correct, but those headers apparently do break
> email based patch reviewing on Thunderbird and Gmail now, and that's
> not very likely to change. If we continue with our current practice,
> we'll end up frustrating users. On top of that, i we make this an
> optional feature, quilt users may think that using that option is a
> good idea when they will actually break their recipients' workflows.
> As Thomas Gleixner wrote in the other thread, most recipients will
> already have a way to deal with messages from other sources that don't
> include patch filenames, so let's just get rid of Content-Disposition
> headers in quilt for good.

It always feels bad to me to work around an issue in one piece of
software when the bug is clearly in another piece of software. I'm not
sure why fixes on the correct side of the problem should be unlikely.
The behavior is clearly broken and against the RFC, and there may be
other sources than quilt triggering the bug. Thunderbird is open
source, the problem is identified, it shouldn't be that hard to fix it.
Gmail is a different story, of course, but I guess there are some
developers maintaining it too.

That being said... I agree that recipients of such e-mails most likely
already have an alternative solution in place for non-quilt sources, so
probably removing the Content-Disposition header is not going to cause
too much trouble. So feel free to go ahead and remove it if you think
this is the best thing to do. If some users complain about the change,
I'll let you deal with them ;-)

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-12  8:34 [PATCH 0/3] sched/swait: Convert to full exclusive mode Peter Zijlstra
                   ` (3 preceding siblings ...)
  2018-06-12 16:47 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Linus Torvalds
@ 2018-06-14  1:27 ` Paul E. McKenney
  2018-06-19 14:49   ` Paul E. McKenney
  4 siblings, 1 reply; 22+ messages in thread
From: Paul E. McKenney @ 2018-06-14  1:27 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mingo, linux-kernel, torvalds, tglx, bigeasy, oleg, pbonzini

On Tue, Jun 12, 2018 at 10:34:49AM +0200, Peter Zijlstra wrote:
> As mentioned by Linus, swait is exclusive mode and had better behave like it
> and be named like it.
> 
> Make it so.

Cool!

When I tried testing it on x86 against recent mainline, I got:

"WARNING: WARNING: Bad or missing .orc_unwind table.  Disabling unwinder."

But when I tested on recent mainline without your patches:

	4597fcff0704 "Merge tag 'for-4.18/dm-changes-v2' of
	git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm"

I get the same warning.  Trying again on today's mainline, but in the
meantime, is anyone else seeing this?

							Thanx, Paul

> ---
>  arch/mips/kvm/mips.c         |  4 ++--
>  arch/powerpc/kvm/book3s_hv.c |  6 +++---
>  arch/s390/kvm/interrupt.c    |  2 +-
>  arch/x86/kernel/kvm.c        |  4 ++--
>  arch/x86/kvm/lapic.c         |  2 +-
>  include/linux/swait.h        | 36 ++++++++++++++++++------------------
>  kernel/power/suspend.c       |  4 ++--
>  kernel/rcu/srcutiny.c        |  4 ++--
>  kernel/rcu/tree.c            |  8 ++++----
>  kernel/rcu/tree_exp.h        |  4 ++--
>  kernel/rcu/tree_plugin.h     | 12 ++++++------
>  kernel/sched/swait.c         | 32 ++++++++++++++++++++++----------
>  virt/kvm/arm/arm.c           |  4 ++--
>  virt/kvm/arm/psci.c          |  2 +-
>  virt/kvm/async_pf.c          |  2 +-
>  virt/kvm/kvm_main.c          |  4 ++--
>  16 files changed, 71 insertions(+), 59 deletions(-)
> 
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/3] sched/swait: Convert to full exclusive mode
  2018-06-14  1:27 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Paul E. McKenney
@ 2018-06-19 14:49   ` Paul E. McKenney
  0 siblings, 0 replies; 22+ messages in thread
From: Paul E. McKenney @ 2018-06-19 14:49 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mingo, linux-kernel, torvalds, tglx, bigeasy, oleg, pbonzini

On Wed, Jun 13, 2018 at 06:27:42PM -0700, Paul E. McKenney wrote:
> On Tue, Jun 12, 2018 at 10:34:49AM +0200, Peter Zijlstra wrote:
> > As mentioned by Linus, swait is exclusive mode and had better behave like it
> > and be named like it.
> > 
> > Make it so.
> 
> Cool!
> 
> When I tried testing it on x86 against recent mainline, I got:
> 
> "WARNING: WARNING: Bad or missing .orc_unwind table.  Disabling unwinder."
> 
> But when I tested on recent mainline without your patches:
> 
> 	4597fcff0704 "Merge tag 'for-4.18/dm-changes-v2' of
> 	git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm"
> 
> I get the same warning.  Trying again on today's mainline, but in the
> meantime, is anyone else seeing this?

And this patch fixes this issue, FWIW:

https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1713797.html

							Thanx, Paul


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:sched/core] sched/swait: Remove __prepare_to_swait
  2018-06-12  8:34 ` [PATCH 1/3] sched/swait: Remove __prepare_to_swait Peter Zijlstra
@ 2018-06-20  9:39   ` tip-bot for Peter Zijlstra
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Peter Zijlstra @ 2018-06-20  9:39 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: mingo, linux-kernel, hpa, torvalds, peterz, tglx

Commit-ID:  6519750210d9d3330e85d12e0128652edde448d2
Gitweb:     https://git.kernel.org/tip/6519750210d9d3330e85d12e0128652edde448d2
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 12 Jun 2018 10:34:50 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 20 Jun 2018 11:35:56 +0200

sched/swait: Remove __prepare_to_swait

There is no public user of this API, remove it.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: bigeasy@linutronix.de
Cc: oleg@redhat.com
Cc: paulmck@linux.vnet.ibm.com
Cc: pbonzini@redhat.com
Link: https://lkml.kernel.org/r/20180612083909.157076812@infradead.org

---
 include/linux/swait.h | 1 -
 kernel/sched/swait.c  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/swait.h b/include/linux/swait.h
index bf8cb0dee23c..d6a5e949e4ca 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -161,7 +161,6 @@ 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);
 
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
index b6fb2c3b3ff7..e68bb1398b05 100644
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -69,7 +69,7 @@ void swake_up_all(struct swait_queue_head *q)
 }
 EXPORT_SYMBOL(swake_up_all);
 
-void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+static void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
 {
 	wait->task = current;
 	if (list_empty(&wait->task_list))

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [tip:sched/core] sched/swait: Switch to full exclusive mode
  2018-06-12  8:34 ` [PATCH 2/3] sched/swait: Switch to full exclusive mode Peter Zijlstra
@ 2018-06-20  9:40   ` tip-bot for Peter Zijlstra
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Peter Zijlstra @ 2018-06-20  9:40 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: hpa, peterz, mingo, tglx, torvalds, linux-kernel

Commit-ID:  0abf17bc7790dd0467ed0e38522242f23c5da1c4
Gitweb:     https://git.kernel.org/tip/0abf17bc7790dd0467ed0e38522242f23c5da1c4
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 12 Jun 2018 10:34:51 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 20 Jun 2018 11:35:56 +0200

sched/swait: Switch to full exclusive mode

Linus noted that swait basically implements exclusive mode -- because
swake_up() only wakes a single waiter. And because of that it should
take care to properly deal with the interruptible case.

In short, the problem is that swake_up() can race with a signal. In
this this case it is possible the swake_up() 'wakes' the waiter that
is already on the way out because it just got a signal and the wakeup
gets lost.

The normal wait code is very careful and avoids this situation, make
sure we do too.

Copy the exact exclusive semantics from wait.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: bigeasy@linutronix.de
Cc: oleg@redhat.com
Cc: paulmck@linux.vnet.ibm.com
Cc: pbonzini@redhat.com
Link: https://lkml.kernel.org/r/20180612083909.209762413@infradead.org

---
 include/linux/swait.h | 11 ++++++-----
 kernel/sched/swait.c  | 22 +++++++++++++++++-----
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/include/linux/swait.h b/include/linux/swait.h
index d6a5e949e4ca..dd032061112d 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -38,8 +38,8 @@
  *    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.
+ *  - the !exclusive mode; because that leads to O(n) wakeups, everything is
+ *    exclusive.
  *
  *  - custom wake callback functions; because you cannot give any guarantees
  *    about random code. This also allows swait to be used in RT, such that
@@ -167,9 +167,10 @@ extern long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queu
 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" */
+/* as per ___wait_event() but for swait, therefore "exclusive == 1" */
 #define ___swait_event(wq, condition, state, ret, cmd)			\
 ({									\
+	__label__ __out;						\
 	struct swait_queue __wait;					\
 	long __ret = ret;						\
 									\
@@ -182,13 +183,13 @@ extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
 									\
 		if (___wait_is_interruptible(state) && __int) {		\
 			__ret = __int;					\
-			break;						\
+			goto __out;					\
 		}							\
 									\
 		cmd;							\
 	}								\
 	finish_swait(&wq, &__wait);					\
-	__ret;								\
+__out:	__ret;								\
 })
 
 #define __swait_event(wq, condition)					\
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
index e68bb1398b05..66890de93ee5 100644
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -73,7 +73,7 @@ static void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *w
 {
 	wait->task = current;
 	if (list_empty(&wait->task_list))
-		list_add(&wait->task_list, &q->task_list);
+		list_add_tail(&wait->task_list, &q->task_list);
 }
 
 void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
@@ -89,12 +89,24 @@ 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;
+	unsigned long flags;
+	long ret = 0;
 
-	prepare_to_swait(q, wait, state);
+	raw_spin_lock_irqsave(&q->lock, flags);
+	if (unlikely(signal_pending_state(state, current))) {
+		/*
+		 * See prepare_to_wait_event(). TL;DR, subsequent swake_up()
+		 * must not see us.
+		 */
+		list_del_init(&wait->task_list);
+		ret = -ERESTARTSYS;
+	} else {
+		__prepare_to_swait(q, wait);
+		set_current_state(state);
+	}
+	raw_spin_unlock_irqrestore(&q->lock, flags);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(prepare_to_swait_event);
 

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [tip:sched/core] sched/swait: Rename to exclusive
  2018-06-12  8:34 ` [PATCH 3/3] sched/swait: Rename to exclusive Peter Zijlstra
@ 2018-06-20  9:40   ` tip-bot for Peter Zijlstra
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Peter Zijlstra @ 2018-06-20  9:40 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: tglx, hpa, mingo, peterz, torvalds, linux-kernel

Commit-ID:  b3dae109fa89d67334bf3349babab3ad9b6f233f
Gitweb:     https://git.kernel.org/tip/b3dae109fa89d67334bf3349babab3ad9b6f233f
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Tue, 12 Jun 2018 10:34:52 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 20 Jun 2018 11:35:56 +0200

sched/swait: Rename to exclusive

Since swait basically implemented exclusive waits only, make sure
the API reflects that.

  $ git grep -l -e "\<swake_up\>"
		-e "\<swait_event[^ (]*"
		-e "\<prepare_to_swait\>" | while read file;
    do
	sed -i -e 's/\<swake_up\>/&_one/g'
	       -e 's/\<swait_event[^ (]*/&_exclusive/g'
	       -e 's/\<prepare_to_swait\>/&_exclusive/g' $file;
    done

With a few manual touch-ups.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: bigeasy@linutronix.de
Cc: oleg@redhat.com
Cc: paulmck@linux.vnet.ibm.com
Cc: pbonzini@redhat.com
Link: https://lkml.kernel.org/r/20180612083909.261946548@infradead.org

---
 arch/mips/kvm/mips.c         |  4 ++--
 arch/powerpc/kvm/book3s_hv.c |  6 +++---
 arch/s390/kvm/interrupt.c    |  2 +-
 arch/x86/kernel/kvm.c        |  4 ++--
 arch/x86/kvm/lapic.c         |  2 +-
 include/linux/swait.h        | 24 ++++++++++++------------
 kernel/power/suspend.c       |  4 ++--
 kernel/rcu/srcutiny.c        |  4 ++--
 kernel/rcu/tree.c            |  8 ++++----
 kernel/rcu/tree_exp.h        |  4 ++--
 kernel/rcu/tree_plugin.h     | 12 ++++++------
 kernel/sched/swait.c         | 10 +++++-----
 virt/kvm/arm/arm.c           |  4 ++--
 virt/kvm/arm/psci.c          |  2 +-
 virt/kvm/async_pf.c          |  2 +-
 virt/kvm/kvm_main.c          |  4 ++--
 16 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 7cd76f93a438..f7ea8e21656b 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -515,7 +515,7 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
 	dvcpu->arch.wait = 0;
 
 	if (swq_has_sleeper(&dvcpu->wq))
-		swake_up(&dvcpu->wq);
+		swake_up_one(&dvcpu->wq);
 
 	return 0;
 }
@@ -1204,7 +1204,7 @@ static void kvm_mips_comparecount_func(unsigned long data)
 
 	vcpu->arch.wait = 0;
 	if (swq_has_sleeper(&vcpu->wq))
-		swake_up(&vcpu->wq);
+		swake_up_one(&vcpu->wq);
 }
 
 /* low level hrtimer wake routine */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index de686b340f4a..ee4a8854985e 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -216,7 +216,7 @@ static void kvmppc_fast_vcpu_kick_hv(struct kvm_vcpu *vcpu)
 
 	wqp = kvm_arch_vcpu_wq(vcpu);
 	if (swq_has_sleeper(wqp)) {
-		swake_up(wqp);
+		swake_up_one(wqp);
 		++vcpu->stat.halt_wakeup;
 	}
 
@@ -3188,7 +3188,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
 		}
 	}
 
-	prepare_to_swait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
+	prepare_to_swait_exclusive(&vc->wq, &wait, TASK_INTERRUPTIBLE);
 
 	if (kvmppc_vcore_check_block(vc)) {
 		finish_swait(&vc->wq, &wait);
@@ -3311,7 +3311,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) {
-			swake_up(&vc->wq);
+			swake_up_one(&vc->wq);
 		}
 
 	}
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index daa09f89ca2d..fcb55b02990e 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1145,7 +1145,7 @@ void kvm_s390_vcpu_wakeup(struct kvm_vcpu *vcpu)
 		 * yield-candidate.
 		 */
 		vcpu->preempted = true;
-		swake_up(&vcpu->wq);
+		swake_up_one(&vcpu->wq);
 		vcpu->stat.halt_wakeup++;
 	}
 	/*
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 5b2300b818af..a37bda38d205 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -154,7 +154,7 @@ void kvm_async_pf_task_wait(u32 token, int interrupt_kernel)
 
 	for (;;) {
 		if (!n.halted)
-			prepare_to_swait(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
+			prepare_to_swait_exclusive(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
 		if (hlist_unhashed(&n.link))
 			break;
 
@@ -188,7 +188,7 @@ static void apf_task_wake_one(struct kvm_task_sleep_node *n)
 	if (n->halted)
 		smp_send_reschedule(n->cpu);
 	else if (swq_has_sleeper(&n->wq))
-		swake_up(&n->wq);
+		swake_up_one(&n->wq);
 }
 
 static void apf_task_wake_all(void)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b5cd8465d44f..d536d457517b 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1379,7 +1379,7 @@ static void apic_timer_expired(struct kvm_lapic *apic)
 	 * using swait_active() is safe.
 	 */
 	if (swait_active(q))
-		swake_up(q);
+		swake_up_one(q);
 
 	if (apic_lvtt_tscdeadline(apic))
 		ktimer->expired_tscdeadline = ktimer->tscdeadline;
diff --git a/include/linux/swait.h b/include/linux/swait.h
index dd032061112d..73e06e9986d4 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -16,7 +16,7 @@
  * wait-queues, but the semantics are actually completely different, and
  * every single user we have ever had has been buggy (or pointless).
  *
- * A "swake_up()" only wakes up _one_ waiter, which is not at all what
+ * A "swake_up_one()" only wakes up _one_ waiter, which is not at all what
  * "wake_up()" does, and has led to problems. In other cases, it has
  * been fine, because there's only ever one waiter (kvm), but in that
  * case gthe whole "simple" wait-queue is just pointless to begin with,
@@ -115,7 +115,7 @@ extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name
  *      CPU0 - waker                    CPU1 - waiter
  *
  *                                      for (;;) {
- *      @cond = true;                     prepare_to_swait(&wq_head, &wait, state);
+ *      @cond = true;                     prepare_to_swait_exclusive(&wq_head, &wait, state);
  *      smp_mb();                         // smp_mb() from set_current_state()
  *      if (swait_active(wq_head))        if (@cond)
  *        wake_up(wq_head);                      break;
@@ -157,11 +157,11 @@ static inline bool swq_has_sleeper(struct swait_queue_head *wq)
 	return swait_active(wq);
 }
 
-extern void swake_up(struct swait_queue_head *q);
+extern void swake_up_one(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, int state);
+extern void prepare_to_swait_exclusive(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);
@@ -196,7 +196,7 @@ __out:	__ret;								\
 	(void)___swait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0,	\
 			    schedule())
 
-#define swait_event(wq, condition)					\
+#define swait_event_exclusive(wq, condition)				\
 do {									\
 	if (condition)							\
 		break;							\
@@ -208,7 +208,7 @@ do {									\
 		      TASK_UNINTERRUPTIBLE, timeout,			\
 		      __ret = schedule_timeout(__ret))
 
-#define swait_event_timeout(wq, condition, timeout)			\
+#define swait_event_timeout_exclusive(wq, condition, timeout)		\
 ({									\
 	long __ret = timeout;						\
 	if (!___wait_cond_timeout(condition))				\
@@ -220,7 +220,7 @@ do {									\
 	___swait_event(wq, condition, TASK_INTERRUPTIBLE, 0,		\
 		      schedule())
 
-#define swait_event_interruptible(wq, condition)			\
+#define swait_event_interruptible_exclusive(wq, condition)		\
 ({									\
 	int __ret = 0;							\
 	if (!(condition))						\
@@ -233,7 +233,7 @@ do {									\
 		      TASK_INTERRUPTIBLE, timeout,			\
 		      __ret = schedule_timeout(__ret))
 
-#define swait_event_interruptible_timeout(wq, condition, timeout)	\
+#define swait_event_interruptible_timeout_exclusive(wq, condition, timeout)\
 ({									\
 	long __ret = timeout;						\
 	if (!___wait_cond_timeout(condition))				\
@@ -246,7 +246,7 @@ do {									\
 	(void)___swait_event(wq, condition, TASK_IDLE, 0, schedule())
 
 /**
- * swait_event_idle - wait without system load contribution
+ * swait_event_idle_exclusive - wait without system load contribution
  * @wq: the waitqueue to wait on
  * @condition: a C expression for the event to wait for
  *
@@ -257,7 +257,7 @@ do {									\
  * condition and doesn't want to contribute to system load. Signals are
  * ignored.
  */
-#define swait_event_idle(wq, condition)					\
+#define swait_event_idle_exclusive(wq, condition)			\
 do {									\
 	if (condition)							\
 		break;							\
@@ -270,7 +270,7 @@ do {									\
 		       __ret = schedule_timeout(__ret))
 
 /**
- * swait_event_idle_timeout - wait up to timeout without load contribution
+ * swait_event_idle_timeout_exclusive - wait up to timeout without load contribution
  * @wq: the waitqueue to wait on
  * @condition: a C expression for the event to wait for
  * @timeout: timeout at which we'll give up in jiffies
@@ -288,7 +288,7 @@ do {									\
  * or the remaining jiffies (at least 1) if the @condition evaluated
  * to %true before the @timeout elapsed.
  */
-#define swait_event_idle_timeout(wq, condition, timeout)		\
+#define swait_event_idle_timeout_exclusive(wq, condition, timeout)	\
 ({									\
 	long __ret = timeout;						\
 	if (!___wait_cond_timeout(condition))				\
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 87331565e505..70178f6ffdc4 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -92,7 +92,7 @@ static void s2idle_enter(void)
 	/* Push all the CPUs into the idle loop. */
 	wake_up_all_idle_cpus();
 	/* Make the current CPU wait so it can enter the idle loop too. */
-	swait_event(s2idle_wait_head,
+	swait_event_exclusive(s2idle_wait_head,
 		    s2idle_state == S2IDLE_STATE_WAKE);
 
 	cpuidle_pause();
@@ -160,7 +160,7 @@ void s2idle_wake(void)
 	raw_spin_lock_irqsave(&s2idle_lock, flags);
 	if (s2idle_state > S2IDLE_STATE_NONE) {
 		s2idle_state = S2IDLE_STATE_WAKE;
-		swake_up(&s2idle_wait_head);
+		swake_up_one(&s2idle_wait_head);
 	}
 	raw_spin_unlock_irqrestore(&s2idle_lock, flags);
 }
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 622792abe41a..04fc2ed71af8 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -110,7 +110,7 @@ void __srcu_read_unlock(struct srcu_struct *sp, int idx)
 
 	WRITE_ONCE(sp->srcu_lock_nesting[idx], newval);
 	if (!newval && READ_ONCE(sp->srcu_gp_waiting))
-		swake_up(&sp->srcu_wq);
+		swake_up_one(&sp->srcu_wq);
 }
 EXPORT_SYMBOL_GPL(__srcu_read_unlock);
 
@@ -140,7 +140,7 @@ void srcu_drive_gp(struct work_struct *wp)
 	idx = sp->srcu_idx;
 	WRITE_ONCE(sp->srcu_idx, !sp->srcu_idx);
 	WRITE_ONCE(sp->srcu_gp_waiting, true);  /* srcu_read_unlock() wakes! */
-	swait_event(sp->srcu_wq, !READ_ONCE(sp->srcu_lock_nesting[idx]));
+	swait_event_exclusive(sp->srcu_wq, !READ_ONCE(sp->srcu_lock_nesting[idx]));
 	WRITE_ONCE(sp->srcu_gp_waiting, false); /* srcu_read_unlock() cheap. */
 
 	/* Invoke the callbacks we removed above. */
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index aa7cade1b9f3..91f888d3b23a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1727,7 +1727,7 @@ static void rcu_gp_kthread_wake(struct rcu_state *rsp)
 	    !READ_ONCE(rsp->gp_flags) ||
 	    !rsp->gp_kthread)
 		return;
-	swake_up(&rsp->gp_wq);
+	swake_up_one(&rsp->gp_wq);
 }
 
 /*
@@ -2002,7 +2002,7 @@ static bool rcu_gp_init(struct rcu_state *rsp)
 }
 
 /*
- * Helper function for swait_event_idle() wakeup at force-quiescent-state
+ * Helper function for swait_event_idle_exclusive() wakeup at force-quiescent-state
  * time.
  */
 static bool rcu_gp_fqs_check_wake(struct rcu_state *rsp, int *gfp)
@@ -2144,7 +2144,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
 					       READ_ONCE(rsp->gpnum),
 					       TPS("reqwait"));
 			rsp->gp_state = RCU_GP_WAIT_GPS;
-			swait_event_idle(rsp->gp_wq, READ_ONCE(rsp->gp_flags) &
+			swait_event_idle_exclusive(rsp->gp_wq, READ_ONCE(rsp->gp_flags) &
 						     RCU_GP_FLAG_INIT);
 			rsp->gp_state = RCU_GP_DONE_GPS;
 			/* Locking provides needed memory barrier. */
@@ -2176,7 +2176,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
 					       READ_ONCE(rsp->gpnum),
 					       TPS("fqswait"));
 			rsp->gp_state = RCU_GP_WAIT_FQS;
-			ret = swait_event_idle_timeout(rsp->gp_wq,
+			ret = swait_event_idle_timeout_exclusive(rsp->gp_wq,
 					rcu_gp_fqs_check_wake(rsp, &gf), j);
 			rsp->gp_state = RCU_GP_DOING_FQS;
 			/* Locking provides needed memory barriers. */
diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index d40708e8c5d6..d428cc1064c8 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -212,7 +212,7 @@ static void __rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp,
 			raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 			if (wake) {
 				smp_mb(); /* EGP done before wake_up(). */
-				swake_up(&rsp->expedited_wq);
+				swake_up_one(&rsp->expedited_wq);
 			}
 			break;
 		}
@@ -518,7 +518,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
 	jiffies_start = jiffies;
 
 	for (;;) {
-		ret = swait_event_timeout(
+		ret = swait_event_timeout_exclusive(
 				rsp->expedited_wq,
 				sync_rcu_preempt_exp_done_unlocked(rnp_root),
 				jiffies_stall);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 7fd12039e512..ad53d133f709 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1854,8 +1854,8 @@ static void __wake_nocb_leader(struct rcu_data *rdp, bool force,
 		WRITE_ONCE(rdp_leader->nocb_leader_sleep, false);
 		del_timer(&rdp->nocb_timer);
 		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
-		smp_mb(); /* ->nocb_leader_sleep before swake_up(). */
-		swake_up(&rdp_leader->nocb_wq);
+		smp_mb(); /* ->nocb_leader_sleep before swake_up_one(). */
+		swake_up_one(&rdp_leader->nocb_wq);
 	} else {
 		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 	}
@@ -2082,7 +2082,7 @@ static void rcu_nocb_wait_gp(struct rcu_data *rdp)
 	 */
 	trace_rcu_this_gp(rnp, rdp, c, TPS("StartWait"));
 	for (;;) {
-		swait_event_interruptible(
+		swait_event_interruptible_exclusive(
 			rnp->nocb_gp_wq[c & 0x1],
 			(d = ULONG_CMP_GE(READ_ONCE(rnp->completed), c)));
 		if (likely(d))
@@ -2111,7 +2111,7 @@ wait_again:
 	/* Wait for callbacks to appear. */
 	if (!rcu_nocb_poll) {
 		trace_rcu_nocb_wake(my_rdp->rsp->name, my_rdp->cpu, TPS("Sleep"));
-		swait_event_interruptible(my_rdp->nocb_wq,
+		swait_event_interruptible_exclusive(my_rdp->nocb_wq,
 				!READ_ONCE(my_rdp->nocb_leader_sleep));
 		raw_spin_lock_irqsave(&my_rdp->nocb_lock, flags);
 		my_rdp->nocb_leader_sleep = true;
@@ -2176,7 +2176,7 @@ wait_again:
 		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 		if (rdp != my_rdp && tail == &rdp->nocb_follower_head) {
 			/* List was empty, so wake up the follower.  */
-			swake_up(&rdp->nocb_wq);
+			swake_up_one(&rdp->nocb_wq);
 		}
 	}
 
@@ -2193,7 +2193,7 @@ static void nocb_follower_wait(struct rcu_data *rdp)
 {
 	for (;;) {
 		trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu, TPS("FollowerSleep"));
-		swait_event_interruptible(rdp->nocb_wq,
+		swait_event_interruptible_exclusive(rdp->nocb_wq,
 					 READ_ONCE(rdp->nocb_follower_head));
 		if (smp_load_acquire(&rdp->nocb_follower_head)) {
 			/* ^^^ Ensure CB invocation follows _head test. */
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
index 66890de93ee5..66b59ac77c22 100644
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -32,7 +32,7 @@ void swake_up_locked(struct swait_queue_head *q)
 }
 EXPORT_SYMBOL(swake_up_locked);
 
-void swake_up(struct swait_queue_head *q)
+void swake_up_one(struct swait_queue_head *q)
 {
 	unsigned long flags;
 
@@ -40,7 +40,7 @@ void swake_up(struct swait_queue_head *q)
 	swake_up_locked(q);
 	raw_spin_unlock_irqrestore(&q->lock, flags);
 }
-EXPORT_SYMBOL(swake_up);
+EXPORT_SYMBOL(swake_up_one);
 
 /*
  * Does not allow usage from IRQ disabled, since we must be able to
@@ -76,7 +76,7 @@ static void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *w
 		list_add_tail(&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_exclusive(struct swait_queue_head *q, struct swait_queue *wait, int state)
 {
 	unsigned long flags;
 
@@ -85,7 +85,7 @@ void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int
 	set_current_state(state);
 	raw_spin_unlock_irqrestore(&q->lock, flags);
 }
-EXPORT_SYMBOL(prepare_to_swait);
+EXPORT_SYMBOL(prepare_to_swait_exclusive);
 
 long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
 {
@@ -95,7 +95,7 @@ long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait
 	raw_spin_lock_irqsave(&q->lock, flags);
 	if (unlikely(signal_pending_state(state, current))) {
 		/*
-		 * See prepare_to_wait_event(). TL;DR, subsequent swake_up()
+		 * See prepare_to_wait_event(). TL;DR, subsequent swake_up_one()
 		 * must not see us.
 		 */
 		list_del_init(&wait->task_list);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 04e554cae3a2..108250e4d376 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -604,7 +604,7 @@ void kvm_arm_resume_guest(struct kvm *kvm)
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		vcpu->arch.pause = false;
-		swake_up(kvm_arch_vcpu_wq(vcpu));
+		swake_up_one(kvm_arch_vcpu_wq(vcpu));
 	}
 }
 
@@ -612,7 +612,7 @@ static void vcpu_req_sleep(struct kvm_vcpu *vcpu)
 {
 	struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
 
-	swait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
+	swait_event_interruptible_exclusive(*wq, ((!vcpu->arch.power_off) &&
 				       (!vcpu->arch.pause)));
 
 	if (vcpu->arch.power_off || vcpu->arch.pause) {
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index c95ab4c5a475..9b73d3ad918a 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -155,7 +155,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);
-	swake_up(wq);
+	swake_up_one(wq);
 
 	return PSCI_RET_SUCCESS;
 }
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 57bcb27dcf30..23c2519c5b32 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -107,7 +107,7 @@ static void async_pf_execute(struct work_struct *work)
 	trace_kvm_async_pf_completed(addr, gva);
 
 	if (swq_has_sleeper(&vcpu->wq))
-		swake_up(&vcpu->wq);
+		swake_up_one(&vcpu->wq);
 
 	mmput(mm);
 	kvm_put_kvm(vcpu->kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ada21f47f22b..940a4aed5b2d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2167,7 +2167,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	kvm_arch_vcpu_blocking(vcpu);
 
 	for (;;) {
-		prepare_to_swait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+		prepare_to_swait_exclusive(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
 		if (kvm_vcpu_check_block(vcpu) < 0)
 			break;
@@ -2209,7 +2209,7 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu)
 
 	wqp = kvm_arch_vcpu_wq(vcpu);
 	if (swq_has_sleeper(wqp)) {
-		swake_up(wqp);
+		swake_up_one(wqp);
 		++vcpu->stat.halt_wakeup;
 		return true;
 	}

^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-06-20  9:41 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-12  8:34 [PATCH 0/3] sched/swait: Convert to full exclusive mode Peter Zijlstra
2018-06-12  8:34 ` [PATCH 1/3] sched/swait: Remove __prepare_to_swait Peter Zijlstra
2018-06-20  9:39   ` [tip:sched/core] " tip-bot for Peter Zijlstra
2018-06-12  8:34 ` [PATCH 2/3] sched/swait: Switch to full exclusive mode Peter Zijlstra
2018-06-20  9:40   ` [tip:sched/core] " tip-bot for Peter Zijlstra
2018-06-12  8:34 ` [PATCH 3/3] sched/swait: Rename to exclusive Peter Zijlstra
2018-06-20  9:40   ` [tip:sched/core] " tip-bot for Peter Zijlstra
2018-06-12 16:47 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Linus Torvalds
2018-06-12 17:14   ` Quilt vs gmail (Was: [PATCH 0/3] sched/swait: Convert to full exclusive mode) Peter Zijlstra
2018-06-13 12:32     ` Jean Delvare
2018-06-13 13:27       ` Andreas Grünbacher
2018-06-13 13:48         ` Linus Torvalds
2018-06-13 14:40         ` Jean Delvare
2018-06-12 18:52   ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Andreas Grünbacher
     [not found]     ` <CA+55aFx81igOjFZcvO03mvDFd3=pxsq2QuNrWrPW+4pvJy780A@mail.gmail.com>
2018-06-12 19:43       ` Thomas Gleixner
2018-06-12 21:54         ` Sebastian Andrzej Siewior
2018-06-12 22:03           ` Linus Torvalds
2018-06-12 22:55             ` Randy Dunlap
2018-06-13 13:00       ` [Quilt-dev] Quilt vs gmail Jean Delvare
2018-06-13 13:35         ` Greg KH
2018-06-14  1:27 ` [PATCH 0/3] sched/swait: Convert to full exclusive mode Paul E. McKenney
2018-06-19 14:49   ` Paul E. McKenney

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.