public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] add Rust version of might_sleep()
@ 2025-04-10 22:56 FUJITA Tomonori
  2025-04-10 22:56 ` [PATCH v2 1/2] sched/core: Add __might_sleep_precision() FUJITA Tomonori
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: FUJITA Tomonori @ 2025-04-10 22:56 UTC (permalink / raw)
  To: linux-kernel, rust-for-linux
  Cc: boqun.feng, ojeda, alex.gaynor, gary, bjorn3_gh, benno.lossin,
	a.hindborg, aliceryhl, tmgross, dakr, mingo, peterz, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, pmladek

This patchset adds Rust version of might_sleep().

These patches were previously part of the IO polling patchset [1], but
they were split out to make upstreaming easier.

The first patch is for sched/core, which adds
__might_sleep_precision(), rust friendly version of __might_sleep(),
which takes a pointer to a string with the length instead of a
null-terminated string. Rust's core::panic::Location::file(), which
gives the file name of a caller, doesn't provide a null-terminated
string. __might_sleep_precision() uses a precision specifier in the
printk format, which specifies the length of a string; a string
doesn't need to be a null-terminated. Providing a null-terminated
string for better C interoperability is under discussion [2].

The second patch adds a Rust implementation of might_sleep(), on top
of the changes in the first patch.

[1]: https://lore.kernel.org/lkml/20250220070611.214262-1-fujita.tomonori@gmail.com/
[2]: https://github.com/rust-lang/libs-team/issues/466

v2:
- improve SAFETY comment
v1: https://lore.kernel.org/lkml/20250406110718.126146-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (2):
  sched/core: Add __might_sleep_precision()
  rust: task: add Rust version of might_sleep()

 include/linux/kernel.h |  2 ++
 kernel/sched/core.c    | 62 ++++++++++++++++++++++++++++--------------
 rust/helpers/task.c    |  6 ++++
 rust/kernel/task.rs    | 28 +++++++++++++++++++
 4 files changed, 77 insertions(+), 21 deletions(-)


base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
-- 
2.43.0


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

* [PATCH v2 1/2] sched/core: Add __might_sleep_precision()
  2025-04-10 22:56 [PATCH v2 0/2] add Rust version of might_sleep() FUJITA Tomonori
@ 2025-04-10 22:56 ` FUJITA Tomonori
  2025-04-10 22:56 ` [PATCH v2 2/2] rust: task: add Rust version of might_sleep() FUJITA Tomonori
  2025-04-22 14:30 ` [PATCH v2 0/2] " Boqun Feng
  2 siblings, 0 replies; 6+ messages in thread
From: FUJITA Tomonori @ 2025-04-10 22:56 UTC (permalink / raw)
  To: linux-kernel, rust-for-linux
  Cc: Daniel Almeida, Alice Ryhl, Boqun Feng, ojeda, alex.gaynor, gary,
	bjorn3_gh, benno.lossin, a.hindborg, tmgross, dakr, mingo, peterz,
	juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall,
	mgorman, vschneid, pmladek

Add __might_sleep_precision(), Rust friendly version of
__might_sleep(), which takes a pointer to a string with the length
instead of a null-terminated string.

Rust's core::panic::Location::file(), which gives the file name of a
caller, doesn't provide a null-terminated
string. __might_sleep_precision() uses a precision specifier in the
printk format, which specifies the length of a string; a string
doesn't need to be a null-terminated.

Modify __might_sleep() to call __might_sleep_precision() but the
impact should be negligible. When printing the error (sleeping
function called from invalid context), the precision string format is
used instead of the simple string format; the precision specifies the
the maximum length of the displayed string.

Note that Location::file() providing a null-terminated string for
better C interoperability is under discussion [1].

[1]: https://github.com/rust-lang/libs-team/issues/466

Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 include/linux/kernel.h |  2 ++
 kernel/sched/core.c    | 62 ++++++++++++++++++++++++++++--------------
 2 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index be2e8c0a187e..086ee1dc447e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -87,6 +87,7 @@ extern int dynamic_might_resched(void);
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
 extern void __might_resched(const char *file, int line, unsigned int offsets);
 extern void __might_sleep(const char *file, int line);
+extern void __might_sleep_precision(const char *file, int len, int line);
 extern void __cant_sleep(const char *file, int line, int preempt_offset);
 extern void __cant_migrate(const char *file, int line);
 
@@ -145,6 +146,7 @@ extern void __cant_migrate(const char *file, int line);
   static inline void __might_resched(const char *file, int line,
 				     unsigned int offsets) { }
 static inline void __might_sleep(const char *file, int line) { }
