[PATCH v1 0/2] add Rust version of might_sleep()

[PATCH v1 0/2] add Rust version of might_sleep()

[FUJITA Tomonori]

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

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    | 26 ++++++++++++++++++
 4 files changed, 75 insertions(+), 21 deletions(-)


base-commit: f4d2ef48250ad057e4f00087967b5ff366da9f39
-- 
2.43.0

[PATCH v1 1/2] sched/core: Add __might_sleep_precision()

[FUJITA Tomonori]

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 cfaca3040b2f..f212e1706d5a 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

[PATCH v1 2/2] rust: task: add Rust version of might_sleep()

[FUJITA Tomonori]

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 | 26 ++++++++++++++++++++++++++
 2 files changed, 32 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..1f0156b38ab5 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -380,3 +380,29 @@ 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();
+        // SAFETY: FFI call.
+        unsafe {
+            crate::bindings::__might_sleep_precision(
+                loc.file().as_ptr().cast(),
+                loc.file().len() as i32,
+                loc.line() as i32,
+            )
+        }
+    }
+
+    // SAFETY: FFI call.
+    unsafe { crate::bindings::might_resched() }
+}
-- 
2.43.0

Re: [PATCH v1 2/2] rust: task: add Rust version of might_sleep()

[Alice Ryhl]

On Sun, Apr 06, 2025 at 08:07:18PM +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>
> ---
>  rust/helpers/task.c |  6 ++++++
>  rust/kernel/task.rs | 26 ++++++++++++++++++++++++++
>  2 files changed, 32 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..1f0156b38ab5 100644
> --- a/rust/kernel/task.rs
> +++ b/rust/kernel/task.rs
> @@ -380,3 +380,29 @@ 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();
> +        // SAFETY: FFI call.

Overall this looks okay to me, but this safety comment could be
improved. This being an FFI call is not the reason *why* it is safe to
make this call.

// SAFETY: `file.as_ptr()` is valid for reading for `file.len()` bytes.

And I might separate the file into a separate variable for clarity:
let loc = core::panic::Location::caller();
let file = loc.file();

> +        unsafe {
> +            crate::bindings::__might_sleep_precision(
> +                loc.file().as_ptr().cast(),
> +                loc.file().len() as i32,
> +                loc.line() as i32,
> +            )
> +        }
> +    }
> +
> +    // SAFETY: FFI call.
> +    unsafe { crate::bindings::might_resched() }

And here you can say
// SAFETY: Always safe to call.

Alice

Re: [PATCH v1 2/2] rust: task: add Rust version of might_sleep()

[FUJITA Tomonori]

On Wed, 9 Apr 2025 08:51:44 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

>> diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
>> index 9e6f6854948d..1f0156b38ab5 100644
>> --- a/rust/kernel/task.rs
>> +++ b/rust/kernel/task.rs
>> @@ -380,3 +380,29 @@ 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();
>> +        // SAFETY: FFI call.
> 
> Overall this looks okay to me, but this safety comment could be
> improved. This being an FFI call is not the reason *why* it is safe to
> make this call.

Undertood.

> // SAFETY: `file.as_ptr()` is valid for reading for `file.len()` bytes.
> 
> And I might separate the file into a separate variable for clarity:
> let loc = core::panic::Location::caller();
> let file = loc.file();

Fixed.

>> +        unsafe {
>> +            crate::bindings::__might_sleep_precision(
>> +                loc.file().as_ptr().cast(),
>> +                loc.file().len() as i32,
>> +                loc.line() as i32,
>> +            )
>> +        }
>> +    }
>> +
>> +    // SAFETY: FFI call.
>> +    unsafe { crate::bindings::might_resched() }
> 
> And here you can say
> // SAFETY: Always safe to call.

Fixed.

Thanks a lot!