drivers/xen/preempt: use need_resched() instead of should_resched()

PATCH Applied Konstantin Khlebnikov 5 participants 16 messages Jul 15, 2015 – Aug 3, 2015

Subsystems (6)

x86 PowerPC Scheduler Generic include/asm header files +2 more subsystems

lore

Share & tools

lore

Konstantin Khlebnikov

author

2015-07-15 09:52 UTC #0 lore raw

PATCH v2 1/3 drivers/xen/preempt: use need_resched() instead of should_resched()

This code is used only when CONFIG_PREEMPT=n and only in non-atomic context:
xen_in_preemptible_hcall is set only in privcmd_ioctl_hypercall(). Thus preempt_count is zero and should_resched() is equal to need_resched().

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 drivers/xen/preempt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/xen/preempt.c +1 -1

index a1800c150839..08cb419eb4e6 100644
@@ -31,7 +31,7 @@ EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
 asmlinkage __visible void xen_maybe_preempt_hcall(void)
 {
 	if (unlikely(__this_cpu_read(xen_in_preemptible_hcall)
-		     && should_resched())) {
+		     && need_resched())) {
 		/*
 		 * Clear flag as we may be rescheduled on a different
 		 * cpu.

Konstantin Khlebnikov

author

2015-07-15 09:52 UTC #1 lore raw

PATCH v2 2/3 KVM: PPC: Book3S HV: Use need_resched() instead of should_resched()

Function should_resched() is equal to (!preempt_count() && need_resched()). In preemptive kernel preempt_count here is non-zero because of vc->lock.

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 arch/powerpc/kvm/book3s_hv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/kvm/book3s_hv.c +1 -1

index 68d067ad4222..a9f753fb73a8 100644
@@ -2178,7 +2178,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 		vc->runner = vcpu;
 		if (n_ceded = vc->n_runnable) {
 			kvmppc_vcore_blocked(vc);
-		} else if (should_resched()) {
+		} else if (need_resched()) {
 			vc->vcore_state = VCORE_PREEMPT;
 			/* Let something else run */
 			cond_resched_lock(&vc->lock);

tip-bot for Konstantin Khlebnikov

merge bot

2015-08-03 17:08 UTC #2 lore raw

[tip:sched/core] sched/preempt, powerpc, kvm: Use need_resched() instead of should_resched()

sched/preempt, powerpc, kvm: Use need_resched() instead of should_resched()

Commit-ID:  c56dadf39761a6157239cac39e3988998c994f98
Gitweb:     http://git.kernel.org/tip/c56dadf39761a6157239cac39e3988998c994f98
Author:     Konstantin Khlebnikov <[email protected]>
AuthorDate: Wed, 15 Jul 2015 12:52:03 +0300
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 3 Aug 2015 12:21:24 +0200

sched/preempt, powerpc, kvm: Use need_resched() instead of should_resched()

Function should_resched() is equal to (!preempt_count() && need_resched()). In preemptive kernel preempt_count here is non-zero because of vc->lock.

Signed-off-by Konstantin Khlebnikov <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Cc Alexander Graf <[email protected]>

Cc Boris Ostrovsky <[email protected]>

Cc David Vrabel <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mike Galbraith <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/20150715095203.12246.72922.stgit@buzz open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/powerpc/kvm/book3s_hv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/kvm/book3s_hv.c +1 -1

index 68d067a..a9f753f 100644
@@ -2178,7 +2178,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 		vc->runner = vcpu;
 		if (n_ceded == vc->n_runnable) {
 			kvmppc_vcore_blocked(vc);
-		} else if (should_resched()) {
+		} else if (need_resched()) {
 			vc->vcore_state = VCORE_PREEMPT;
 			/* Let something else run */
 			cond_resched_lock(&vc->lock);

Konstantin Khlebnikov

author

2015-07-15 09:52 UTC #3 lore raw

PATCH v2 2/3 KVM: PPC: Book3S HV: Use need_resched() instead of should_resched()

In reply to Konstantin Khlebnikov

Function should_resched() is equal to (!preempt_count() && need_resched()). In preemptive kernel preempt_count here is non-zero because of vc->lock.

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 arch/powerpc/kvm/book3s_hv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/kvm/book3s_hv.c +1 -1

index 68d067ad4222..a9f753fb73a8 100644
@@ -2178,7 +2178,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
 		vc->runner = vcpu;
 		if (n_ceded == vc->n_runnable) {
 			kvmppc_vcore_blocked(vc);
-		} else if (should_resched()) {
+		} else if (need_resched()) {
 			vc->vcore_state = VCORE_PREEMPT;
 			/* Let something else run */
 			cond_resched_lock(&vc->lock);

Konstantin Khlebnikov

author

2015-07-15 09:52 UTC #4 lore raw

PATCH v2 3/3 sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

In reply to Konstantin Khlebnikov

These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.

This patch adds argument "preempt_offset" to should_resched().

preempt_count offset constants for that:

PREEMPT_DISABLE_OFFSET  - offset after preempt_disable()
PREEMPT_LOCK_OFFSET     - offset after spin_lock()
SOFTIRQ_DISABLE_OFFSET  - offset after local_bh_distable()
SOFTIRQ_LOCK_OFFSET     - offset after spin_lock_bh()

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 arch/x86/include/asm/preempt.h |    4 ++--
 include/asm-generic/preempt.h  |    5 +++--
 include/linux/preempt.h        |   19 ++++++++++++++-----
 include/linux/sched.h          |    6 ------
 kernel/sched/core.c            |    6 +++---
 5 files changed, 22 insertions(+), 18 deletions(-)

arch/x86/include/asm/preempt.h +2 -2

index dca71714f860..b12f81022a6b 100644
@@ -90,9 +90,9 @@ static __always_inline bool __preempt_count_dec_and_test(void)
 /*
  * Returns true when we need to resched and can (barring IRQ state).
  */
-static __always_inline bool should_resched(void)
+static __always_inline bool should_resched(int preempt_offset)
 {
-	return unlikely(!raw_cpu_read_4(__preempt_count));
+	return unlikely(raw_cpu_read_4(__preempt_count) = preempt_offset);
 }
 
 #ifdef CONFIG_PREEMPT

include/asm-generic/preempt.h +3 -2

index d0a7a4753db2..0bec580a4885 100644
@@ -71,9 +71,10 @@ static __always_inline bool __preempt_count_dec_and_test(void)
 /*
  * Returns true when we need to resched and can (barring IRQ state).
  */
-static __always_inline bool should_resched(void)
+static __always_inline bool should_resched(int preempt_offset)
 {
-	return unlikely(!preempt_count() && tif_need_resched());
+	return unlikely(preempt_count() = preempt_offset &&
+			tif_need_resched());
 }
 
 #ifdef CONFIG_PREEMPT

include/linux/preempt.h +14 -5

index 84991f185173..bea8dd8ff5e0 100644
@@ -84,13 +84,21 @@
  */
 #define in_nmi()	(preempt_count() & NMI_MASK)
 
+/*
+ * The preempt_count offset after preempt_disable();
+ */
 #if defined(CONFIG_PREEMPT_COUNT)
-# define PREEMPT_DISABLE_OFFSET 1
+# define PREEMPT_DISABLE_OFFSET	PREEMPT_OFFSET
 #else
-# define PREEMPT_DISABLE_OFFSET 0
+# define PREEMPT_DISABLE_OFFSET	0
 #endif
 
 /*
+ * The preempt_count offset after spin_lock()
+ */
+#define PREEMPT_LOCK_OFFSET	PREEMPT_DISABLE_OFFSET
+
+/*
  * The preempt_count offset needed for things like:
  *
  *  spin_lock_bh()

@@ -103,7 +111,7 @@
  *
  * Work as expected.
  */
-#define SOFTIRQ_LOCK_OFFSET (SOFTIRQ_DISABLE_OFFSET + PREEMPT_DISABLE_OFFSET)
+#define SOFTIRQ_LOCK_OFFSET (SOFTIRQ_DISABLE_OFFSET + PREEMPT_LOCK_OFFSET)
 
 /*
  * Are we running in atomic context?  WARNING: this macro cannot

@@ -124,7 +132,8 @@
 #if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_PREEMPT_TRACER)
 extern void preempt_count_add(int val);
 extern void preempt_count_sub(int val);
-#define preempt_count_dec_and_test() ({ preempt_count_sub(1); should_resched(); })
+#define preempt_count_dec_and_test() \
+	({ preempt_count_sub(1); should_resched(0); })
 #else
 #define preempt_count_add(val)	__preempt_count_add(val)
 #define preempt_count_sub(val)	__preempt_count_sub(val)

@@ -184,7 +193,7 @@ do { \
 
 #define preempt_check_resched() \
 do { \
-	if (should_resched()) \
+	if (should_resched(0)) \
 		__preempt_schedule(); \
 } while (0)

include/linux/sched.h +0 -6

index ae21f1591615..a8e9b17acdee 100644
@@ -2885,12 +2885,6 @@ extern int _cond_resched(void);
 
 extern int __cond_resched_lock(spinlock_t *lock);
 
-#ifdef CONFIG_PREEMPT_COUNT
-#define PREEMPT_LOCK_OFFSET	PREEMPT_OFFSET
-#else
-#define PREEMPT_LOCK_OFFSET	0
-#endif
-
 #define cond_resched_lock(lock) ({				\
 	___might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET);\
 	__cond_resched_lock(lock);				\

kernel/sched/core.c +3 -3

index 78b4bad10081..d9a4d93dc879 100644
@@ -4492,7 +4492,7 @@ SYSCALL_DEFINE0(sched_yield)
 
 int __sched _cond_resched(void)
 {
-	if (should_resched()) {
+	if (should_resched(0)) {
 		preempt_schedule_common();
 		return 1;
 	}

@@ -4510,7 +4510,7 @@ EXPORT_SYMBOL(_cond_resched);
  */
 int __cond_resched_lock(spinlock_t *lock)
 {
-	int resched = should_resched();
+	int resched = should_resched(PREEMPT_LOCK_OFFSET);
 	int ret = 0;
 
 	lockdep_assert_held(lock);

@@ -4532,7 +4532,7 @@ int __sched __cond_resched_softirq(void)
 {
 	BUG_ON(!in_softirq());
 
-	if (should_resched()) {
+	if (should_resched(SOFTIRQ_DISABLE_OFFSET)) {
 		local_bh_enable();
 		preempt_schedule_common();
 		local_bh_disable();

Eric Dumazet

2015-07-15 12:16 UTC #5 lore raw

Re: [PATCH v2 3/3] sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

Quoted 3 lines · Konstantin Khlebnikov

These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.

Interesting, this definitely used to work (linux-3.11)

Any idea of which commit broke things ?

Konstantin Khlebnikov

author

2015-07-15 12:52 UTC #6 lore raw

Re: [PATCH v2 3/3] sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

Quoted 9 lines · Eric Dumazet

Quoted 3 lines · Konstantin Khlebnikov
These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.
Interesting, this definitely used to work (linux-3.11)
Any idea of which commit broke things ?

Searching... done

This one: bdb43806589096ac4272fe1307e789846ac08d7c in v3.13

before

-static inline int should_resched(void)
-{
- return need_resched() && !(preempt_count() & PREEMPT_ACTIVE);
-}

after

+static __always_inline bool should_resched(void)
+{
+ return unlikely(!*preempt_count_ptr());
+}

So,

Fixes bdb438065890 ("sched: Extract the basic add/sub preempt_count

modifiers")

Signature · 1 line

Konstantin

Peter Zijlstra

maintainer

2015-07-15 13:35 UTC #7 lore raw

Re: [PATCH v2 3/3] sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

Quoted 24 lines · Konstantin Khlebnikov

Quoted 9 lines · Eric Dumazet
Quoted 3 lines · Konstantin Khlebnikov
These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.
Interesting, this definitely used to work (linux-3.11)
Any idea of which commit broke things ?
Searching... done
This one: bdb43806589096ac4272fe1307e789846ac08d7c in v3.13
before
(quoted diff) +0 -4
-static inline int should_resched(void)
-{
-       return need_resched() && !(preempt_count() & PREEMPT_ACTIVE);
-}
after

Review

Quoted 4 lines

(quoted diff) +4 -0

+static __always_inline bool should_resched(void)
+{
+       return unlikely(!*preempt_count_ptr());
+}

Argh, indeed!

Quoted 5 lines

So,
Fixes bdb438065890 ("sched: Extract the basic add/sub preempt_count
modifiers")

Thanks!

Peter Zijlstra

maintainer

2015-07-15 13:35 UTC #8 lore raw

Re: [PATCH v2 3/3] sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

In reply to Konstantin Khlebnikov

Quoted 24 lines · Konstantin Khlebnikov

Quoted 9 lines · Eric Dumazet
Quoted 3 lines · Konstantin Khlebnikov
These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.
Interesting, this definitely used to work (linux-3.11)
Any idea of which commit broke things ?
Searching... done
This one: bdb43806589096ac4272fe1307e789846ac08d7c in v3.13
before
(quoted diff) +0 -4
-static inline int should_resched(void)
-{
-       return need_resched() && !(preempt_count() & PREEMPT_ACTIVE);
-}
after

Review

Quoted 4 lines

(quoted diff) +4 -0

+static __always_inline bool should_resched(void)
+{
+       return unlikely(!*preempt_count_ptr());
+}

Argh, indeed!

Quoted 5 lines

So,
Fixes bdb438065890 ("sched: Extract the basic add/sub preempt_count
modifiers")

Thanks!

Konstantin Khlebnikov

author

2015-07-15 12:52 UTC #9 lore raw

Re: [PATCH v2 3/3] sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

In reply to Eric Dumazet

Quoted 9 lines · Eric Dumazet

Quoted 3 lines · Konstantin Khlebnikov
These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.
Interesting, this definitely used to work (linux-3.11)
Any idea of which commit broke things ?

Searching... done

This one: bdb43806589096ac4272fe1307e789846ac08d7c in v3.13

before

-static inline int should_resched(void)
-{
- return need_resched() && !(preempt_count() & PREEMPT_ACTIVE);
-}

after

+static __always_inline bool should_resched(void)
+{
+ return unlikely(!*preempt_count_ptr());
+}

So,

Fixes bdb438065890 ("sched: Extract the basic add/sub preempt_count modifiers")

Signature · 1 line

Konstantin

Eric Dumazet

2015-07-15 12:16 UTC #10 lore raw

Re: [PATCH v2 3/3] sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

In reply to Konstantin Khlebnikov

Quoted 3 lines · Konstantin Khlebnikov

These functions check should_resched() before unlocking spinlock/bh-enable:
preempt_count always non-zero => should_resched() always returns false. cond_resched_lock() worked iff spin_needbreak is set.

Interesting, this definitely used to work (linux-3.11)

Any idea of which commit broke things ?

tip-bot for Konstantin Khlebnikov

merge bot

2015-08-03 17:09 UTC #11 lore raw

[tip:sched/core] sched/preempt: Fix cond_resched_lock() and cond_resched_softirq()

In reply to Konstantin Khlebnikov

sched/preempt: Fix cond_resched_lock() and cond_resched_softirq()

Commit-ID:  fe32d3cd5e8eb0f82e459763374aa80797023403
Gitweb:     http://git.kernel.org/tip/fe32d3cd5e8eb0f82e459763374aa80797023403
Author:     Konstantin Khlebnikov <[email protected]>
AuthorDate: Wed, 15 Jul 2015 12:52:04 +0300
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 3 Aug 2015 12:21:24 +0200

sched/preempt: Fix cond_resched_lock() and cond_resched_softirq()

This patch adds argument "preempt_offset" to should_resched().

preempt_count offset constants for that:

  PREEMPT_DISABLE_OFFSET  - offset after preempt_disable()
  PREEMPT_LOCK_OFFSET     - offset after spin_lock()
  SOFTIRQ_DISABLE_OFFSET  - offset after local_bh_distable()
  SOFTIRQ_LOCK_OFFSET     - offset after spin_lock_bh()

Signed-off-by Konstantin Khlebnikov <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Cc Alexander Graf <[email protected]>

Cc Boris Ostrovsky <[email protected]>

Cc David Vrabel <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mike Galbraith <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Fixes bdb438065890 ("sched: Extract the basic add/sub preempt_count modifiers")

Link http://lkml.kernel.org/r/20150715095204.12246.98268.stgit@buzz open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/include/asm/preempt.h |  4 ++--
 include/asm-generic/preempt.h  |  5 +++--
 include/linux/preempt.h        | 19 ++++++++++++++-----
 include/linux/sched.h          |  6 ------
 kernel/sched/core.c            |  6 +++---
 5 files changed, 22 insertions(+), 18 deletions(-)

arch/x86/include/asm/preempt.h +2 -2

index dca7171..b12f810 100644
@@ -90,9 +90,9 @@ static __always_inline bool __preempt_count_dec_and_test(void)
 /*
  * Returns true when we need to resched and can (barring IRQ state).
  */
-static __always_inline bool should_resched(void)
+static __always_inline bool should_resched(int preempt_offset)
 {
-	return unlikely(!raw_cpu_read_4(__preempt_count));
+	return unlikely(raw_cpu_read_4(__preempt_count) == preempt_offset);
 }
 
 #ifdef CONFIG_PREEMPT

include/asm-generic/preempt.h +3 -2

index d0a7a47..0bec580 100644
@@ -71,9 +71,10 @@ static __always_inline bool __preempt_count_dec_and_test(void)
 /*
  * Returns true when we need to resched and can (barring IRQ state).
  */
-static __always_inline bool should_resched(void)
+static __always_inline bool should_resched(int preempt_offset)
 {
-	return unlikely(!preempt_count() && tif_need_resched());
+	return unlikely(preempt_count() == preempt_offset &&
+			tif_need_resched());
 }
 
 #ifdef CONFIG_PREEMPT

include/linux/preempt.h +14 -5

index 84991f1..bea8dd8 100644
@@ -84,13 +84,21 @@
  */
 #define in_nmi()	(preempt_count() & NMI_MASK)
 
+/*
+ * The preempt_count offset after preempt_disable();
+ */
 #if defined(CONFIG_PREEMPT_COUNT)
-# define PREEMPT_DISABLE_OFFSET 1
+# define PREEMPT_DISABLE_OFFSET	PREEMPT_OFFSET
 #else
-# define PREEMPT_DISABLE_OFFSET 0
+# define PREEMPT_DISABLE_OFFSET	0
 #endif
 
 /*
+ * The preempt_count offset after spin_lock()
+ */
+#define PREEMPT_LOCK_OFFSET	PREEMPT_DISABLE_OFFSET
+
+/*
  * The preempt_count offset needed for things like:
  *
  *  spin_lock_bh()

@@ -103,7 +111,7 @@
  *
  * Work as expected.
  */
-#define SOFTIRQ_LOCK_OFFSET (SOFTIRQ_DISABLE_OFFSET + PREEMPT_DISABLE_OFFSET)
+#define SOFTIRQ_LOCK_OFFSET (SOFTIRQ_DISABLE_OFFSET + PREEMPT_LOCK_OFFSET)
 
 /*
  * Are we running in atomic context?  WARNING: this macro cannot

@@ -124,7 +132,8 @@
 #if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_PREEMPT_TRACER)
 extern void preempt_count_add(int val);
 extern void preempt_count_sub(int val);
-#define preempt_count_dec_and_test() ({ preempt_count_sub(1); should_resched(); })
+#define preempt_count_dec_and_test() \
+	({ preempt_count_sub(1); should_resched(0); })
 #else
 #define preempt_count_add(val)	__preempt_count_add(val)
 #define preempt_count_sub(val)	__preempt_count_sub(val)

@@ -184,7 +193,7 @@ do { \
 
 #define preempt_check_resched() \
 do { \
-	if (should_resched()) \
+	if (should_resched(0)) \
 		__preempt_schedule(); \
 } while (0)

include/linux/sched.h +0 -6

index 65a8a86..9c14465 100644
@@ -2891,12 +2891,6 @@ extern int _cond_resched(void);
 
 extern int __cond_resched_lock(spinlock_t *lock);
 
-#ifdef CONFIG_PREEMPT_COUNT
-#define PREEMPT_LOCK_OFFSET	PREEMPT_OFFSET
-#else
-#define PREEMPT_LOCK_OFFSET	0
-#endif
-
 #define cond_resched_lock(lock) ({				\
 	___might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET);\
 	__cond_resched_lock(lock);				\

kernel/sched/core.c +3 -3

index fa5826c..f5fad2b 100644
@@ -4496,7 +4496,7 @@ SYSCALL_DEFINE0(sched_yield)
 
 int __sched _cond_resched(void)
 {
-	if (should_resched()) {
+	if (should_resched(0)) {
 		preempt_schedule_common();
 		return 1;
 	}

@@ -4514,7 +4514,7 @@ EXPORT_SYMBOL(_cond_resched);
  */
 int __cond_resched_lock(spinlock_t *lock)
 {
-	int resched = should_resched();
+	int resched = should_resched(PREEMPT_LOCK_OFFSET);
 	int ret = 0;
 
 	lockdep_assert_held(lock);

@@ -4536,7 +4536,7 @@ int __sched __cond_resched_softirq(void)
 {
 	BUG_ON(!in_softirq());
 
-	if (should_resched()) {
+	if (should_resched(SOFTIRQ_DISABLE_OFFSET)) {
 		local_bh_enable();
 		preempt_schedule_common();
 		local_bh_disable();

Konstantin Khlebnikov

author

2015-07-15 09:52 UTC #12 lore raw

PATCH v2 3/3 sched/preempt: fix cond_resched_lock() and cond_resched_softirq()

In reply to Konstantin Khlebnikov

This patch adds argument "preempt_offset" to should_resched().

preempt_count offset constants for that:

PREEMPT_DISABLE_OFFSET  - offset after preempt_disable()
PREEMPT_LOCK_OFFSET     - offset after spin_lock()
SOFTIRQ_DISABLE_OFFSET  - offset after local_bh_distable()
SOFTIRQ_LOCK_OFFSET     - offset after spin_lock_bh()

Signed-off-by Konstantin Khlebnikov <[email protected]>

---
 arch/x86/include/asm/preempt.h |    4 ++--
 include/asm-generic/preempt.h  |    5 +++--
 include/linux/preempt.h        |   19 ++++++++++++++-----
 include/linux/sched.h          |    6 ------
 kernel/sched/core.c            |    6 +++---
 5 files changed, 22 insertions(+), 18 deletions(-)

arch/x86/include/asm/preempt.h +2 -2

index dca71714f860..b12f81022a6b 100644
@@ -90,9 +90,9 @@ static __always_inline bool __preempt_count_dec_and_test(void)
 /*
  * Returns true when we need to resched and can (barring IRQ state).
  */
-static __always_inline bool should_resched(void)
+static __always_inline bool should_resched(int preempt_offset)
 {
-	return unlikely(!raw_cpu_read_4(__preempt_count));
+	return unlikely(raw_cpu_read_4(__preempt_count) == preempt_offset);
 }
 
 #ifdef CONFIG_PREEMPT

include/asm-generic/preempt.h +3 -2

index d0a7a4753db2..0bec580a4885 100644
@@ -71,9 +71,10 @@ static __always_inline bool __preempt_count_dec_and_test(void)
 /*
  * Returns true when we need to resched and can (barring IRQ state).
  */
-static __always_inline bool should_resched(void)
+static __always_inline bool should_resched(int preempt_offset)
 {
-	return unlikely(!preempt_count() && tif_need_resched());
+	return unlikely(preempt_count() == preempt_offset &&
+			tif_need_resched());
 }
 
 #ifdef CONFIG_PREEMPT

include/linux/preempt.h +14 -5

index 84991f185173..bea8dd8ff5e0 100644
@@ -84,13 +84,21 @@
  */
 #define in_nmi()	(preempt_count() & NMI_MASK)
 
+/*
+ * The preempt_count offset after preempt_disable();
+ */
 #if defined(CONFIG_PREEMPT_COUNT)
-# define PREEMPT_DISABLE_OFFSET 1
+# define PREEMPT_DISABLE_OFFSET	PREEMPT_OFFSET
 #else
-# define PREEMPT_DISABLE_OFFSET 0
+# define PREEMPT_DISABLE_OFFSET	0
 #endif
 
 /*
+ * The preempt_count offset after spin_lock()
+ */
+#define PREEMPT_LOCK_OFFSET	PREEMPT_DISABLE_OFFSET
+
+/*
  * The preempt_count offset needed for things like:
  *
  *  spin_lock_bh()

@@ -103,7 +111,7 @@
  *
  * Work as expected.
  */
-#define SOFTIRQ_LOCK_OFFSET (SOFTIRQ_DISABLE_OFFSET + PREEMPT_DISABLE_OFFSET)
+#define SOFTIRQ_LOCK_OFFSET (SOFTIRQ_DISABLE_OFFSET + PREEMPT_LOCK_OFFSET)
 
 /*
  * Are we running in atomic context?  WARNING: this macro cannot

@@ -124,7 +132,8 @@
 #if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_PREEMPT_TRACER)
 extern void preempt_count_add(int val);
 extern void preempt_count_sub(int val);
-#define preempt_count_dec_and_test() ({ preempt_count_sub(1); should_resched(); })
+#define preempt_count_dec_and_test() \
+	({ preempt_count_sub(1); should_resched(0); })
 #else
 #define preempt_count_add(val)	__preempt_count_add(val)
 #define preempt_count_sub(val)	__preempt_count_sub(val)

@@ -184,7 +193,7 @@ do { \
 
 #define preempt_check_resched() \
 do { \
-	if (should_resched()) \
+	if (should_resched(0)) \
 		__preempt_schedule(); \
 } while (0)

include/linux/sched.h +0 -6

index ae21f1591615..a8e9b17acdee 100644
@@ -2885,12 +2885,6 @@ extern int _cond_resched(void);
 
 extern int __cond_resched_lock(spinlock_t *lock);
 
-#ifdef CONFIG_PREEMPT_COUNT
-#define PREEMPT_LOCK_OFFSET	PREEMPT_OFFSET
-#else
-#define PREEMPT_LOCK_OFFSET	0
-#endif
-
 #define cond_resched_lock(lock) ({				\
 	___might_sleep(__FILE__, __LINE__, PREEMPT_LOCK_OFFSET);\
 	__cond_resched_lock(lock);				\

kernel/sched/core.c +3 -3

index 78b4bad10081..d9a4d93dc879 100644
@@ -4492,7 +4492,7 @@ SYSCALL_DEFINE0(sched_yield)
 
 int __sched _cond_resched(void)
 {
-	if (should_resched()) {
+	if (should_resched(0)) {
 		preempt_schedule_common();
 		return 1;
 	}

@@ -4510,7 +4510,7 @@ EXPORT_SYMBOL(_cond_resched);
  */
 int __cond_resched_lock(spinlock_t *lock)
 {
-	int resched = should_resched();
+	int resched = should_resched(PREEMPT_LOCK_OFFSET);
 	int ret = 0;
 
 	lockdep_assert_held(lock);

@@ -4532,7 +4532,7 @@ int __sched __cond_resched_softirq(void)
 {
 	BUG_ON(!in_softirq());
 
-	if (should_resched()) {
+	if (should_resched(SOFTIRQ_DISABLE_OFFSET)) {
 		local_bh_enable();
 		preempt_schedule_common();
 		local_bh_disable();

David Vrabel

Applied

2015-07-20 13:41 UTC #13 lore raw

In reply to Konstantin Khlebnikov

Quoted 3 lines · Konstantin Khlebnikov

This code is used only when CONFIG_PREEMPT=n and only in non-atomic context:
xen_in_preemptible_hcall is set only in privcmd_ioctl_hypercall(). Thus preempt_count is zero and should_resched() is equal to need_resched().

Applied to for-linus-4.3, thanks.

David

David Vrabel

Applied

2015-07-20 13:41 UTC #14 lore raw

In reply to Konstantin Khlebnikov

Quoted 3 lines · Konstantin Khlebnikov

This code is used only when CONFIG_PREEMPT=n and only in non-atomic context:
xen_in_preemptible_hcall is set only in privcmd_ioctl_hypercall(). Thus preempt_count is zero and should_resched() is equal to need_resched().

Applied to for-linus-4.3, thanks.

David

tip-bot for Konstantin Khlebnikov

merge bot

2015-08-03 17:08 UTC #15 lore raw

[tip:sched/core] sched/preempt, xen: Use need_resched() instead of should_resched()

In reply to Konstantin Khlebnikov

sched/preempt, xen: Use need_resched() instead of should_resched()

Commit-ID:  0fa2f5cb2b0ecd8d56baa51f35f09aab234eb0bf
Gitweb:     http://git.kernel.org/tip/0fa2f5cb2b0ecd8d56baa51f35f09aab234eb0bf
Author:     Konstantin Khlebnikov <[email protected]>
AuthorDate: Wed, 15 Jul 2015 12:52:01 +0300
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 3 Aug 2015 12:21:23 +0200

sched/preempt, xen: Use need_resched() instead of should_resched()

Signed-off-by Konstantin Khlebnikov <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Cc Alexander Graf <[email protected]>

Cc Boris Ostrovsky <[email protected]>

Cc David Vrabel <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Mike Galbraith <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/20150715095201.12246.49283.stgit@buzz open here

Signed-off-by Ingo Molnar <[email protected]>

---
 drivers/xen/preempt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/xen/preempt.c +1 -1

index a1800c1..08cb419 100644
@@ -31,7 +31,7 @@ EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
 asmlinkage __visible void xen_maybe_preempt_hcall(void)
 {
 	if (unlikely(__this_cpu_read(xen_in_preemptible_hcall)
-		     && should_resched())) {
+		     && need_resched())) {
 		/*
 		 * Clear flag as we may be rescheduled on a different
 		 * cpu.