+static inline void __might_sleep_precision(const char *file, int len, int line) { }
 # define might_sleep() do { might_resched(); } while (0)
 # define cant_sleep() do { } while (0)
 # define cant_migrate()		do { } while (0)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c81cf642dba0..6e87d997d03a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8730,24 +8730,6 @@ void __init sched_init(void)
 
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
 
-void __might_sleep(const char *file, int line)
-{
-	unsigned int state = get_current_state();
-	/*
-	 * Blocking primitives will set (and therefore destroy) current->state,
-	 * since we will exit with TASK_RUNNING make sure we enter with it,
-	 * otherwise we will destroy state.
-	 */
-	WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
-			"do not call blocking ops when !TASK_RUNNING; "
-			"state=%x set at [<%p>] %pS\n", state,
-			(void *)current->task_state_change,
-			(void *)current->task_state_change);
-
-	__might_resched(file, line, 0);
-}
-EXPORT_SYMBOL(__might_sleep);
-
 static void print_preempt_disable_ip(int preempt_offset, unsigned long ip)
 {
 	if (!IS_ENABLED(CONFIG_DEBUG_PREEMPT))
@@ -8769,7 +8751,8 @@ static inline bool resched_offsets_ok(unsigned int offsets)
 	return nested == offsets;
 }
 
-void __might_resched(const char *file, int line, unsigned int offsets)
+static void __might_resched_precision(const char *file, int file_len, int line,
+				      unsigned int offsets)
 {
 	/* Ratelimiting timestamp: */
 	static unsigned long prev_jiffy;
@@ -8792,8 +8775,8 @@ void __might_resched(const char *file, int line, unsigned int offsets)
 	/* Save this before calling printk(), since that will clobber it: */
 	preempt_disable_ip = get_preempt_disable_ip(current);
 
-	pr_err("BUG: sleeping function called from invalid context at %s:%d\n",
-	       file, line);
+	pr_err("BUG: sleeping function called from invalid context at %.*s:%d\n",
+	       file_len, file, line);
 	pr_err("in_atomic(): %d, irqs_disabled(): %d, non_block: %d, pid: %d, name: %s\n",
 	       in_atomic(), irqs_disabled(), current->non_block_count,
 	       current->pid, current->comm);
@@ -8818,8 +8801,45 @@ void __might_resched(const char *file, int line, unsigned int offsets)
 	dump_stack();
 	add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
 }
+
+/*
+ * The precision in vsnprintf() specifies the maximum length of the
+ * displayed string. The precision needs to be larger than the actual
+ * length of the string, so a sufficiently large value should be used
+ * for the filename length.
+ */
+#define MAX_FILENAME_LEN (1<<14)
+
+void __might_resched(const char *file, int line, unsigned int offsets)
+{
+	__might_resched_precision(file, MAX_FILENAME_LEN, line, offsets);
+}
 EXPORT_SYMBOL(__might_resched);
 
