All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
@ 2025-05-09 11:36 Sebastian Andrzej Siewior
  2025-05-13 13:34 ` Miroslav Benes
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-05-09 11:36 UTC (permalink / raw)
  To: linux-kernel, live-patching
  Cc: Peter Zijlstra, Josh Poimboeuf, mingo, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, jpoimboe, jikos, mbenes, pmladek, joe.lawrence,
	Thomas Gleixner

From: Peter Zijlstra <peterz@infradead.org>

With the goal of deprecating / removing VOLUNTARY preempt, live-patch
needs to stop relying on cond_resched() to make forward progress.

Instead, rely on schedule() with TASK_FREEZABLE set. Just like
live-patching, the freezer needs to be able to stop tasks in a safe /
known state.

Compile tested only.

[bigeasy: use likely() in __klp_sched_try_switch() and update comments]

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: https://lore.kernel.org/all/20250324134909.GA14718@noisy.programming.kicks-ass.net/
  - Updated comments in __klp_sched_try_switch()
  - Replaced unlikely with likely in __klp_sched_try_switch()
  - Dropped RFC

 include/linux/livepatch_sched.h | 14 ++++-----
 include/linux/sched.h           |  6 ----
 kernel/livepatch/transition.c   | 52 ++++++++++-----------------------
 kernel/sched/core.c             | 50 +++++--------------------------
 4 files changed, 29 insertions(+), 93 deletions(-)

diff --git a/include/linux/livepatch_sched.h b/include/linux/livepatch_sched.h
index 013794fb5da08..065c185f27638 100644
--- a/include/linux/livepatch_sched.h
+++ b/include/linux/livepatch_sched.h
@@ -3,27 +3,23 @@
 #define _LINUX_LIVEPATCH_SCHED_H_
 
 #include <linux/jump_label.h>
-#include <linux/static_call_types.h>
+#include <linux/sched.h>
 
 #ifdef CONFIG_LIVEPATCH
 
 void __klp_sched_try_switch(void);
 
-#if !defined(CONFIG_PREEMPT_DYNAMIC) || !defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
-
 DECLARE_STATIC_KEY_FALSE(klp_sched_try_switch_key);
 
-static __always_inline void klp_sched_try_switch(void)
+static __always_inline void klp_sched_try_switch(struct task_struct *curr)
 {
-	if (static_branch_unlikely(&klp_sched_try_switch_key))
+	if (static_branch_unlikely(&klp_sched_try_switch_key) &&
+	    READ_ONCE(curr->__state) & TASK_FREEZABLE)
 		__klp_sched_try_switch();
 }
 
-#endif /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
-
 #else /* !CONFIG_LIVEPATCH */
-static inline void klp_sched_try_switch(void) {}
-static inline void __klp_sched_try_switch(void) {}
+static inline void klp_sched_try_switch(struct task_struct *curr) {}
 #endif /* CONFIG_LIVEPATCH */
 
 #endif /* _LINUX_LIVEPATCH_SCHED_H_ */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f96ac19828934..b98195991031c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -44,7 +44,6 @@
 #include <linux/seqlock_types.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
-#include <linux/livepatch_sched.h>
 #include <linux/uidgid_types.h>
 #include <linux/tracepoint-defs.h>
 #include <asm/kmap_size.h>
@@ -2089,9 +2088,6 @@ extern int __cond_resched(void);
 
 #if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 
-void sched_dynamic_klp_enable(void);
-void sched_dynamic_klp_disable(void);
-
 DECLARE_STATIC_CALL(cond_resched, __cond_resched);
 
 static __always_inline int _cond_resched(void)
@@ -2112,7 +2108,6 @@ static __always_inline int _cond_resched(void)
 
 static inline int _cond_resched(void)
 {
-	klp_sched_try_switch();
 	return __cond_resched();
 }
 
@@ -2122,7 +2117,6 @@ static inline int _cond_resched(void)
 
 static inline int _cond_resched(void)
 {
-	klp_sched_try_switch();
 	return 0;
 }
 
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index ba069459c1017..25b9372a4b66f 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -29,22 +29,13 @@ static unsigned int klp_signals_cnt;
 
 /*
  * When a livepatch is in progress, enable klp stack checking in
- * cond_resched().  This helps CPU-bound kthreads get patched.
+ * schedule().  This helps CPU-bound kthreads get patched.
  */
-#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
-
-#define klp_cond_resched_enable() sched_dynamic_klp_enable()
-#define klp_cond_resched_disable() sched_dynamic_klp_disable()
-
-#else /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
 
 DEFINE_STATIC_KEY_FALSE(klp_sched_try_switch_key);
-EXPORT_SYMBOL(klp_sched_try_switch_key);
 
-#define klp_cond_resched_enable() static_branch_enable(&klp_sched_try_switch_key)
-#define klp_cond_resched_disable() static_branch_disable(&klp_sched_try_switch_key)
-
-#endif /* CONFIG_PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
+#define klp_resched_enable() static_branch_enable(&klp_sched_try_switch_key)
+#define klp_resched_disable() static_branch_disable(&klp_sched_try_switch_key)
 
 /*
  * This work can be performed periodically to finish patching or unpatching any
@@ -365,27 +356,20 @@ static bool klp_try_switch_task(struct task_struct *task)
 
 void __klp_sched_try_switch(void)
 {
+	/*
+	 * This function is called from __schedule() while a context switch is
+	 * about to happen. Preemption is already disabled and klp_mutex
+	 * can't be acquired.
+	 * Disabled preemption is used to prevent racing with other callers of
+	 * klp_try_switch_task(). Thanks to task_call_func() they won't be
+	 * able to switch to this task while it's running.
+	 */
+	lockdep_assert_preemption_disabled();
+
+	/* Make sure current didn't get patched */
 	if (likely(!klp_patch_pending(current)))
 		return;
 
-	/*
-	 * This function is called from cond_resched() which is called in many
-	 * places throughout the kernel.  Using the klp_mutex here might
-	 * deadlock.
-	 *
-	 * Instead, disable preemption to prevent racing with other callers of
-	 * klp_try_switch_task().  Thanks to task_call_func() they won't be
-	 * able to switch this task while it's running.
-	 */
-	preempt_disable();
-
-	/*
-	 * Make sure current didn't get patched between the above check and
-	 * preempt_disable().
-	 */
-	if (unlikely(!klp_patch_pending(current)))
-		goto out;
-
 	/*
 	 * Enforce the order of the TIF_PATCH_PENDING read above and the
 	 * klp_target_state read in klp_try_switch_task().  The corresponding
@@ -395,11 +379,7 @@ void __klp_sched_try_switch(void)
 	smp_rmb();
 
 	klp_try_switch_task(current);
-
-out:
-	preempt_enable();
 }
-EXPORT_SYMBOL(__klp_sched_try_switch);
 
 /*
  * Sends a fake signal to all non-kthread tasks with TIF_PATCH_PENDING set.
@@ -508,7 +488,7 @@ void klp_try_complete_transition(void)
 	}
 
 	/* Done!  Now cleanup the data structures. */
-	klp_cond_resched_disable();
+	klp_resched_disable();
 	patch = klp_transition_patch;
 	klp_complete_transition();
 
@@ -560,7 +540,7 @@ void klp_start_transition(void)
 			set_tsk_thread_flag(task, TIF_PATCH_PENDING);
 	}
 
-	klp_cond_resched_enable();
+	klp_resched_enable();
 
 	klp_signals_cnt = 0;
 }
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c81cf642dba05..2a973d0e414a3 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -66,6 +66,7 @@
 #include <linux/vtime.h>
 #include <linux/wait_api.h>
 #include <linux/workqueue_api.h>
+#include <linux/livepatch_sched.h>
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 # ifdef CONFIG_GENERIC_ENTRY
@@ -6668,6 +6669,8 @@ static void __sched notrace __schedule(int sched_mode)
 	if (sched_feat(HRTICK) || sched_feat(HRTICK_DL))
 		hrtick_clear(rq);
 
+	klp_sched_try_switch(prev);
+
 	local_irq_disable();
 	rcu_note_context_switch(preempt);
 
@@ -7328,7 +7331,6 @@ EXPORT_STATIC_CALL_TRAMP(might_resched);
 static DEFINE_STATIC_KEY_FALSE(sk_dynamic_cond_resched);
 int __sched dynamic_cond_resched(void)
 {
-	klp_sched_try_switch();
 	if (!static_branch_unlikely(&sk_dynamic_cond_resched))
 		return 0;
 	return __cond_resched();
@@ -7500,7 +7502,6 @@ int sched_dynamic_mode(const char *str)
 #endif
 
 static DEFINE_MUTEX(sched_dynamic_mutex);
-static bool klp_override;
 
 static void __sched_dynamic_update(int mode)
 {
@@ -7508,8 +7509,7 @@ static void __sched_dynamic_update(int mode)
 	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
 	 * the ZERO state, which is invalid.
 	 */
-	if (!klp_override)
-		preempt_dynamic_enable(cond_resched);
+	preempt_dynamic_enable(cond_resched);
 	preempt_dynamic_enable(might_resched);
 	preempt_dynamic_enable(preempt_schedule);
 	preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7518,8 +7518,7 @@ static void __sched_dynamic_update(int mode)
 
 	switch (mode) {
 	case preempt_dynamic_none:
-		if (!klp_override)
-			preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_disable(preempt_schedule);
 		preempt_dynamic_disable(preempt_schedule_notrace);
@@ -7530,8 +7529,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_voluntary:
-		if (!klp_override)
-			preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(cond_resched);
 		preempt_dynamic_enable(might_resched);
 		preempt_dynamic_disable(preempt_schedule);
 		preempt_dynamic_disable(preempt_schedule_notrace);
@@ -7542,8 +7540,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_full:
-		if (!klp_override)
-			preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_enable(preempt_schedule);
 		preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7554,8 +7551,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_lazy:
-		if (!klp_override)
-			preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_enable(preempt_schedule);
 		preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7576,36 +7572,6 @@ void sched_dynamic_update(int mode)
 	mutex_unlock(&sched_dynamic_mutex);
 }
 
-#ifdef CONFIG_HAVE_PREEMPT_DYNAMIC_CALL
-
-static int klp_cond_resched(void)
-{
-	__klp_sched_try_switch();
-	return __cond_resched();
-}
-
-void sched_dynamic_klp_enable(void)
-{
-	mutex_lock(&sched_dynamic_mutex);
-
-	klp_override = true;
-	static_call_update(cond_resched, klp_cond_resched);
-
-	mutex_unlock(&sched_dynamic_mutex);
-}
-
-void sched_dynamic_klp_disable(void)
-{
-	mutex_lock(&sched_dynamic_mutex);
-
-	klp_override = false;
-	__sched_dynamic_update(preempt_dynamic_mode);
-
-	mutex_unlock(&sched_dynamic_mutex);
-}
-
-#endif /* CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
-
 static int __init setup_preempt_mode(char *str)
 {
 	int mode = sched_dynamic_mode(str);
-- 
2.49.0


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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-09 11:36 [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching Sebastian Andrzej Siewior
@ 2025-05-13 13:34 ` Miroslav Benes
  2025-05-13 14:03   ` Peter Zijlstra
  2025-05-14  9:44   ` Petr Mladek
  2025-05-13 21:40 ` Josh Poimboeuf
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Miroslav Benes @ 2025-05-13 13:34 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, live-patching, Peter Zijlstra, Josh Poimboeuf,
	mingo, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, jpoimboe, jikos, pmladek,
	joe.lawrence, Thomas Gleixner

Hi,

thanks for the updated version.

On Fri, 9 May 2025, Sebastian Andrzej Siewior wrote:

> From: Peter Zijlstra <peterz@infradead.org>
> 
> With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> needs to stop relying on cond_resched() to make forward progress.
> 
> Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> live-patching, the freezer needs to be able to stop tasks in a safe /
> known state.
> 
> Compile tested only.

livepatch selftests pass and I also ran some more.
 
> [bigeasy: use likely() in __klp_sched_try_switch() and update comments]
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Acked-by: Miroslav Benes <mbenes@suse.cz>

A nit below if there is an another version, otherwise Petr might fix it 
when merging.

> @@ -365,27 +356,20 @@ static bool klp_try_switch_task(struct task_struct *task)
>  
>  void __klp_sched_try_switch(void)
>  {
> +	/*
> +	 * This function is called from __schedule() while a context switch is
> +	 * about to happen. Preemption is already disabled and klp_mutex
> +	 * can't be acquired.
> +	 * Disabled preemption is used to prevent racing with other callers of
> +	 * klp_try_switch_task(). Thanks to task_call_func() they won't be
> +	 * able to switch to this task while it's running.
> +	 */
> +	lockdep_assert_preemption_disabled();
> +
> +	/* Make sure current didn't get patched */
>       if (likely(!klp_patch_pending(current)))
>                return;

This last comment is not precise. If !klp_patch_pending(), there is 
nothing to do. Fast way out. So if it was up to me, I would remove the 
line all together.

Miroslav

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-13 13:34 ` Miroslav Benes
@ 2025-05-13 14:03   ` Peter Zijlstra
  2025-05-13 14:05     ` Miroslav Benes
  2025-05-14  9:44   ` Petr Mladek
  1 sibling, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2025-05-13 14:03 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Sebastian Andrzej Siewior, linux-kernel, live-patching,
	Josh Poimboeuf, mingo, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, jpoimboe,
	jikos, pmladek, joe.lawrence, Thomas Gleixner

On Tue, May 13, 2025 at 03:34:50PM +0200, Miroslav Benes wrote:
> Hi,
> 
> thanks for the updated version.
> 
> On Fri, 9 May 2025, Sebastian Andrzej Siewior wrote:
> 
> > From: Peter Zijlstra <peterz@infradead.org>
> > 
> > With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> > needs to stop relying on cond_resched() to make forward progress.
> > 
> > Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> > live-patching, the freezer needs to be able to stop tasks in a safe /
> > known state.
> > 
> > Compile tested only.
> 
> livepatch selftests pass and I also ran some more.
>  
> > [bigeasy: use likely() in __klp_sched_try_switch() and update comments]
> > 
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> 
> A nit below if there is an another version, otherwise Petr might fix it 
> when merging.

Petr or Peter?

That is, who are we expecting to merge this :-)

Anyway, I've zapped the line in my copy.

> > @@ -365,27 +356,20 @@ static bool klp_try_switch_task(struct task_struct *task)
> >  
> >  void __klp_sched_try_switch(void)
> >  {
> > +	/*
> > +	 * This function is called from __schedule() while a context switch is
> > +	 * about to happen. Preemption is already disabled and klp_mutex
> > +	 * can't be acquired.
> > +	 * Disabled preemption is used to prevent racing with other callers of
> > +	 * klp_try_switch_task(). Thanks to task_call_func() they won't be
> > +	 * able to switch to this task while it's running.
> > +	 */
> > +	lockdep_assert_preemption_disabled();
> > +
> > +	/* Make sure current didn't get patched */
> >       if (likely(!klp_patch_pending(current)))
> >                return;
> 
> This last comment is not precise. If !klp_patch_pending(), there is 
> nothing to do. Fast way out. So if it was up to me, I would remove the 
> line all together.
> 
> Miroslav

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-13 14:03   ` Peter Zijlstra
@ 2025-05-13 14:05     ` Miroslav Benes
  2025-05-14  9:51       ` Petr Mladek
  0 siblings, 1 reply; 11+ messages in thread
