* [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16
@ 2025-05-06 4:58 Boqun Feng
2025-05-06 4:58 ` [PATCH 1/5] rust: sync: Mark CondVar::notify_*() inline Boqun Feng
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Boqun Feng @ 2025-05-06 4:58 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm
Hi Ingo & Peter,
Here are the Rust changes related to task and schedule, I figure the
best route would be via tip/sched/core hence I send this pull request.
Let me know if you want to do this differently.
Same as lockdep changes, I send this pull request in the format of
patchset, but here are also the tag and pull-request message.
Regards,
Boqun
The following changes since commit c70fc32f44431bb30f9025ce753ba8be25acbba3:
sched/fair: Adhere to place_entity() constraints (2025-04-16 21:09:12 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git tags/rust-sched.2025.05.05
for you to fetch changes up to d131cde69445b8a9a01ef1cbeed7232e171d4bd0:
rust: task: Add Rust version of might_sleep() (2025-05-04 20:12:12 -0700)
----------------------------------------------------------------
Rust task & schedule changes for v6.16:
- Make Task, CondVar and PollCondVar methods inline to avoid unnecessary
function calls
- Add might_sleep() support for Rust code: Rust's "#[track_caller]"
mechanism is used so that Rust's might_sleep() doesn't need to be
defined as a macro.
----------------------------------------------------------------
FUJITA Tomonori (2):
sched/core: Add __might_sleep_precision()
rust: task: Add Rust version of might_sleep()
Kunwu Chan (2):
rust: sync: Mark CondVar::notify_*() inline
rust: sync: Mark PollCondVar::drop() inline
Panagiotis Foliadis (1):
rust: task: Mark Task methods inline
include/linux/kernel.h | 2 ++
kernel/sched/core.c | 62 ++++++++++++++++++++++++-------------
rust/helpers/task.c | 6 ++++
rust/kernel/sync/condvar.rs | 3 ++
rust/kernel/sync/poll.rs | 1 +
rust/kernel/task.rs | 38 +++++++++++++++++++++++
6 files changed, 91 insertions(+), 21 deletions(-)
--
2.39.5 (Apple Git-154)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] rust: sync: Mark CondVar::notify_*() inline
2025-05-06 4:58 [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16 Boqun Feng
@ 2025-05-06 4:58 ` Boqun Feng
2025-05-06 4:58 ` [PATCH 2/5] rust: sync: Mark PollCondVar::drop() inline Boqun Feng
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Boqun Feng @ 2025-05-06 4:58 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Grace Deng
From: Kunwu Chan <kunwu.chan@hotmail.com>
When build the kernel using the llvm-18.1.3-rust-1.85.0-x86_64
with ARCH=arm64, the following symbols are generated:
$nm vmlinux | grep ' _R'.*CondVar | rustfilt
... T <kernel::sync::condvar::CondVar>::notify_all
... T <kernel::sync::condvar::CondVar>::notify_one
... T <kernel::sync::condvar::CondVar>::notify_sync
...
These notify_*() symbols are trivial wrappers around the C functions
__wake_up() and __wake_up_sync(). It doesn't make sense to go through
a trivial wrapper for these functions, so mark them inline.
[boqun: Reword the commit title for consistency and reformat the commit
log.]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Link: https://github.com/Rust-for-Linux/linux/issues/1145
Co-developed-by: Grace Deng <Grace.Deng006@Gmail.com>
Signed-off-by: Grace Deng <Grace.Deng006@Gmail.com>
Signed-off-by: Kunwu Chan <kunwu.chan@hotmail.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250324061835.1693125-1-kunwu.chan@linux.dev
---
rust/kernel/sync/condvar.rs | 3 +++
1 file changed, 3 insertions(+)
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index caebf03f553b..c6ec64295c9f 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -216,6 +216,7 @@ fn notify(&self, count: c_int) {
/// This method behaves like `notify_one`, except that it hints to the scheduler that the
/// current thread is about to go to sleep, so it should schedule the target thread on the same
/// CPU.
+ #[inline]
pub fn notify_sync(&self) {
// SAFETY: `wait_queue_head` points to valid memory.
unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), TASK_NORMAL) };
@@ -225,6 +226,7 @@ pub fn notify_sync(&self) {
///
/// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
/// completely (as opposed to automatically waking up the next waiter).
+ #[inline]
pub fn notify_one(&self) {
self.notify(1);
}
@@ -233,6 +235,7 @@ pub fn notify_one(&self) {
///
/// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost
/// completely (as opposed to automatically waking up the next waiter).
+ #[inline]
pub fn notify_all(&self) {
self.notify(0);
}
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] rust: sync: Mark PollCondVar::drop() inline
2025-05-06 4:58 [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16 Boqun Feng
2025-05-06 4:58 ` [PATCH 1/5] rust: sync: Mark CondVar::notify_*() inline Boqun Feng
@ 2025-05-06 4:58 ` Boqun Feng
2025-05-06 4:58 ` [PATCH 3/5] rust: task: Mark Task methods inline Boqun Feng
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Boqun Feng @ 2025-05-06 4:58 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Grace Deng
From: Kunwu Chan <kunwu.chan@hotmail.com>
When building the kernel using the llvm-18.1.3-rust-1.85.0-x86_64
with ARCH=arm64, the following symbols are generated:
$nm vmlinux | grep ' _R'.*PollCondVar | rustfilt
... T <kernel::sync::poll::PollCondVar as kernel::init::PinnedDrop>::drop
...
This Rust symbol is trivial wrappers around the C functions
__wake_up_pollfree() and synchronize_rcu(). It doesn't make sense to go
through a trivial wrapper for its functions, so mark it inline.
[boqun: Reword the commit title and re-format the commit log per tip
tree's requirement, remove unnecessary information from "nm vmlinux"
result.]
Link: https://github.com/Rust-for-Linux/linux/issues/1145
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Co-developed-by: Grace Deng <Grace.Deng006@Gmail.com>
Signed-off-by: Grace Deng <Grace.Deng006@Gmail.com>
Signed-off-by: Kunwu Chan <kunwu.chan@hotmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250317025205.2366518-1-kunwu.chan@linux.dev
---
rust/kernel/sync/poll.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index d7e6e59e124b..7b973d72229b 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -107,6 +107,7 @@ fn deref(&self) -> &CondVar {
#[pinned_drop]
impl PinnedDrop for PollCondVar {
+ #[inline]
fn drop(self: Pin<&mut Self>) {
// Clear anything registered using `register_wait`.
//
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] rust: task: Mark Task methods inline
2025-05-06 4:58 [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16 Boqun Feng
2025-05-06 4:58 ` [PATCH 1/5] rust: sync: Mark CondVar::notify_*() inline Boqun Feng
2025-05-06 4:58 ` [PATCH 2/5] rust: sync: Mark PollCondVar::drop() inline Boqun Feng
@ 2025-05-06 4:58 ` Boqun Feng
2025-05-06 4:58 ` [PATCH 4/5] sched/core: Add __might_sleep_precision() Boqun Feng
2025-05-06 4:58 ` [PATCH 5/5] rust: task: Add Rust version of might_sleep() Boqun Feng
4 siblings, 0 replies; 12+ messages in thread
From: Boqun Feng @ 2025-05-06 4:58 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Christian Schrefl, Charalampos Mitrodimas
From: Panagiotis Foliadis <pfoliadis@posteo.net>
When building the kernel using the llvm-18.1.3-rust-1.85.0-x86_64
toolchain provided by kernel.org, the following symbols are generated:
$ nm vmlinux | grep ' _R'.*Task | rustfilt
... T <kernel::task::Task>::get_pid_ns
... T <kernel::task::Task>::tgid_nr_ns
... T <kernel::task::Task>::current_pid_ns
... T <kernel::task::Task>::signal_pending
... T <kernel::task::Task>::uid
... T <kernel::task::Task>::euid
... T <kernel::task::Task>::current
... T <kernel::task::Task>::wake_up
... T <kernel::task::Task as kernel::types::AlwaysRefCounted>::dec_ref
... T <kernel::task::Task as kernel::types::AlwaysRefCounted>::inc_ref
These Rust symbols are trivial wrappers around the C functions. It
doesn't make sense to go through a trivial wrapper for these functions,
so mark them inline.
[boqun: Capitalize the title, reword a bit to avoid listing all the C
functions as the code already shows them and remove the addresses of the
symbols in the commit log as they are different from build to build.]
Link: https://github.com/Rust-for-Linux/linux/issues/1145
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Christian Schrefl <chrisi.schrefl@gmail.com>
Reviewed-by: Charalampos Mitrodimas <charmitro@posteo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Panagiotis Foliadis <pfoliadis@posteo.net>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250315-inline-c-wrappers-v3-1-048e43fcef7d@posteo.net
---
rust/kernel/task.rs | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 9e6f6854948d..0bf5fdf75c37 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -134,6 +134,7 @@ pub fn current_raw() -> *mut bindings::task_struct {
/// # Safety
///
/// Callers must ensure that the returned object doesn't outlive the current task/thread.
+ #[inline]
pub unsafe fn current() -> impl Deref<Target = Task> {
struct TaskRef<'a> {
task: &'a Task,
@@ -168,6 +169,7 @@ fn deref(&self) -> &Self::Target {
/// # Safety
///
/// Callers must ensure that the returned object doesn't outlive the current task/thread.
+ #[inline]
pub unsafe fn current_pid_ns() -> impl Deref<Target = PidNamespace> {
struct PidNamespaceRef<'a> {
task: &'a PidNamespace,
@@ -275,24 +277,28 @@ pub fn pid(&self) -> Pid {
}
/// Returns the UID of the given task.
+ #[inline]
pub fn uid(&self) -> Kuid {
// SAFETY: It's always safe to call `task_uid` on a valid task.
Kuid::from_raw(unsafe { bindings::task_uid(self.as_ptr()) })
}
/// Returns the effective UID of the given task.
+ #[inline]
pub fn euid(&self) -> Kuid {
// SAFETY: It's always safe to call `task_euid` on a valid task.
Kuid::from_raw(unsafe { bindings::task_euid(self.as_ptr()) })
}
/// Determines whether the given task has pending signals.
+ #[inline]
pub fn signal_pending(&self) -> bool {
// SAFETY: It's always safe to call `signal_pending` on a valid task.
unsafe { bindings::signal_pending(self.as_ptr()) != 0 }
}
/// Returns task's pid namespace with elevated reference count
+ #[inline]
pub fn get_pid_ns(&self) -> Option<ARef<PidNamespace>> {
// SAFETY: By the type invariant, we know that `self.0` is valid.
let ptr = unsafe { bindings::task_get_pid_ns(self.as_ptr()) };
@@ -308,6 +314,7 @@ pub fn get_pid_ns(&self) -> Option<ARef<PidNamespace>> {
/// Returns the given task's pid in the provided pid namespace.
#[doc(alias = "task_tgid_nr_ns")]
+ #[inline]
pub fn tgid_nr_ns(&self, pidns: Option<&PidNamespace>) -> Pid {
let pidns = match pidns {
Some(pidns) => pidns.as_ptr(),
@@ -321,6 +328,7 @@ pub fn tgid_nr_ns(&self, pidns: Option<&PidNamespace>) -> Pid {
}
/// Wakes up the task.
+ #[inline]
pub fn wake_up(&self) {
// SAFETY: It's always safe to call `wake_up_process` on a valid task, even if the task
// running.
@@ -330,11 +338,13 @@ pub fn wake_up(&self) {
// SAFETY: The type invariants guarantee that `Task` is always refcounted.
unsafe impl crate::types::AlwaysRefCounted for Task {
+ #[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
unsafe { bindings::get_task_struct(self.as_ptr()) };
}
+ #[inline]
unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
// SAFETY: The safety requirements guarantee that the refcount is nonzero.
unsafe { bindings::put_task_struct(obj.cast().as_ptr()) }
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-06 4:58 [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16 Boqun Feng
` (2 preceding siblings ...)
2025-05-06 4:58 ` [PATCH 3/5] rust: task: Mark Task methods inline Boqun Feng
@ 2025-05-06 4:58 ` Boqun Feng
2025-05-09 6:00 ` Ingo Molnar
2025-05-06 4:58 ` [PATCH 5/5] rust: task: Add Rust version of might_sleep() Boqun Feng
4 siblings, 1 reply; 12+ messages in thread
From: Boqun Feng @ 2025-05-06 4:58 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida
From: FUJITA Tomonori <fujita.tomonori@gmail.com>
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>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250410225623.152616-2-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 79692f85643f..496bd462d29e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8732,24 +8732,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))
@@ -8771,7 +8753,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;
@@ -8794,8 +8777,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);
@@ -8820,8 +8803,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.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] rust: task: Add Rust version of might_sleep()
2025-05-06 4:58 [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16 Boqun Feng
` (3 preceding siblings ...)
2025-05-06 4:58 ` [PATCH 4/5] sched/core: Add __might_sleep_precision() Boqun Feng
@ 2025-05-06 4:58 ` Boqun Feng
4 siblings, 0 replies; 12+ messages in thread
From: Boqun Feng @ 2025-05-06 4:58 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm
From: FUJITA Tomonori <fujita.tomonori@gmail.com>
Add 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>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250410225623.152616-3-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 0bf5fdf75c37..067546754939 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -390,3 +390,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.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-06 4:58 ` [PATCH 4/5] sched/core: Add __might_sleep_precision() Boqun Feng
@ 2025-05-09 6:00 ` Ingo Molnar
2025-05-09 7:19 ` Boqun Feng
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Ingo Molnar @ 2025-05-09 6:00 UTC (permalink / raw)
To: Boqun Feng
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida, Linus Torvalds
* Boqun Feng <boqun.feng@gmail.com> wrote:
> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> 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>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> Link: https://lore.kernel.org/r/20250410225623.152616-2-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);
Ugh.
Firstly, '_precision' is really ambiguous in this context and suggests
'precise sleep' or something like that, which this is not about at all.
So the naming here is all sorts of bad already.
But more importantly, this is really a Rust problem. Does Rust really
have no NUL-terminated strings? It should hide them in shame and
construct proper, robust strings, instead of spreading this disease to
the rest of the kernel, IMHO ...
Rust is supposed to be about increased security, right? How does extra,
nonsensical complexity for simple concepts such as strings achieve
that? If the Rust runtime wants to hook into debug facilities of the
Linux kernel then I have bad news: almost all strings used by kernel
debugging facilities are NUL-terminated.
So I really don't like this patch. Is there no other way to do this?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-09 6:00 ` Ingo Molnar
@ 2025-05-09 7:19 ` Boqun Feng
2025-05-19 12:40 ` Boqun Feng
2025-05-09 7:41 ` Andreas Hindborg
2025-05-09 9:20 ` Alice Ryhl
2 siblings, 1 reply; 12+ messages in thread
From: Boqun Feng @ 2025-05-09 7:19 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida, Linus Torvalds
On Fri, May 09, 2025 at 08:00:32AM +0200, Ingo Molnar wrote:
>
> * Boqun Feng <boqun.feng@gmail.com> wrote:
>
> > From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >
> > 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>
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > Link: https://lore.kernel.org/r/20250410225623.152616-2-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);
>
> Ugh.
>
> Firstly, '_precision' is really ambiguous in this context and suggests
> 'precise sleep' or something like that, which this is not about at all.
> So the naming here is all sorts of bad already.
>
I accept this is not a good naming.
> But more importantly, this is really a Rust problem. Does Rust really
> have no NUL-terminated strings? It should hide them in shame and
You can create NUL-terminated strings in Rust of course, but in this
case, because we want to use the "#[trace_caller]" attribute [1], which
allows might_sleep() in Rust to be defined as a function, and can use
Location::caller() to get the caller file and line number information,
and `Location` type yet doesn't return a Nul-terminated string literal,
so we have to work this around.
> construct proper, robust strings, instead of spreading this disease to
> the rest of the kernel, IMHO ...
>
> Rust is supposed to be about increased security, right? How does extra,
> nonsensical complexity for simple concepts such as strings achieve
> that? If the Rust runtime wants to hook into debug facilities of the
> Linux kernel then I have bad news: almost all strings used by kernel
> debugging facilities are NUL-terminated.
This is more of a special case because `Location` is used (i.e. file
name is the string literal). For things like user-defined string, we use
the macro c_str!(), which generates NUL-terminated strings. For example,
lockdep class names.
>
> So I really don't like this patch. Is there no other way to do this?
>
There's a `c_str` [2] macro which could generates a NUL-terminated
string, but using that will requires might_sleep() defined as a macro as
well. Given that might_sleep() is the user interface that most users
will use, and how it handles string literal for file names is an
implementation detail, so I figured it's better we resolve in the
current way.
[1]: https://rustc-dev-guide.rust-lang.org/backend/implicit-caller-location.html
[2]: https://rust.docs.kernel.org/kernel/macro.c_str.html
Regards,
Boqun
> Thanks,
>
> Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-09 6:00 ` Ingo Molnar
2025-05-09 7:19 ` Boqun Feng
@ 2025-05-09 7:41 ` Andreas Hindborg
2025-05-09 9:20 ` Alice Ryhl
2 siblings, 0 replies; 12+ messages in thread
From: Andreas Hindborg @ 2025-05-09 7:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: Boqun Feng, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, Miguel Ojeda, Alex Gaynor,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida, Linus Torvalds
Ingo Molnar <mingo@kernel.org> writes:
> * Boqun Feng <boqun.feng@gmail.com> wrote:
>
>> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>
>> 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>
>> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
>> Link: https://lore.kernel.org/r/20250410225623.152616-2-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);
>
> Ugh.
>
> Firstly, '_precision' is really ambiguous in this context and suggests
> 'precise sleep' or something like that, which this is not about at all.
> So the naming here is all sorts of bad already.
>
> But more importantly, this is really a Rust problem. Does Rust really
> have no NUL-terminated strings? It should hide them in shame and
> construct proper, robust strings, instead of spreading this disease to
> the rest of the kernel, IMHO ...
Not to discuss this particular patch or language interop in general, but
I am curious why you think that null terminated strings are more robust
than pointer + length strings? From the paragraph below, it seems you
also believe that pointer + length is worse for security. Why is that?
Best regards,
Andreas Hindborg
>
> Rust is supposed to be about increased security, right? How does extra,
> nonsensical complexity for simple concepts such as strings achieve
> that? If the Rust runtime wants to hook into debug facilities of the
> Linux kernel then I have bad news: almost all strings used by kernel
> debugging facilities are NUL-terminated.
>
> So I really don't like this patch. Is there no other way to do this?
>
> Thanks,
>
> Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-09 6:00 ` Ingo Molnar
2025-05-09 7:19 ` Boqun Feng
2025-05-09 7:41 ` Andreas Hindborg
@ 2025-05-09 9:20 ` Alice Ryhl
2 siblings, 0 replies; 12+ messages in thread
From: Alice Ryhl @ 2025-05-09 9:20 UTC (permalink / raw)
To: Ingo Molnar
Cc: Boqun Feng, Peter Zijlstra, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, Miguel Ojeda, Alex Gaynor,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida, Linus Torvalds
On Fri, May 09, 2025 at 08:00:32AM +0200, Ingo Molnar wrote:
>
> * Boqun Feng <boqun.feng@gmail.com> wrote:
>
> > From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >
> > 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>
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > Link: https://lore.kernel.org/r/20250410225623.152616-2-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);
>
> Ugh.
>
> Firstly, '_precision' is really ambiguous in this context and suggests
> 'precise sleep' or something like that, which this is not about at all.
> So the naming here is all sorts of bad already.
>
> But more importantly, this is really a Rust problem. Does Rust really
> have no NUL-terminated strings? It should hide them in shame and
> construct proper, robust strings, instead of spreading this disease to
> the rest of the kernel, IMHO ...
Rust does have NUL-terminated strings, but they aren't the default. In
most circumstances, obtaining a NUL-terminated string is possible, but
we can't do it in this particular case.
Specifically, it's because we're relying on the #[track_caller] language
feature. When this annotation is placed on a function, it implicitly
adds an extra hidden argument to the function signature containing a
pointer to a location struct that holds the file, line, and column of
the caller. This works recursively until it hits a function without the
annotation, so code like this:
#[track_caller]
fn schedule() {
might_sleep();
// Call into C implementation of schedule.
unsafe { bindings::schedule() };
}
would report a line number in the *caller* of schedule() rather than
just reporting the line number inside the schedule() function.
The problem is that the location struct is generated by the compiler,
and we don't have any control over how its generated. Unfortunately, the
compiler does not place a NUL terminator in the file name.
> Rust is supposed to be about increased security, right? How does extra,
> nonsensical complexity for simple concepts such as strings achieve
> that? If the Rust runtime wants to hook into debug facilities of the
> Linux kernel then I have bad news: almost all strings used by kernel
> debugging facilities are NUL-terminated.
>
> So I really don't like this patch. Is there no other way to do this?
I filed a proposal to add a NUL-terminator to the file names used by
rustc-generated location structs:
https://github.com/rust-lang/libs-team/issues/466
But I have not had success with landing it, unfortunately.
Alice
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-09 7:19 ` Boqun Feng
@ 2025-05-19 12:40 ` Boqun Feng
2025-06-02 18:16 ` Boqun Feng
0 siblings, 1 reply; 12+ messages in thread
From: Boqun Feng @ 2025-05-19 12:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida, Linus Torvalds
On Fri, May 09, 2025 at 12:19:03AM -0700, Boqun Feng wrote:
> On Fri, May 09, 2025 at 08:00:32AM +0200, Ingo Molnar wrote:
> >
> > * Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > > From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > >
> > > 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>
> > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > Link: https://lore.kernel.org/r/20250410225623.152616-2-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);
> >
> > Ugh.
> >
> > Firstly, '_precision' is really ambiguous in this context and suggests
> > 'precise sleep' or something like that, which this is not about at all.
> > So the naming here is all sorts of bad already.
> >
>
> I accept this is not a good naming.
>
> > But more importantly, this is really a Rust problem. Does Rust really
> > have no NUL-terminated strings? It should hide them in shame and
>
> You can create NUL-terminated strings in Rust of course, but in this
> case, because we want to use the "#[trace_caller]" attribute [1], which
> allows might_sleep() in Rust to be defined as a function, and can use
> Location::caller() to get the caller file and line number information,
> and `Location` type yet doesn't return a Nul-terminated string literal,
> so we have to work this around.
>
> > construct proper, robust strings, instead of spreading this disease to
> > the rest of the kernel, IMHO ...
> >
> > Rust is supposed to be about increased security, right? How does extra,
> > nonsensical complexity for simple concepts such as strings achieve
> > that? If the Rust runtime wants to hook into debug facilities of the
> > Linux kernel then I have bad news: almost all strings used by kernel
> > debugging facilities are NUL-terminated.
>
> This is more of a special case because `Location` is used (i.e. file
> name is the string literal). For things like user-defined string, we use
> the macro c_str!(), which generates NUL-terminated strings. For example,
> lockdep class names.
>
> >
> > So I really don't like this patch. Is there no other way to do this?
> >
>
Trying to see if we can make some forward-progress on this one,
considering:
1. #[track_caller] is really a desired feature to be used for Rust's
might_sleep(), Alice's reply [3] also explains a bit more on the
"why" part.
2. To achieve #1, we will need to handle the file name returned by
Rust's `Location` struct, especially Location::file() will return a
string literal without a tailing NUL.
3. Other than the current approach proposed by this patch, if the
existing might_sleep() functionality does not couple (task) state
inquiries with debug printing, we can maybe avoid printing the
non-NUL-terminated string in C's __might_sleep*() function by
printing Location::file() in Rust code:
#[track_caller]
fn might_sleep() {
let loc = Location::caller();
if (__might_sleep_is_violated()) {
pr_err!("BUG: sleeping function called from invalid context at {loc}\n");
...
}
}
but this essentially would add more changes into C code compared to
the current patch.
4. This is only a special case where we need the "debug information"
provided by Rust, so this won't happen a lot; and printing a
non-NUL-terminated string is already supported by printk already, so
we reuse what kernel already has here.
Given the above, I think the current patch is the best solution.
[3]: https://lore.kernel.org/lkml/aB3I62o8hWSULGBm@google.com/
Regards,
Boqun
> There's a `c_str` [2] macro which could generates a NUL-terminated
> string, but using that will requires might_sleep() defined as a macro as
> well. Given that might_sleep() is the user interface that most users
> will use, and how it handles string literal for file names is an
> implementation detail, so I figured it's better we resolve in the
> current way.
>
> [1]: https://rustc-dev-guide.rust-lang.org/backend/implicit-caller-location.html
> [2]: https://rust.docs.kernel.org/kernel/macro.c_str.html
>
> Regards,
> Boqun
>
> > Thanks,
> >
> > Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] sched/core: Add __might_sleep_precision()
2025-05-19 12:40 ` Boqun Feng
@ 2025-06-02 18:16 ` Boqun Feng
0 siblings, 0 replies; 12+ messages in thread
From: Boqun Feng @ 2025-06-02 18:16 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, FUJITA Tomonori,
Tamir Duberstein, Kunwu Chan, Mitchell Levy,
Martin Rodriguez Reboredo, Borys Tyran, Christian Brauner,
Panagiotis Foliadis, linux-kernel, rust-for-linux, llvm,
Daniel Almeida, Linus Torvalds
On Mon, May 19, 2025 at 05:40:51AM -0700, Boqun Feng wrote:
> On Fri, May 09, 2025 at 12:19:03AM -0700, Boqun Feng wrote:
> > On Fri, May 09, 2025 at 08:00:32AM +0200, Ingo Molnar wrote:
> > >
> > > * Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > > From: FUJITA Tomonori <fujita.tomonori@gmail.com>
> > > >
> > > > 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>
> > > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > > > Link: https://lore.kernel.org/r/20250410225623.152616-2-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);
> > >
> > > Ugh.
> > >
> > > Firstly, '_precision' is really ambiguous in this context and suggests
> > > 'precise sleep' or something like that, which this is not about at all.
> > > So the naming here is all sorts of bad already.
> > >
> >
> > I accept this is not a good naming.
> >
> > > But more importantly, this is really a Rust problem. Does Rust really
> > > have no NUL-terminated strings? It should hide them in shame and
> >
> > You can create NUL-terminated strings in Rust of course, but in this
> > case, because we want to use the "#[trace_caller]" attribute [1], which
> > allows might_sleep() in Rust to be defined as a function, and can use
> > Location::caller() to get the caller file and line number information,
> > and `Location` type yet doesn't return a Nul-terminated string literal,
> > so we have to work this around.
> >
> > > construct proper, robust strings, instead of spreading this disease to
> > > the rest of the kernel, IMHO ...
> > >
> > > Rust is supposed to be about increased security, right? How does extra,
> > > nonsensical complexity for simple concepts such as strings achieve
> > > that? If the Rust runtime wants to hook into debug facilities of the
> > > Linux kernel then I have bad news: almost all strings used by kernel
> > > debugging facilities are NUL-terminated.
> >
> > This is more of a special case because `Location` is used (i.e. file
> > name is the string literal). For things like user-defined string, we use
> > the macro c_str!(), which generates NUL-terminated strings. For example,
> > lockdep class names.
> >
> > >
> > > So I really don't like this patch. Is there no other way to do this?
> > >
> >
>
> Trying to see if we can make some forward-progress on this one,
> considering:
>
> 1. #[track_caller] is really a desired feature to be used for Rust's
> might_sleep(), Alice's reply [3] also explains a bit more on the
> "why" part.
>
> 2. To achieve #1, we will need to handle the file name returned by
> Rust's `Location` struct, especially Location::file() will return a
> string literal without a tailing NUL.
>
> 3. Other than the current approach proposed by this patch, if the
> existing might_sleep() functionality does not couple (task) state
> inquiries with debug printing, we can maybe avoid printing the
> non-NUL-terminated string in C's __might_sleep*() function by
> printing Location::file() in Rust code:
>
> #[track_caller]
> fn might_sleep() {
> let loc = Location::caller();
>
> if (__might_sleep_is_violated()) {
> pr_err!("BUG: sleeping function called from invalid context at {loc}\n");
>
> ...
> }
> }
>
> but this essentially would add more changes into C code compared to
> the current patch.
>
> 4. This is only a special case where we need the "debug information"
> provided by Rust, so this won't happen a lot; and printing a
> non-NUL-terminated string is already supported by printk already, so
> we reuse what kernel already has here.
>
> Given the above, I think the current patch is the best solution.
>
Ingo,
Alice made some progress on providing the NUL-terminated string for `Location`
[4] [5], which means in the future, we can avoid the __might_sleep_precision()
workaround here, and yet remain the benefit of `#[track_caller]` (Thanks
Alice!). This also means we'd better keep the workaround right now, because that
keeps the same interface if we have NUL-terminated string from
`Location::file()`. And we can revert this workaround easily when the feature is
available in Rust. So I think we should take this.
Moving forwards, let me know if you need me to resend the pull request (there
are also a very trivial improvements in it as well), and I could rename
__might_sleep_precision() to something else (like __might_sleep_nonnulfilename()
or anything) in the resend. Thoughts?
[4]: https://github.com/rust-lang/libs-team/issues/466#issuecomment-2914476468
[5]: https://github.com/rust-lang/rust/issues/141727
Regards,
Boqun
> [3]: https://lore.kernel.org/lkml/aB3I62o8hWSULGBm@google.com/
>
> Regards,
> Boqun
>
> > There's a `c_str` [2] macro which could generates a NUL-terminated
> > string, but using that will requires might_sleep() defined as a macro as
> > well. Given that might_sleep() is the user interface that most users
> > will use, and how it handles string literal for file names is an
> > implementation detail, so I figured it's better we resolve in the
> > current way.
> >
> > [1]: https://rustc-dev-guide.rust-lang.org/backend/implicit-caller-location.html
> > [2]: https://rust.docs.kernel.org/kernel/macro.c_str.html
> >
> > Regards,
> > Boqun
> >
> > > Thanks,
> > >
> > > Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-06-02 18:16 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-06 4:58 [GIT PULL] [PATCH 0/5] rust: Task & schedule related changes for v6.16 Boqun Feng
2025-05-06 4:58 ` [PATCH 1/5] rust: sync: Mark CondVar::notify_*() inline Boqun Feng
2025-05-06 4:58 ` [PATCH 2/5] rust: sync: Mark PollCondVar::drop() inline Boqun Feng
2025-05-06 4:58 ` [PATCH 3/5] rust: task: Mark Task methods inline Boqun Feng
2025-05-06 4:58 ` [PATCH 4/5] sched/core: Add __might_sleep_precision() Boqun Feng
2025-05-09 6:00 ` Ingo Molnar
2025-05-09 7:19 ` Boqun Feng
2025-05-19 12:40 ` Boqun Feng
2025-06-02 18:16 ` Boqun Feng
2025-05-09 7:41 ` Andreas Hindborg
2025-05-09 9:20 ` Alice Ryhl
2025-05-06 4:58 ` [PATCH 5/5] rust: task: Add Rust version of might_sleep() Boqun Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).