+void __might_sleep_precision(const char *file, int len, int line)
+{
+	unsigned int state = get_current_state();
+	/*
+	 * Blocking primitives will set (and therefore destroy) current->state,
+	 * since we will exit with TASK_RUNNING make sure we enter with it,
+	 * otherwise we will destroy state.
+	 */
+	WARN_ONCE(state != TASK_RUNNING && current->task_state_change,
+			"do not call blocking ops when !TASK_RUNNING; "
+			"state=%x set at [<%p>] %pS\n", state,
+			(void *)current->task_state_change,
+			(void *)current->task_state_change);
+
+	__might_resched_precision(file, len, line, 0);
+}
+EXPORT_SYMBOL_GPL(__might_sleep_precision);
+
+void __might_sleep(const char *file, int line)
+{
+	__might_sleep_precision(file, MAX_FILENAME_LEN, line);
+}
+EXPORT_SYMBOL(__might_sleep);
+
 void __cant_sleep(const char *file, int line, int preempt_offset)
 {
 	static unsigned long prev_jiffy;
-- 
2.43.0


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

* [PATCH v2 2/2] rust: task: add Rust version of might_sleep()
  2025-04-10 22:56 [PATCH v2 0/2] add Rust version of might_sleep() FUJITA Tomonori
  2025-04-10 22:56 ` [PATCH v2 1/2] sched/core: Add __might_sleep_precision() FUJITA Tomonori
@ 2025-04-10 22:56 ` FUJITA Tomonori
  2025-04-11  9:11   ` Alice Ryhl
  2025-04-22 14:30 ` [PATCH v2 0/2] " Boqun Feng
  2 siblings, 1 reply; 6+ messages in thread
From: FUJITA Tomonori @ 2025-04-10 22:56 UTC (permalink / raw)
  To: linux-kernel, rust-for-linux
  Cc: boqun.feng, ojeda, alex.gaynor, gary, bjorn3_gh, benno.lossin,
	a.hindborg, aliceryhl, tmgross, dakr, mingo, peterz, juri.lelli,
	vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
	vschneid, pmladek

Adds a helper function equivalent to the C's might_sleep(), which
serves as a debugging aid and a potential scheduling point.

Note that this function can only be used in a nonatomic context.

This will be used by Rust version of read_poll_timeout().

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/task.c |  6 ++++++
 rust/kernel/task.rs | 28 ++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/rust/helpers/task.c b/rust/helpers/task.c
index 31c33ea2dce6..2c85bbc2727e 100644
--- a/rust/helpers/task.c
+++ b/rust/helpers/task.c
@@ -1,7 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <linux/kernel.h>
 #include <linux/sched/task.h>
 
+void rust_helper_might_resched(void)
+{
+	might_resched();
+}
+
 struct task_struct *rust_helper_get_current(void)
 {
 	return current;
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 9e6f6854948d..bb1102f6cc02 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -380,3 +380,31 @@ fn eq(&self, other: &Kuid) -> bool {
 }
 
 impl Eq for Kuid {}
+
+/// Annotation for functions that can sleep.
+///
+/// Equivalent to the C side [`might_sleep()`], this function serves as
+/// a debugging aid and a potential scheduling point.
+///
+/// This function can only be used in a nonatomic context.
+#[track_caller]
+#[inline]
+pub fn might_sleep() {
+    #[cfg(CONFIG_DEBUG_ATOMIC_SLEEP)]
+    {
+        let loc = core::panic::Location::caller();
+        let file = loc.file();
+
+        // SAFETY: `file.as_ptr()` is valid for reading for `file.len()` bytes.
+        unsafe {
+            crate::bindings::__might_sleep_precision(
+                file.as_ptr().cast(),
+                file.len() as i32,
+                loc.line() as i32,
+            )
+        }
+    }
+
+    // SAFETY: Always safe to call.
+    unsafe { crate::bindings::might_resched() }
+}
-- 
2.43.0


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

* Re: [PATCH v2 2/2] rust: task: add Rust version of might_sleep()
  2025-04-10 22:56 ` [PATCH v2 2/2] rust: task: add Rust version of might_sleep() FUJITA Tomonori
@ 2025-04-11  9:11   ` Alice Ryhl
  0 siblings, 0 replies; 6+ messages in thread
From: Alice Ryhl @ 2025-04-11  9:11 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: linux-kernel, rust-for-linux, boqun.feng, ojeda, alex.gaynor,
	gary, bjorn3_gh, benno.lossin, a.hindborg, tmgross, dakr, mingo,
	peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, pmladek

On Fri, Apr 11, 2025 at 07:56:23AM +0900, FUJITA Tomonori wrote:
> Adds a helper function equivalent to the C's might_sleep(), which
> serves as a debugging aid and a potential scheduling point.
> 
> Note that this function can only be used in a nonatomic context.
> 
> This will be used by Rust version of read_poll_timeout().
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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

* Re: [PATCH v2 0/2] add Rust version of might_sleep()
  2025-04-10 22:56 [PATCH v2 0/2] add Rust version of might_sleep() FUJITA Tomonori
  2025-04-10 22:56 ` [PATCH v2 1/2] sched/core: Add __might_sleep_precision() FUJITA Tomonori
  2025-04-10 22:56 ` [PATCH v2 2/2] rust: task: add Rust version of might_sleep() FUJITA Tomonori
@ 2025-04-22 14:30 ` Boqun Feng
  2025-04-22 14:31   ` Peter Zijlstra
  2 siblings, 1 reply; 6+ messages in thread
From: Boqun Feng @ 2025-04-22 14:30 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: linux-kernel, rust-for-linux, ojeda, alex.gaynor, gary, bjorn3_gh,
	benno.lossin, a.hindborg, aliceryhl, tmgross, dakr, mingo, peterz,
	juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall,
	mgorman, vschneid, pmladek

On Fri, Apr 11, 2025 at 07:56:21AM +0900, FUJITA Tomonori wrote:
> This patchset adds Rust version of might_sleep().
> 
> These patches were previously part of the IO polling patchset [1], but
> they were split out to make upstreaming easier.
> 
> The first patch is for sched/core, which adds
> __might_sleep_precision(), rust friendly version of __might_sleep(),
> which takes a pointer to a string with the length instead of a
> null-terminated string. Rust's core::panic::Location::file(), which
> gives the file name of a caller, doesn't provide a null-terminated
> string. __might_sleep_precision() uses a precision specifier in the
> printk format, which specifies the length of a string; a string
> doesn't need to be a null-terminated. Providing a null-terminated
> string for better C interoperability is under discussion [2].
> 
> The second patch adds a Rust implementation of might_sleep(), on top
> of the changes in the first patch.
> 
> [1]: https://lore.kernel.org/lkml/20250220070611.214262-1-fujita.tomonori@gmail.com/
> [2]: https://github.com/rust-lang/libs-team/issues/466
> 
> v2:
> - improve SAFETY comment
> v1: https://lore.kernel.org/lkml/20250406110718.126146-1-fujita.tomonori@gmail.com/
> 

@scheduler, if there is no objection, I'm going to take these two into a
pull request to tip soon. Thanks!

Regards,
Boqun

> FUJITA Tomonori (2):
>   sched/core: Add __might_sleep_precision()
>   rust: task: add Rust version of might_sleep()
> 
>  include/linux/kernel.h |  2 ++
>  kernel/sched/core.c    | 62 ++++++++++++++++++++++++++++--------------
>  rust/helpers/task.c    |  6 ++++
>  rust/kernel/task.rs    | 28 +++++++++++++++++++
>  4 files changed, 77 insertions(+), 21 deletions(-)
> 
> 
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> -- 
> 2.43.0
> 
> 

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

* Re: [PATCH v2 0/2] add Rust version of might_sleep()
  2025-04-22 14:30 ` [PATCH v2 0/2] " Boqun Feng
@ 2025-04-22 14:31   ` Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2025-04-22 14:31 UTC (permalink / raw)
  To: Boqun Feng
  Cc: FUJITA Tomonori, linux-kernel, rust-for-linux, ojeda, alex.gaynor,
	gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross,
	dakr, mingo, juri.lelli, vincent.guittot, dietmar.eggemann,
	rostedt, bsegall, mgorman, vschneid, pmladek

On Tue, Apr 22, 2025 at 07:30:05AM -0700, Boqun Feng wrote:
> On Fri, Apr 11, 2025 at 07:56:21AM +0900, FUJITA Tomonori wrote:
> > This patchset adds Rust version of might_sleep().
> > 
> > These patches were previously part of the IO polling patchset [1], but
> > they were split out to make upstreaming easier.
> > 
> > The first patch is for sched/core, which adds
> > __might_sleep_precision(), rust friendly version of __might_sleep(),
> > which takes a pointer to a string with the length instead of a
> > null-terminated string. Rust's core::panic::Location::file(), which
> > gives the file name of a caller, doesn't provide a null-terminated
> > string. __might_sleep_precision() uses a precision specifier in the
> > printk format, which specifies the length of a string; a string
> > doesn't need to be a null-terminated. Providing a null-terminated
> > string for better C interoperability is under discussion [2].
> > 
> > The second patch adds a Rust implementation of might_sleep(), on top
> > of the changes in the first patch.
> > 
> > [1]: https://lore.kernel.org/lkml/20250220070611.214262-1-fujita.tomonori@gmail.com/
> > [2]: https://github.com/rust-lang/libs-team/issues/466
> > 
> > v2:
> > - improve SAFETY comment
> > v1: https://lore.kernel.org/lkml/20250406110718.126146-1-fujita.tomonori@gmail.com/
> > 
> 
> @scheduler, if there is no objection, I'm going to take these two into a
> pull request to tip soon. Thanks!

Yeah, go ahead. I still think Rust should be fixed, but sure, we can
live with this until that time.

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

end of thread, other threads:[~2025-04-22 14:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10 22:56 [PATCH v2 0/2] add Rust version of might_sleep() FUJITA Tomonori
2025-04-10 22:56 ` [PATCH v2 1/2] sched/core: Add __might_sleep_precision() FUJITA Tomonori
2025-04-10 22:56 ` [PATCH v2 2/2] rust: task: add Rust version of might_sleep() FUJITA Tomonori
2025-04-11  9:11   ` Alice Ryhl
2025-04-22 14:30 ` [PATCH v2 0/2] " Boqun Feng
2025-04-22 14:31   ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox