livepatch,sched: Add livepatch task switching to cond_resched()

---
 kernel/livepatch/transition.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/livepatch/transition.c +5 -1

index f1b25ec581e0..4d1f443778f7 100644
@@ -307,7 +307,11 @@ static bool klp_try_switch_task(struct task_struct *task)
 	 * functions.  If all goes well, switch the task to the target patch
 	 * state.
 	 */
-	ret = task_call_func(task, klp_check_and_switch_task, &old_name);
+	if (task == current)
+		ret = klp_check_and_switch_task(current, &old_name);
+	else
+		ret = task_call_func(task, klp_check_and_switch_task, &old_name);
+
 	switch (ret) {
 	case 0:		/* success */
 		break;

Signature · 1 line

2.39.1

Josh Poimboeuf

author maintainer

2023-02-24 16:50 UTC #3 lore raw

PATCH v3 2/3 livepatch,sched: Add livepatch task switching to cond_resched()

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lkml.kernel.org/lkml/[email protected]

There have been reports [1][2] of live patches failing to complete within a reasonable amount of time due to CPU-bound kthreads.

Fix it by patching tasks in cond_resched().

There are four different flavors of cond_resched(), depending on the kernel configuration. Hook into all of them.

A more elegant solution might be to use a preempt notifier. However, non-ORC unwinders can't unwind a preempted task reliably.

Tested-by Seth Forshee (DigitalOcean) <[email protected]>

---
 include/linux/livepatch.h       |   1 +
 include/linux/livepatch_sched.h |  29 +++++++++
 include/linux/sched.h           |  20 ++++--
 kernel/livepatch/core.c         |   1 +
 kernel/livepatch/transition.c   | 107 +++++++++++++++++++++++++++-----
 kernel/sched/core.c             |  64 ++++++++++++++++---
 6 files changed, 194 insertions(+), 28 deletions(-)
 create mode 100644 include/linux/livepatch_sched.h

include/linux/livepatch.h +1 -0

index 293e29960c6e..9b9b38e89563 100644
@@ -13,6 +13,7 @@
 #include <linux/ftrace.h>
 #include <linux/completion.h>
 #include <linux/list.h>
+#include <linux/livepatch_sched.h>
 
 #if IS_ENABLED(CONFIG_LIVEPATCH)

include/linux/livepatch_sched.h new file +29 -0

new file mode 100644
index 000000000000..013794fb5da0
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _LINUX_LIVEPATCH_SCHED_H_
+#define _LINUX_LIVEPATCH_SCHED_H_
+
+#include <linux/jump_label.h>
+#include <linux/static_call_types.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)
+{
+	if (static_branch_unlikely(&klp_sched_try_switch_key))
+		__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) {}
+#endif /* CONFIG_LIVEPATCH */
+
+#endif /* _LINUX_LIVEPATCH_SCHED_H_ */

include/linux/sched.h +15 -5

index 853d08f7562b..bd1e6f02facb 100644
@@ -36,6 +36,7 @@
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
+#include <linux/livepatch_sched.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */

@@ -2064,6 +2065,9 @@ 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)

@@ -2072,6 +2076,7 @@ static __always_inline int _cond_resched(void)
 }
 
 #elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+
 extern int dynamic_cond_resched(void);
 
 static __always_inline int _cond_resched(void)

@@ -2079,20 +2084,25 @@ static __always_inline int _cond_resched(void)
 	return dynamic_cond_resched();
 }
 
-#else
+#else /* !CONFIG_PREEMPTION */
 
 static inline int _cond_resched(void)
 {
+	klp_sched_try_switch();
 	return __cond_resched();
 }
 
-#endif /* CONFIG_PREEMPT_DYNAMIC */
+#endif /* PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
 
-#else
+#else /* CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC */
 
-static inline int _cond_resched(void) { return 0; }
+static inline int _cond_resched(void)
+{
+	klp_sched_try_switch();
+	return 0;
+}
 
-#endif /* !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC) */
+#endif /* !CONFIG_PREEMPTION || CONFIG_PREEMPT_DYNAMIC */
 
 #define cond_resched() ({			\
 	__might_resched(__FILE__, __LINE__, 0);	\

kernel/livepatch/core.c +1 -0

index 201f0c0482fb..3f79265dd6e5 100644
@@ -33,6 +33,7 @@
  *
  * - klp_ftrace_handler()
  * - klp_update_patch_state()
+ * - __klp_sched_try_switch()
  */
 DEFINE_MUTEX(klp_mutex);

kernel/livepatch/transition.c +92 ~~-15~~

171 lines not on this page · raw message

kernel/sched/core.c +56 -8

111 lines not on this page · raw message

Signature · 1 line

2.39.1

Josh Poimboeuf

author maintainer

2023-02-24 16:50 UTC #12 lore raw

PATCH v3 3/3 vhost: Fix livepatch timeouts in vhost_worker()

Link https://lkml.kernel.org/lkml/[email protected] open here

Livepatch timeouts were reported due to busy vhost_worker() kthreads. Now that cond_resched() can do livepatch task switching, use cond_resched() in vhost_worker(). That's the better way to conditionally call schedule() anyway.

Reported-by Seth Forshee (DigitalOcean) <[email protected]>

Tested-by Seth Forshee (DigitalOcean) <[email protected]>

---
 drivers/vhost/vhost.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/vhost/vhost.c +1 -2

index 43c9770b86e5..87e3cf12da1c 100644
@@ -363,8 +363,7 @@ static int vhost_worker(void *data)
 			kcov_remote_start_common(dev->kcov_handle);
 			work->fn(work);
 			kcov_remote_stop();
-			if (need_resched())
-				schedule();
+			cond_resched();
 		}
 	}
 	kthread_unuse_mm(dev->mm);

Signature · 1 line

2.39.1

Petr Mladek

maintainer

2023-02-27 15:55 UTC #4 lore raw

Quoted 11 lines · Josh Poimboeuf

There have been reports [1][2] of live patches failing to complete within a reasonable amount of time due to CPU-bound kthreads.
Fix it by patching tasks in cond_resched().
There are four different flavors of cond_resched(), depending on the kernel configuration. Hook into all of them.
A more elegant solution might be to use a preempt notifier. However, non-ORC unwinders can't unwind a preempted task reliably.

Quoted 6 lines

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lkml.kernel.org/lkml/[email protected]
Tested-by Seth Forshee (DigitalOcean) <[email protected]>
Reviewed-by Petr Mladek <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>

Just for record, I have double checked the changes against v2 and everything looks good to me.

Best Regards,
Petr

Seth Forshee

2023-02-28 16:41 UTC #14 lore raw

Quoted 9 lines · Josh Poimboeuf

v3:
- Add barriers (pmladek)
- Update comments
v2:
- Avoid calling klp_cond_resched_disable() in klp_cancel_transition()
- Fix race in klp_reverse_transition()
Fix patching stalls caused by busy kthreads.

I ran the v3 patches through my reproducer, still working great.

Thanks,
Seth

Josh Poimboeuf

author maintainer

2023-02-28 16:56 UTC #5 lore raw

In reply to Petr Mladek

Quoted 15 lines · Petr Mladek

Quoted 14 lines · Josh Poimboeuf
There have been reports [1][2] of live patches failing to complete within a reasonable amount of time due to CPU-bound kthreads.
Fix it by patching tasks in cond_resched().
There are four different flavors of cond_resched(), depending on the kernel configuration. Hook into all of them.
A more elegant solution might be to use a preempt notifier. However, non-ORC unwinders can't unwind a preempted task reliably.
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lkml.kernel.org/lkml/[email protected]

Quoted 6 lines

Quoted 3 lines
Tested-by Seth Forshee (DigitalOcean) <[email protected]>
Reviewed-by Petr Mladek <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
Just for record, I have double checked the changes against v2 and everything looks good to me.

Whoops, so I found another little surprise:

static int klp_check_stack(struct task_struct *task, const char **oldname)

{
        static unsigned long entries[MAX_STACK_ENTRIES];
	^^^^^^

That entries array is shared between the klp_mutex owner and all cond_resched() callers.

MAX_STACK_ENTRIES is 100, which seems excessive. If we halved that, the array would be "only" 400 bytes, which is *almost* reasonable to allocate on the stack?

Alternatively we could have a percpu entries array... :-/

Signature · 1 line

Josh

Petr Mladek

maintainer

2023-03-03 14:00 UTC #6 lore raw

Quoted 25 lines · Josh Poimboeuf

Quoted 15 lines · Petr Mladek
Quoted 14 lines · Josh Poimboeuf
There have been reports [1][2] of live patches failing to complete within a reasonable amount of time due to CPU-bound kthreads.
Fix it by patching tasks in cond_resched().
There are four different flavors of cond_resched(), depending on the kernel configuration. Hook into all of them.
A more elegant solution might be to use a preempt notifier. However, non-ORC unwinders can't unwind a preempted task reliably.
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lkml.kernel.org/lkml/[email protected]
Quoted 6 lines
Quoted 3 lines
Tested-by Seth Forshee (DigitalOcean) <[email protected]>
Reviewed-by Petr Mladek <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
Just for record, I have double checked the changes against v2 and everything looks good to me.
Whoops, so I found another little surprise:

Quoted 7 lines

static int klp_check_stack(struct task_struct *task, const char **oldname)
{
        static unsigned long entries[MAX_STACK_ENTRIES];
	^^^^^^
That entries array is shared between the klp_mutex owner and all cond_resched() callers.

Huh, great catch!

Quoted 3 lines

MAX_STACK_ENTRIES is 100, which seems excessive. If we halved that, the array would be "only" 400 bytes, which is *almost* reasonable to allocate on the stack?

It is just for the stack in the process context. Right?

I think that I have never seen a stack with over 50 entries. And in the worst case, a bigger amount of entries would "just" result in a non-reliable stack which might be acceptable.

It looks acceptable to me.

Quoted 1 line

Alternatively we could have a percpu entries array... :-/

That said, percpu entries would be fine as well. It sounds like a good price for the livepatching feature. I think that livepatching is used on big systems anyway.

I slightly prefer the per-cpu solution.

Best Regards,
Petr

Josh Poimboeuf

author maintainer

2023-03-13 23:33 UTC #7 lore raw

PATCH 1/1 livepatch: Convert stack entries array to percpu

Quoted 12 lines · Petr Mladek

Quoted 3 lines
MAX_STACK_ENTRIES is 100, which seems excessive. If we halved that, the array would be "only" 400 bytes, which is *almost* reasonable to allocate on the stack?
It is just for the stack in the process context. Right?
I think that I have never seen a stack with over 50 entries. And in the worst case, a bigger amount of entries would "just" result in a non-reliable stack which might be acceptable.
It looks acceptable to me.

Quoted 7 lines

Quoted 1 line
Alternatively we could have a percpu entries array... :-/
That said, percpu entries would be fine as well. It sounds like a good price for the livepatching feature. I think that livepatching is used on big systems anyway.
I slightly prefer the per-cpu solution.

Booting a kernel with PREEMPT+LOCKDEP gave me a high-water mark of 60+ stack entries, seen when probing a device. I decided not to mess with MAX_STACK_ENTRIES, and instead just convert the entries to percpu. This patch could be inserted at the beginning of the set.

---8<---

Subject: [PATCH 0.5/3] livepatch: Convert stack entries array to percpu

The entries array in klp_check_stack() is static local because it's too big to be reasonably allocated on the stack. Serialized access is enforced by the klp_mutex.

In preparation for calling klp_check_stack() without the mutex (from cond_resched), convert it to a percpu variable.

---
 kernel/livepatch/transition.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

kernel/livepatch/transition.c +7 -2

index f1b25ec581e0..135fc73e2e5d 100644
@@ -14,6 +14,8 @@
 #include "transition.h"
 
 #define MAX_STACK_ENTRIES  100
+DEFINE_PER_CPU(unsigned long[MAX_STACK_ENTRIES], klp_stack_entries);
+
 #define STACK_ERR_BUF_SIZE 128
 
 #define SIGNALS_TIMEOUT 15

@@ -240,12 +242,15 @@ static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
  */
 static int klp_check_stack(struct task_struct *task, const char **oldname)
 {
-	static unsigned long entries[MAX_STACK_ENTRIES];
+	unsigned long *entries = this_cpu_ptr(klp_stack_entries);
 	struct klp_object *obj;
 	struct klp_func *func;
 	int ret, nr_entries;
 
-	ret = stack_trace_save_tsk_reliable(task, entries, ARRAY_SIZE(entries));
+	/* Protect 'klp_stack_entries' */
+	lockdep_assert_preemption_disabled();
+
+	ret = stack_trace_save_tsk_reliable(task, entries, MAX_STACK_ENTRIES);
 	if (ret < 0)
 		return -EINVAL;
 	nr_entries = ret;

Signature · 1 line

2.39.2

Petr Mladek

maintainer

2023-03-14 10:51 UTC #8 lore raw

Re: [PATCH 0.5/3] livepatch: Convert stack entries array to percpu

Quoted 19 lines · Josh Poimboeuf

Quoted 18 lines · Petr Mladek
Quoted 3 lines
MAX_STACK_ENTRIES is 100, which seems excessive. If we halved that, the array would be "only" 400 bytes, which is *almost* reasonable to allocate on the stack?
It is just for the stack in the process context. Right?
I think that I have never seen a stack with over 50 entries. And in the worst case, a bigger amount of entries would "just" result in a non-reliable stack which might be acceptable.
It looks acceptable to me.
Quoted 1 line
Alternatively we could have a percpu entries array... :-/
That said, percpu entries would be fine as well. It sounds like a good price for the livepatching feature. I think that livepatching is used on big systems anyway.

Quoted 6 lines

Quoted 1 line
I slightly prefer the per-cpu solution.
Booting a kernel with PREEMPT+LOCKDEP gave me a high-water mark of 60+ stack entries, seen when probing a device. I decided not to mess with MAX_STACK_ENTRIES, and instead just convert the entries to percpu. This patch could be inserted at the beginning of the set.

Good to know.

Quoted 10 lines

---8<---
Subject: [PATCH 0.5/3] livepatch: Convert stack entries array to percpu
kernel/livepatch/transition.c +0 -0
@@ -240,12 +242,15 @@ static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
  */
 static int klp_check_stack(struct task_struct *task, const char **oldname)

kernel/livepatch/transition.c static int klp_check_stack_func(struct klp_func *func, unsigned long *entries, · line 251

Quoted 10 lines

kernel/livepatch/transition.c +3 -2

 {
-	static unsigned long entries[MAX_STACK_ENTRIES];
+	unsigned long *entries = this_cpu_ptr(klp_stack_entries);
 	struct klp_object *obj;
 	struct klp_func *func;
 	int ret, nr_entries;
 
-	ret = stack_trace_save_tsk_reliable(task, entries, ARRAY_SIZE(entries));
+	/* Protect 'klp_stack_entries' */
+	lockdep_assert_preemption_disabled();

I think about adding:

	/*
	 * Stay on the safe side even when cond_resched() is called from
	 * an IRQ context by mistake.
	 */
	if (!in_task())
		return -EINVAL;

Or is this prevented another way, please?

kernel/livepatch/transition.c static int klp_check_stack_func(struct klp_func *func, unsigned long *entries, · line 256

Quoted 5 lines

kernel/livepatch/transition.c +2 -0

+
+	ret = stack_trace_save_tsk_reliable(task, entries, MAX_STACK_ENTRIES);
 	if (ret < 0)
 		return -EINVAL;
 	nr_entries = ret;

Otherwise, it looks good to me.

Best Regards,
Petr

Josh Poimboeuf

author maintainer

2023-03-15 00:26 UTC #9 lore raw

Re: [PATCH 0.5/3] livepatch: Convert stack entries array to percpu

Quoted 14 lines · Petr Mladek

kernel/livepatch/transition.c static int klp_check_stack_func(struct klp_func *func, unsigned long *entries, · line 251
Quoted 11 lines
static int klp_check_stack(struct task_struct *task, const char **oldname) {
kernel/livepatch/transition.c +3 -2
-	static unsigned long entries[MAX_STACK_ENTRIES];
+	unsigned long *entries = this_cpu_ptr(klp_stack_entries);
 	struct klp_object *obj;
 	struct klp_func *func;
 	int ret, nr_entries;
 
-	ret = stack_trace_save_tsk_reliable(task, entries, ARRAY_SIZE(entries));
+	/* Protect 'klp_stack_entries' */
+	lockdep_assert_preemption_disabled();
I think about adding:

Quoted 8 lines

	/*
	 * Stay on the safe side even when cond_resched() is called from
	 * an IRQ context by mistake.
	 */
	if (!in_task())
		return -EINVAL;

Or is this prevented another way, please?

An IRQ handler trying to schedule would be a pretty major bug, no?

I think there are already several different checks for that. For example, the __might_resched() call in cond_resched().

Signature · 1 line

Josh

tip-bot2 for Josh Poimboeuf

merge bot Applied

2023-03-22 22:48 UTC #13 lore raw

[tip: sched/core] vhost: Fix livepatch timeouts in vhost_worker()

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

Commit-ID:     05bfb338fa8dd40b008ce443e397fc374f6bd107
Gitweb:        https://git.kernel.org/tip/05bfb338fa8dd40b008ce443e397fc374f6bd107
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Fri, 24 Feb 2023 08:50:01 -08:00
Committer:     Peter Zijlstra <[email protected]>
CommitterDate: Wed, 22 Mar 2023 17:09:29 +01:00

vhost: Fix livepatch timeouts in vhost_worker()

Reported-by Seth Forshee (DigitalOcean) <[email protected]>

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

Link https://lore.kernel.org/r/509f6ea6fe6505f0a75a66026ba531c765ef922f.1677257135.git.jpoimboe@kernel.org open here

Tested-by Seth Forshee (DigitalOcean) <[email protected]>

---
 drivers/vhost/vhost.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/vhost/vhost.c +1 -2

index f11bdbe..822fbdb 100644
@@ -363,8 +363,7 @@ static int vhost_worker(void *data)
 			kcov_remote_start_common(dev->kcov_handle);
 			work->fn(work);
 			kcov_remote_stop();
-			if (need_resched())
-				schedule();
+			cond_resched();
 		}
 	}
 	kthread_unuse_mm(dev->mm);

tip-bot2 for Josh Poimboeuf

merge bot Applied

2023-03-22 22:48 UTC #2 lore raw

[tip: sched/core] livepatch: Skip task_call_func() for current task

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

Commit-ID:     383439d3d400d4c5a7ffb4495124adc6be6a05e2
Gitweb:        https://git.kernel.org/tip/383439d3d400d4c5a7ffb4495124adc6be6a05e2
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Fri, 24 Feb 2023 08:49:59 -08:00
Committer:     Peter Zijlstra <[email protected]>
CommitterDate: Wed, 22 Mar 2023 17:09:28 +01:00

livepatch: Skip task_call_func() for current task

The current task doesn't need the scheduler's protection to unwind its own stack.

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

Link https://lore.kernel.org/r/4b92e793462d532a05f03767151fa29db3e68e13.1677257135.git.jpoimboe@kernel.org open here

Tested-by Seth Forshee (DigitalOcean) <[email protected]>

---
 kernel/livepatch/transition.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

kernel/livepatch/transition.c +5 -1

index 135fc73..824e2e3 100644
@@ -312,7 +312,11 @@ static bool klp_try_switch_task(struct task_struct *task)
 	 * functions.  If all goes well, switch the task to the target patch
 	 * state.
 	 */
-	ret = task_call_func(task, klp_check_and_switch_task, &old_name);
+	if (task == current)
+		ret = klp_check_and_switch_task(current, &old_name);
+	else
+		ret = task_call_func(task, klp_check_and_switch_task, &old_name);
+
 	switch (ret) {
 	case 0:		/* success */
 		break;

tip-bot2 for Josh Poimboeuf

merge bot Applied

2023-03-22 22:49 UTC #11 lore raw

[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lkml.kernel.org/lkml/[email protected]

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

Commit-ID:     e3ff7c609f39671d1aaff4fb4a8594e14f3e03f8
Gitweb:        https://git.kernel.org/tip/e3ff7c609f39671d1aaff4fb4a8594e14f3e03f8
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Fri, 24 Feb 2023 08:50:00 -08:00
Committer:     Peter Zijlstra <[email protected]>
CommitterDate: Wed, 22 Mar 2023 17:09:28 +01:00

livepatch,sched: Add livepatch task switching to cond_resched()

There have been reports [1][2] of live patches failing to complete within a reasonable amount of time due to CPU-bound kthreads.

Fix it by patching tasks in cond_resched().

There are four different flavors of cond_resched(), depending on the kernel configuration. Hook into all of them.

A more elegant solution might be to use a preempt notifier. However, non-ORC unwinders can't unwind a preempted task reliably.

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

Link https://lore.kernel.org/r/4ae981466b7814ec221014fc2554b2f86f3fb70b.1677257135.git.jpoimboe@kernel.org open here

Tested-by Seth Forshee (DigitalOcean) <[email protected]>

---
 include/linux/livepatch.h       |   1 +-
 include/linux/livepatch_sched.h |  29 ++++++++-
 include/linux/sched.h           |  20 ++++--
 kernel/livepatch/core.c         |   1 +-
 kernel/livepatch/transition.c   | 107 ++++++++++++++++++++++++++-----
 kernel/sched/core.c             |  64 ++++++++++++++++---
 6 files changed, 194 insertions(+), 28 deletions(-)
 create mode 100644 include/linux/livepatch_sched.h

include/linux/livepatch.h +1 -0

index 293e299..9b9b38e 100644
@@ -13,6 +13,7 @@
 #include <linux/ftrace.h>
 #include <linux/completion.h>
 #include <linux/list.h>
+#include <linux/livepatch_sched.h>
 
 #if IS_ENABLED(CONFIG_LIVEPATCH)

include/linux/livepatch_sched.h new file +29 -0

new file mode 100644
index 0000000..013794f
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _LINUX_LIVEPATCH_SCHED_H_
+#define _LINUX_LIVEPATCH_SCHED_H_
+
+#include <linux/jump_label.h>
+#include <linux/static_call_types.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)
+{
+	if (static_branch_unlikely(&klp_sched_try_switch_key))
+		__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) {}
+#endif /* CONFIG_LIVEPATCH */
+
+#endif /* _LINUX_LIVEPATCH_SCHED_H_ */

include/linux/sched.h +15 -5

index 63d2421..6d654eb 100644
@@ -36,6 +36,7 @@
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
+#include <linux/livepatch_sched.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */

@@ -2070,6 +2071,9 @@ 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)

@@ -2078,6 +2082,7 @@ static __always_inline int _cond_resched(void)
 }
 
 #elif defined(CONFIG_PREEMPT_DYNAMIC) && defined(CONFIG_HAVE_PREEMPT_DYNAMIC_KEY)
+
 extern int dynamic_cond_resched(void);
 
 static __always_inline int _cond_resched(void)

@@ -2085,20 +2090,25 @@ static __always_inline int _cond_resched(void)
 	return dynamic_cond_resched();
 }
 
-#else
+#else /* !CONFIG_PREEMPTION */
 
 static inline int _cond_resched(void)
 {
+	klp_sched_try_switch();
 	return __cond_resched();
 }
 
-#endif /* CONFIG_PREEMPT_DYNAMIC */
+#endif /* PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
 
-#else
+#else /* CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC */
 
-static inline int _cond_resched(void) { return 0; }
+static inline int _cond_resched(void)
+{
+	klp_sched_try_switch();
+	return 0;
+}
 
-#endif /* !defined(CONFIG_PREEMPTION) || defined(CONFIG_PREEMPT_DYNAMIC) */
+#endif /* !CONFIG_PREEMPTION || CONFIG_PREEMPT_DYNAMIC */
 
 #define cond_resched() ({			\
 	__might_resched(__FILE__, __LINE__, 0);	\

kernel/livepatch/core.c +1 -0

index 4bd2d5e..eea7c8e 100644
@@ -33,6 +33,7 @@
  *
  * - klp_ftrace_handler()
  * - klp_update_patch_state()
+ * - __klp_sched_try_switch()
  */
 DEFINE_MUTEX(klp_mutex);

kernel/livepatch/transition.c +92 ~~-15~~

171 lines not on this page · raw message

kernel/sched/core.c +56 -8

111 lines not on this page · raw message

tip-bot2 for Josh Poimboeuf

merge bot Applied

2023-03-22 22:49 UTC #10 lore raw

[tip: sched/core] livepatch: Convert stack entries array to percpu

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

Commit-ID:     e92606fa172f63a26054885b9715be86c643229d
Gitweb:        https://git.kernel.org/tip/e92606fa172f63a26054885b9715be86c643229d
Author:        Josh Poimboeuf <[email protected]>
AuthorDate:    Mon, 13 Mar 2023 16:33:46 -07:00
Committer:     Peter Zijlstra <[email protected]>
CommitterDate: Wed, 22 Mar 2023 17:09:28 +01:00

livepatch: Convert stack entries array to percpu

The entries array in klp_check_stack() is static local because it's too big to be reasonably allocated on the stack. Serialized access is enforced by the klp_mutex.

In preparation for calling klp_check_stack() without the mutex (from cond_resched), convert it to a percpu variable.

Link https://lkml.kernel.org/r/20230313233346.kayh4t2lpicjkpsv@treble open here

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

---
 kernel/livepatch/transition.c |  9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

kernel/livepatch/transition.c +7 -2

index f1b25ec..135fc73 100644
@@ -14,6 +14,8 @@
 #include "transition.h"
 
 #define MAX_STACK_ENTRIES  100
+DEFINE_PER_CPU(unsigned long[MAX_STACK_ENTRIES], klp_stack_entries);
+
 #define STACK_ERR_BUF_SIZE 128
 
 #define SIGNALS_TIMEOUT 15

@@ -240,12 +242,15 @@ static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
  */
 static int klp_check_stack(struct task_struct *task, const char **oldname)
 {
-	static unsigned long entries[MAX_STACK_ENTRIES];
+	unsigned long *entries = this_cpu_ptr(klp_stack_entries);
 	struct klp_object *obj;
 	struct klp_func *func;
 	int ret, nr_entries;
 
-	ret = stack_trace_save_tsk_reliable(task, entries, ARRAY_SIZE(entries));
+	/* Protect 'klp_stack_entries' */
+	lockdep_assert_preemption_disabled();
+
+	ret = stack_trace_save_tsk_reliable(task, entries, MAX_STACK_ENTRIES);
 	if (ret < 0)
 		return -EINVAL;
 	nr_entries = ret;

Miroslav Benes

maintainer Acked-by on the v3 series

2023-03-30 16:11 UTC #15 lore raw