From: Miroslav Benes @ 2025-05-13 14:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Sebastian Andrzej Siewior, linux-kernel, live-patching,
	Josh Poimboeuf, mingo, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, jpoimboe,
	jikos, pmladek, joe.lawrence, Thomas Gleixner

On Tue, 13 May 2025, Peter Zijlstra wrote:

> On Tue, May 13, 2025 at 03:34:50PM +0200, Miroslav Benes wrote:
> > Hi,
> > 
> > thanks for the updated version.
> > 
> > On Fri, 9 May 2025, Sebastian Andrzej Siewior wrote:
> > 
> > > From: Peter Zijlstra <peterz@infradead.org>
> > > 
> > > With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> > > needs to stop relying on cond_resched() to make forward progress.
> > > 
> > > Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> > > live-patching, the freezer needs to be able to stop tasks in a safe /
> > > known state.
> > > 
> > > Compile tested only.
> > 
> > livepatch selftests pass and I also ran some more.
> >  
> > > [bigeasy: use likely() in __klp_sched_try_switch() and update comments]
> > > 
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > 
> > Acked-by: Miroslav Benes <mbenes@suse.cz>
> > 
> > A nit below if there is an another version, otherwise Petr might fix it 
> > when merging.
> 
> Petr or Peter?
> 
> That is, who are we expecting to merge this :-)

Petr Mladek if it goes through the live patching tree, you if tip. Feel 
free to pick it up :).

> Anyway, I've zapped the line in my copy.

Thanks!

Miroslav

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-09 11:36 [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching Sebastian Andrzej Siewior
  2025-05-13 13:34 ` Miroslav Benes
@ 2025-05-13 21:40 ` Josh Poimboeuf
  2025-05-14  9:59 ` Thomas Gleixner
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Josh Poimboeuf @ 2025-05-13 21:40 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, live-patching, Peter Zijlstra, mingo, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, jikos, mbenes, pmladek, joe.lawrence, Thomas Gleixner

On Fri, May 09, 2025 at 01:36:59PM +0200, Sebastian Andrzej Siewior wrote:
> From: Peter Zijlstra <peterz@infradead.org>
> 
> With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> needs to stop relying on cond_resched() to make forward progress.
> 
> Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> live-patching, the freezer needs to be able to stop tasks in a safe /
> known state.
> 
> Compile tested only.
> 
> [bigeasy: use likely() in __klp_sched_try_switch() and update comments]
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>

-- 
Josh

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-13 13:34 ` Miroslav Benes
  2025-05-13 14:03   ` Peter Zijlstra
@ 2025-05-14  9:44   ` Petr Mladek
  1 sibling, 0 replies; 11+ messages in thread
From: Petr Mladek @ 2025-05-14  9:44 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Sebastian Andrzej Siewior, linux-kernel, live-patching,
	Peter Zijlstra, Josh Poimboeuf, mingo, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, jpoimboe, jikos, joe.lawrence, Thomas Gleixner

On Tue 2025-05-13 15:34:50, Miroslav Benes wrote:
> On Fri, 9 May 2025, Sebastian Andrzej Siewior wrote:
> 
> > From: Peter Zijlstra <peterz@infradead.org>
> > 
> > With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> > needs to stop relying on cond_resched() to make forward progress.
> > 
> > Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> > live-patching, the freezer needs to be able to stop tasks in a safe /
> > known state.
> > 
> > @@ -365,27 +356,20 @@ static bool klp_try_switch_task(struct task_struct *task)
> >  
> >  void __klp_sched_try_switch(void)
> >  {
> > +	/*
> > +	 * This function is called from __schedule() while a context switch is
> > +	 * about to happen. Preemption is already disabled and klp_mutex
> > +	 * can't be acquired.
> > +	 * Disabled preemption is used to prevent racing with other callers of
> > +	 * klp_try_switch_task(). Thanks to task_call_func() they won't be
> > +	 * able to switch to this task while it's running.
> > +	 */
> > +	lockdep_assert_preemption_disabled();
> > +
> > +	/* Make sure current didn't get patched */
> >       if (likely(!klp_patch_pending(current)))
> >                return;
> 
> This last comment is not precise. If !klp_patch_pending(), there is 
> nothing to do. Fast way out. So if it was up to me, I would remove the 
> line all together.

I think that this is not just a speedup. Right below is a read memory
barrier. It says that we need to read TIF_PATCH_PENDING here to
make sure that klp_target_state is correct after the barrier.

Anyway, I agree that the comment is confusing and might be removed.
The comment above the memory barrier might/should be enough.

Best Regards,
Petr

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-13 14:05     ` Miroslav Benes
@ 2025-05-14  9:51       ` Petr Mladek
  2025-05-14 11:17         ` Peter Zijlstra
  0 siblings, 1 reply; 11+ messages in thread
From: Petr Mladek @ 2025-05-14  9:51 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Peter Zijlstra, Sebastian Andrzej Siewior, linux-kernel,
	live-patching, Josh Poimboeuf, mingo, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, jpoimboe,
	jikos, joe.lawrence, Thomas Gleixner

On Tue 2025-05-13 16:05:51, Miroslav Benes wrote:
> On Tue, 13 May 2025, Peter Zijlstra wrote:
> 
> > On Tue, May 13, 2025 at 03:34:50PM +0200, Miroslav Benes wrote:
> > > Hi,
> > > 
> > > thanks for the updated version.
> > > 
> > > On Fri, 9 May 2025, Sebastian Andrzej Siewior wrote:
> > > 
> > > > From: Peter Zijlstra <peterz@infradead.org>
> > > > 
> > > > With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> > > > needs to stop relying on cond_resched() to make forward progress.
> > > > 
> > > > Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> > > > live-patching, the freezer needs to be able to stop tasks in a safe /
> > > > known state.
> > > > 
> > > > Compile tested only.
> > > 
> > > livepatch selftests pass and I also ran some more.
> > >  
> > > > [bigeasy: use likely() in __klp_sched_try_switch() and update comments]
> > > > 
> > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > 
> > > Acked-by: Miroslav Benes <mbenes@suse.cz>
> > > 
> > > A nit below if there is an another version, otherwise Petr might fix it 
> > > when merging.
> > 
> > Petr or Peter?
> > 
> > That is, who are we expecting to merge this :-)
> 
> Petr Mladek if it goes through the live patching tree, you if tip. Feel 
> free to pick it up :).

IMHO, it might be easier when it goes via tip. Peter, feel free to
take it.

The patch does not create any conflict with the klp tree.
But I guess that there might be some dependent patches in tip...

That said, I could take it via the livepatch tree if Peter preferred
it from some reasons.

Anyway, the patch looks good and passes the tests. Feel free to use:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-09 11:36 [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching Sebastian Andrzej Siewior
  2025-05-13 13:34 ` Miroslav Benes
  2025-05-13 21:40 ` Josh Poimboeuf
@ 2025-05-14  9:59 ` Thomas Gleixner
  2025-05-14 11:17 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
  2025-05-14 11:26 ` tip-bot2 for Peter Zijlstra
  4 siblings, 0 replies; 11+ messages in thread
From: Thomas Gleixner @ 2025-05-14  9:59 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, linux-kernel, live-patching
  Cc: Peter Zijlstra, Josh Poimboeuf, mingo, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, jpoimboe, jikos, mbenes, pmladek, joe.lawrence

On Fri, May 09 2025 at 13:36, Sebastian Andrzej Siewior wrote:
> From: Peter Zijlstra <peterz@infradead.org>
>
> With the goal of deprecating / removing VOLUNTARY preempt, live-patch
> needs to stop relying on cond_resched() to make forward progress.
>
> Instead, rely on schedule() with TASK_FREEZABLE set. Just like
> live-patching, the freezer needs to be able to stop tasks in a safe /
> known state.
>
> Compile tested only.
>
> [bigeasy: use likely() in __klp_sched_try_switch() and update comments]
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

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

* [tip: sched/core] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-09 11:36 [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching Sebastian Andrzej Siewior
                   ` (2 preceding siblings ...)
  2025-05-14  9:59 ` Thomas Gleixner
@ 2025-05-14 11:17 ` tip-bot2 for Peter Zijlstra
  2025-05-14 11:26 ` tip-bot2 for Peter Zijlstra
  4 siblings, 0 replies; 11+ messages in thread
From: tip-bot2 for Peter Zijlstra @ 2025-05-14 11:17 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Peter Zijlstra (Intel), Sebastian Andrzej Siewior,
	Thomas Gleixner, Miroslav Benes, Josh Poimboeuf, x86,
	linux-kernel

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     fe2ece0700df81ee1fc91d51d80a1a3f37e59806
Gitweb:        https://git.kernel.org/tip/fe2ece0700df81ee1fc91d51d80a1a3f37e59806
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Fri, 09 May 2025 13:36:59 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 14 May 2025 13:09:50 +02:00

sched,livepatch: Untangle cond_resched() and live-patching

With the goal of deprecating / removing VOLUNTARY preempt, live-patch
needs to stop relying on cond_resched() to make forward progress.

Instead, rely on schedule() with TASK_FREEZABLE set. Just like
live-patching, the freezer needs to be able to stop tasks in a safe /
known state.

[bigeasy: use likely() in __klp_sched_try_switch() and update comments]

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
Link: https://lore.kernel.org/r/20250509113659.wkP_HJ5z@linutronix.de
---
 include/linux/livepatch_sched.h | 14 +++------
 include/linux/sched.h           |  6 +----
 kernel/livepatch/transition.c   | 49 ++++++++-----------------------
 kernel/sched/core.c             | 50 +++++---------------------------
 4 files changed, 27 insertions(+), 92 deletions(-)

diff --git a/include/linux/livepatch_sched.h b/include/linux/livepatch_sched.h
index 013794f..065c185 100644
--- a/include/linux/livepatch_sched.h
+++ b/include/linux/livepatch_sched.h
@@ -3,27 +3,23 @@
 #define _LINUX_LIVEPATCH_SCHED_H_
 
 #include <linux/jump_label.h>
-#include <linux/static_call_types.h>
+#include <linux/sched.h>
 
 #ifdef CONFIG_LIVEPATCH
 
 void __klp_sched_try_switch(void);
 
-#if !defined(CONFIG_PREEMPT_DYNAMIC) || !defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
-
 DECLARE_STATIC_KEY_FALSE(klp_sched_try_switch_key);
 
-static __always_inline void klp_sched_try_switch(void)
+static __always_inline void klp_sched_try_switch(struct task_struct *curr)
 {
-	if (static_branch_unlikely(&klp_sched_try_switch_key))
+	if (static_branch_unlikely(&klp_sched_try_switch_key) &&
+	    READ_ONCE(curr->__state) & TASK_FREEZABLE)
 		__klp_sched_try_switch();
 }
 
-#endif /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
-
 #else /* !CONFIG_LIVEPATCH */
-static inline void klp_sched_try_switch(void) {}
-static inline void __klp_sched_try_switch(void) {}
+static inline void klp_sched_try_switch(struct task_struct *curr) {}
 #endif /* CONFIG_LIVEPATCH */
 
 #endif /* _LINUX_LIVEPATCH_SCHED_H_ */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f96ac19..b981959 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -44,7 +44,6 @@
 #include <linux/seqlock_types.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
-#include <linux/livepatch_sched.h>
 #include <linux/uidgid_types.h>
 #include <linux/tracepoint-defs.h>
 #include <asm/kmap_size.h>
@@ -2089,9 +2088,6 @@ extern int __cond_resched(void);
 
 #if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 
-void sched_dynamic_klp_enable(void);
-void sched_dynamic_klp_disable(void);
-
 DECLARE_STATIC_CALL(cond_resched, __cond_resched);
 
 static __always_inline int _cond_resched(void)
@@ -2112,7 +2108,6 @@ static __always_inline int _cond_resched(void)
 
 static inline int _cond_resched(void)
 {
-	klp_sched_try_switch();
 	return __cond_resched();
 }
 
@@ -2122,7 +2117,6 @@ static inline int _cond_resched(void)
 
 static inline int _cond_resched(void)
 {
-	klp_sched_try_switch();
 	return 0;
 }
 
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index ba06945..2351a19 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -29,22 +29,13 @@ static unsigned int klp_signals_cnt;
 
 /*
  * When a livepatch is in progress, enable klp stack checking in
- * cond_resched().  This helps CPU-bound kthreads get patched.
+ * schedule().  This helps CPU-bound kthreads get patched.
  */
-#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
-
-#define klp_cond_resched_enable() sched_dynamic_klp_enable()
-#define klp_cond_resched_disable() sched_dynamic_klp_disable()
-
-#else /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
 
 DEFINE_STATIC_KEY_FALSE(klp_sched_try_switch_key);
-EXPORT_SYMBOL(klp_sched_try_switch_key);
 
-#define klp_cond_resched_enable() static_branch_enable(&klp_sched_try_switch_key)
-#define klp_cond_resched_disable() static_branch_disable(&klp_sched_try_switch_key)
-
-#endif /* CONFIG_PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
+#define klp_resched_enable() static_branch_enable(&klp_sched_try_switch_key)
+#define klp_resched_disable() static_branch_disable(&klp_sched_try_switch_key)
 
 /*
  * This work can be performed periodically to finish patching or unpatching any
@@ -365,26 +356,18 @@ static bool klp_try_switch_task(struct task_struct *task)
 
 void __klp_sched_try_switch(void)
 {
-	if (likely(!klp_patch_pending(current)))
-		return;
-
 	/*
-	 * This function is called from cond_resched() which is called in many
-	 * places throughout the kernel.  Using the klp_mutex here might
-	 * deadlock.
-	 *
-	 * Instead, disable preemption to prevent racing with other callers of
-	 * klp_try_switch_task().  Thanks to task_call_func() they won't be
-	 * able to switch this task while it's running.
+	 * This function is called from __schedule() while a context switch is
+	 * about to happen. Preemption is already disabled and klp_mutex
+	 * can't be acquired.
+	 * Disabled preemption is used to prevent racing with other callers of
+	 * klp_try_switch_task(). Thanks to task_call_func() they won't be
+	 * able to switch to this task while it's running.
 	 */
-	preempt_disable();
+	lockdep_assert_preemption_disabled();
 
-	/*
-	 * Make sure current didn't get patched between the above check and
-	 * preempt_disable().
-	 */
-	if (unlikely(!klp_patch_pending(current)))
-		goto out;
+	if (likely(!klp_patch_pending(current)))
+		return;
 
 	/*
 	 * Enforce the order of the TIF_PATCH_PENDING read above and the
@@ -395,11 +378,7 @@ void __klp_sched_try_switch(void)
 	smp_rmb();
 
 	klp_try_switch_task(current);
-
-out:
-	preempt_enable();
 }
-EXPORT_SYMBOL(__klp_sched_try_switch);
 
 /*
  * Sends a fake signal to all non-kthread tasks with TIF_PATCH_PENDING set.
@@ -508,7 +487,7 @@ void klp_try_complete_transition(void)
 	}
 
 	/* Done!  Now cleanup the data structures. */
-	klp_cond_resched_disable();
+	klp_resched_disable();
 	patch = klp_transition_patch;
 	klp_complete_transition();
 
@@ -560,7 +539,7 @@ void klp_start_transition(void)
 			set_tsk_thread_flag(task, TIF_PATCH_PENDING);
 	}
 
-	klp_cond_resched_enable();
+	klp_resched_enable();
 
 	klp_signals_cnt = 0;
 }
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a3507ed..bece0ba 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -66,6 +66,7 @@
 #include <linux/vtime.h>
 #include <linux/wait_api.h>
 #include <linux/workqueue_api.h>
+#include <linux/livepatch_sched.h>
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 # ifdef CONFIG_GENERIC_ENTRY
@@ -6676,6 +6677,8 @@ static void __sched notrace __schedule(int sched_mode)
 	if (sched_feat(HRTICK) || sched_feat(HRTICK_DL))
 		hrtick_clear(rq);
 
+	klp_sched_try_switch(prev);
+
 	local_irq_disable();
 	rcu_note_context_switch(preempt);
 
@@ -7336,7 +7339,6 @@ EXPORT_STATIC_CALL_TRAMP(might_resched);
 static DEFINE_STATIC_KEY_FALSE(sk_dynamic_cond_resched);
 int __sched dynamic_cond_resched(void)
 {
-	klp_sched_try_switch();
 	if (!static_branch_unlikely(&sk_dynamic_cond_resched))
 		return 0;
 	return __cond_resched();
@@ -7508,7 +7510,6 @@ int sched_dynamic_mode(const char *str)
 #endif
 
 static DEFINE_MUTEX(sched_dynamic_mutex);
-static bool klp_override;
 
 static void __sched_dynamic_update(int mode)
 {
@@ -7516,8 +7517,7 @@ static void __sched_dynamic_update(int mode)
 	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
 	 * the ZERO state, which is invalid.
 	 */
-	if (!klp_override)
-		preempt_dynamic_enable(cond_resched);
+	preempt_dynamic_enable(cond_resched);
 	preempt_dynamic_enable(might_resched);
 	preempt_dynamic_enable(preempt_schedule);
 	preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7526,8 +7526,7 @@ static void __sched_dynamic_update(int mode)
 
 	switch (mode) {
 	case preempt_dynamic_none:
-		if (!klp_override)
-			preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_disable(preempt_schedule);
 		preempt_dynamic_disable(preempt_schedule_notrace);
@@ -7538,8 +7537,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_voluntary:
-		if (!klp_override)
-			preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(cond_resched);
 		preempt_dynamic_enable(might_resched);
 		preempt_dynamic_disable(preempt_schedule);
 		preempt_dynamic_disable(preempt_schedule_notrace);
@@ -7550,8 +7548,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_full:
-		if (!klp_override)
-			preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_enable(preempt_schedule);
 		preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7562,8 +7559,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_lazy:
-		if (!klp_override)
-			preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_enable(preempt_schedule);
 		preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7584,36 +7580,6 @@ void sched_dynamic_update(int mode)
 	mutex_unlock(&sched_dynamic_mutex);
 }
 
-#ifdef CONFIG_HAVE_PREEMPT_DYNAMIC_CALL
-
-static int klp_cond_resched(void)
-{
-	__klp_sched_try_switch();
-	return __cond_resched();
-}
-
-void sched_dynamic_klp_enable(void)
-{
-	mutex_lock(&sched_dynamic_mutex);
-
-	klp_override = true;
-	static_call_update(cond_resched, klp_cond_resched);
-
-	mutex_unlock(&sched_dynamic_mutex);
-}
-
-void sched_dynamic_klp_disable(void)
-{
-	mutex_lock(&sched_dynamic_mutex);
-
-	klp_override = false;
-	__sched_dynamic_update(preempt_dynamic_mode);
-
-	mutex_unlock(&sched_dynamic_mutex);
-}
-
-#endif /* CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
-
 static int __init setup_preempt_mode(char *str)
 {
 	int mode = sched_dynamic_mode(str);

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

* Re: [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-14  9:51       ` Petr Mladek
@ 2025-05-14 11:17         ` Peter Zijlstra
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Zijlstra @ 2025-05-14 11:17 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Miroslav Benes, Sebastian Andrzej Siewior, linux-kernel,
	live-patching, Josh Poimboeuf, mingo, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, jpoimboe,
	jikos, joe.lawrence, Thomas Gleixner

On Wed, May 14, 2025 at 11:51:44AM +0200, Petr Mladek wrote:

> IMHO, it might be easier when it goes via tip. Peter, feel free to
> take it.

Done!

> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Tested-by: Petr Mladek <pmladek@suse.com>

For some reason b4 didn't pick up these tags, added them manually and
force pushed it again.

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

* [tip: sched/core] sched,livepatch: Untangle cond_resched() and live-patching
  2025-05-09 11:36 [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching Sebastian Andrzej Siewior
                   ` (3 preceding siblings ...)
  2025-05-14 11:17 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
@ 2025-05-14 11:26 ` tip-bot2 for Peter Zijlstra
  4 siblings, 0 replies; 11+ messages in thread
From: tip-bot2 for Peter Zijlstra @ 2025-05-14 11:26 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Peter Zijlstra (Intel), Sebastian Andrzej Siewior,
	Thomas Gleixner, Petr Mladek, Miroslav Benes, Josh Poimboeuf, x86,
	linux-kernel

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     676e8cf70cb0533e1118e29898c9a9c33ae3a10f
Gitweb:        https://git.kernel.org/tip/676e8cf70cb0533e1118e29898c9a9c33ae3a10f
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Fri, 09 May 2025 13:36:59 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 14 May 2025 13:16:24 +02:00

sched,livepatch: Untangle cond_resched() and live-patching

With the goal of deprecating / removing VOLUNTARY preempt, live-patch
needs to stop relying on cond_resched() to make forward progress.

Instead, rely on schedule() with TASK_FREEZABLE set. Just like
live-patching, the freezer needs to be able to stop tasks in a safe /
known state.

[bigeasy: use likely() in __klp_sched_try_switch() and update comments]

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>
Tested-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Miroslav Benes <mbenes@suse.cz>
Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
Link: https://lore.kernel.org/r/20250509113659.wkP_HJ5z@linutronix.de
---
 include/linux/livepatch_sched.h | 14 +++------
 include/linux/sched.h           |  6 +----
 kernel/livepatch/transition.c   | 49 ++++++++-----------------------
 kernel/sched/core.c             | 50 +++++---------------------------
 4 files changed, 27 insertions(+), 92 deletions(-)

diff --git a/include/linux/livepatch_sched.h b/include/linux/livepatch_sched.h
index 013794f..065c185 100644
--- a/include/linux/livepatch_sched.h
+++ b/include/linux/livepatch_sched.h
@@ -3,27 +3,23 @@
 #define _LINUX_LIVEPATCH_SCHED_H_
 
 #include <linux/jump_label.h>
-#include <linux/static_call_types.h>
+#include <linux/sched.h>
 
 #ifdef CONFIG_LIVEPATCH
 
 void __klp_sched_try_switch(void);
 
-#if !defined(CONFIG_PREEMPT_DYNAMIC) || !defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
-
 DECLARE_STATIC_KEY_FALSE(klp_sched_try_switch_key);
 
-static __always_inline void klp_sched_try_switch(void)
+static __always_inline void klp_sched_try_switch(struct task_struct *curr)
 {
-	if (static_branch_unlikely(&klp_sched_try_switch_key))
+	if (static_branch_unlikely(&klp_sched_try_switch_key) &&
+	    READ_ONCE(curr->__state) & TASK_FREEZABLE)
 		__klp_sched_try_switch();
 }
 
-#endif /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
-
 #else /* !CONFIG_LIVEPATCH */
-static inline void klp_sched_try_switch(void) {}
-static inline void __klp_sched_try_switch(void) {}
+static inline void klp_sched_try_switch(struct task_struct *curr) {}
 #endif /* CONFIG_LIVEPATCH */
 
 #endif /* _LINUX_LIVEPATCH_SCHED_H_ */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f96ac19..b981959 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -44,7 +44,6 @@
 #include <linux/seqlock_types.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
-#include <linux/livepatch_sched.h>
 #include <linux/uidgid_types.h>
 #include <linux/tracepoint-defs.h>
 #include <asm/kmap_size.h>
@@ -2089,9 +2088,6 @@ extern int __cond_resched(void);
 
 #if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
 
-void sched_dynamic_klp_enable(void);
-void sched_dynamic_klp_disable(void);
-
 DECLARE_STATIC_CALL(cond_resched, __cond_resched);
 
 static __always_inline int _cond_resched(void)
@@ -2112,7 +2108,6 @@ static __always_inline int _cond_resched(void)
 
 static inline int _cond_resched(void)
 {
-	klp_sched_try_switch();
 	return __cond_resched();
 }
 
@@ -2122,7 +2117,6 @@ static inline int _cond_resched(void)
 
 static inline int _cond_resched(void)
 {
-	klp_sched_try_switch();
 	return 0;
 }
 
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index ba06945..2351a19 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -29,22 +29,13 @@ static unsigned int klp_signals_cnt;
 
 /*
  * When a livepatch is in progress, enable klp stack checking in
- * cond_resched().  This helps CPU-bound kthreads get patched.
+ * schedule().  This helps CPU-bound kthreads get patched.
  */
-#if defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_CALL)
-
-#define klp_cond_resched_enable() sched_dynamic_klp_enable()
-#define klp_cond_resched_disable() sched_dynamic_klp_disable()
-
-#else /* !CONFIG_PREEMPT_DYNAMIC || !CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
 
 DEFINE_STATIC_KEY_FALSE(klp_sched_try_switch_key);
-EXPORT_SYMBOL(klp_sched_try_switch_key);
 
-#define klp_cond_resched_enable() static_branch_enable(&klp_sched_try_switch_key)
-#define klp_cond_resched_disable() static_branch_disable(&klp_sched_try_switch_key)
-
-#endif /* CONFIG_PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
+#define klp_resched_enable() static_branch_enable(&klp_sched_try_switch_key)
+#define klp_resched_disable() static_branch_disable(&klp_sched_try_switch_key)
 
 /*
  * This work can be performed periodically to finish patching or unpatching any
@@ -365,26 +356,18 @@ static bool klp_try_switch_task(struct task_struct *task)
 
 void __klp_sched_try_switch(void)
 {
-	if (likely(!klp_patch_pending(current)))
-		return;
-
 	/*
-	 * This function is called from cond_resched() which is called in many
-	 * places throughout the kernel.  Using the klp_mutex here might
-	 * deadlock.
-	 *
-	 * Instead, disable preemption to prevent racing with other callers of
-	 * klp_try_switch_task().  Thanks to task_call_func() they won't be
-	 * able to switch this task while it's running.
+	 * This function is called from __schedule() while a context switch is
+	 * about to happen. Preemption is already disabled and klp_mutex
+	 * can't be acquired.
+	 * Disabled preemption is used to prevent racing with other callers of
+	 * klp_try_switch_task(). Thanks to task_call_func() they won't be
+	 * able to switch to this task while it's running.
 	 */
-	preempt_disable();
+	lockdep_assert_preemption_disabled();
 
-	/*
-	 * Make sure current didn't get patched between the above check and
-	 * preempt_disable().
-	 */
-	if (unlikely(!klp_patch_pending(current)))
-		goto out;
+	if (likely(!klp_patch_pending(current)))
+		return;
 
 	/*
 	 * Enforce the order of the TIF_PATCH_PENDING read above and the
@@ -395,11 +378,7 @@ void __klp_sched_try_switch(void)
 	smp_rmb();
 
 	klp_try_switch_task(current);
-
-out:
-	preempt_enable();
 }
-EXPORT_SYMBOL(__klp_sched_try_switch);
 
 /*
  * Sends a fake signal to all non-kthread tasks with TIF_PATCH_PENDING set.
@@ -508,7 +487,7 @@ void klp_try_complete_transition(void)
 	}
 
 	/* Done!  Now cleanup the data structures. */
-	klp_cond_resched_disable();
+	klp_resched_disable();
 	patch = klp_transition_patch;
 	klp_complete_transition();
 
@@ -560,7 +539,7 @@ void klp_start_transition(void)
 			set_tsk_thread_flag(task, TIF_PATCH_PENDING);
 	}
 
-	klp_cond_resched_enable();
+	klp_resched_enable();
 
 	klp_signals_cnt = 0;
 }
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a3507ed..bece0ba 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -66,6 +66,7 @@
 #include <linux/vtime.h>
 #include <linux/wait_api.h>
 #include <linux/workqueue_api.h>
+#include <linux/livepatch_sched.h>
 
 #ifdef CONFIG_PREEMPT_DYNAMIC
 # ifdef CONFIG_GENERIC_ENTRY
@@ -6676,6 +6677,8 @@ static void __sched notrace __schedule(int sched_mode)
 	if (sched_feat(HRTICK) || sched_feat(HRTICK_DL))
 		hrtick_clear(rq);
 
+	klp_sched_try_switch(prev);
+
 	local_irq_disable();
 	rcu_note_context_switch(preempt);
 
@@ -7336,7 +7339,6 @@ EXPORT_STATIC_CALL_TRAMP(might_resched);
 static DEFINE_STATIC_KEY_FALSE(sk_dynamic_cond_resched);
 int __sched dynamic_cond_resched(void)
 {
-	klp_sched_try_switch();
 	if (!static_branch_unlikely(&sk_dynamic_cond_resched))
 		return 0;
 	return __cond_resched();
@@ -7508,7 +7510,6 @@ int sched_dynamic_mode(const char *str)
 #endif
 
 static DEFINE_MUTEX(sched_dynamic_mutex);
-static bool klp_override;
 
 static void __sched_dynamic_update(int mode)
 {
@@ -7516,8 +7517,7 @@ static void __sched_dynamic_update(int mode)
 	 * Avoid {NONE,VOLUNTARY} -> FULL transitions from ever ending up in
 	 * the ZERO state, which is invalid.
 	 */
-	if (!klp_override)
-		preempt_dynamic_enable(cond_resched);
+	preempt_dynamic_enable(cond_resched);
 	preempt_dynamic_enable(might_resched);
 	preempt_dynamic_enable(preempt_schedule);
 	preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7526,8 +7526,7 @@ static void __sched_dynamic_update(int mode)
 
 	switch (mode) {
 	case preempt_dynamic_none:
-		if (!klp_override)
-			preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_disable(preempt_schedule);
 		preempt_dynamic_disable(preempt_schedule_notrace);
@@ -7538,8 +7537,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_voluntary:
-		if (!klp_override)
-			preempt_dynamic_enable(cond_resched);
+		preempt_dynamic_enable(cond_resched);
 		preempt_dynamic_enable(might_resched);
 		preempt_dynamic_disable(preempt_schedule);
 		preempt_dynamic_disable(preempt_schedule_notrace);
@@ -7550,8 +7548,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_full:
-		if (!klp_override)
-			preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_enable(preempt_schedule);
 		preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7562,8 +7559,7 @@ static void __sched_dynamic_update(int mode)
 		break;
 
 	case preempt_dynamic_lazy:
-		if (!klp_override)
-			preempt_dynamic_disable(cond_resched);
+		preempt_dynamic_disable(cond_resched);
 		preempt_dynamic_disable(might_resched);
 		preempt_dynamic_enable(preempt_schedule);
 		preempt_dynamic_enable(preempt_schedule_notrace);
@@ -7584,36 +7580,6 @@ void sched_dynamic_update(int mode)
 	mutex_unlock(&sched_dynamic_mutex);
 }
 
-#ifdef CONFIG_HAVE_PREEMPT_DYNAMIC_CALL
-
-static int klp_cond_resched(void)
-{
-	__klp_sched_try_switch();
-	return __cond_resched();
-}
-
-void sched_dynamic_klp_enable(void)
-{
-	mutex_lock(&sched_dynamic_mutex);
-
-	klp_override = true;
-	static_call_update(cond_resched, klp_cond_resched);
-
-	mutex_unlock(&sched_dynamic_mutex);
-}
-
-void sched_dynamic_klp_disable(void)
-{
-	mutex_lock(&sched_dynamic_mutex);
-
-	klp_override = false;
-	__sched_dynamic_update(preempt_dynamic_mode);
-
-	mutex_unlock(&sched_dynamic_mutex);
-}
-
-#endif /* CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
-
 static int __init setup_preempt_mode(char *str)
 {
 	int mode = sched_dynamic_mode(str);

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

end of thread, other threads:[~2025-05-14 11:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-09 11:36 [PATCH v2] sched,livepatch: Untangle cond_resched() and live-patching Sebastian Andrzej Siewior
2025-05-13 13:34 ` Miroslav Benes
2025-05-13 14:03   ` Peter Zijlstra
2025-05-13 14:05     ` Miroslav Benes
2025-05-14  9:51       ` Petr Mladek
2025-05-14 11:17         ` Peter Zijlstra
2025-05-14  9:44   ` Petr Mladek
2025-05-13 21:40 ` Josh Poimboeuf
2025-05-14  9:59 ` Thomas Gleixner
2025-05-14 11:17 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2025-05-14 11:26 ` tip-bot2 for Peter Zijlstra

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.