Add read_poll_timeout functions which poll periodically until a
condition is met or a timeout is reached.
C's read_poll_timeout (include/linux/iopoll.h) is a complicated macro
and a simple wrapper for Rust doesn't work. So this implements the
same functionality in Rust.
The C version uses usleep_range() while the Rust version uses
fsleep(), which uses the best sleep method so it works with spans that
usleep_range() doesn't work nicely with.
Unlike the C version, __might_sleep() is used instead of might_sleep()
to show proper debug info; the file name and line
number. might_resched() could be added to match what the C version
does but this function works without it.
The sleep_before_read argument isn't supported since there is no user
for now. It's rarely used in the C version.
For the proper debug info, readx_poll_timeout() and __might_sleep()
are implemented as a macro. We could implement them as a normal
function if there is a clean way to get a null-terminated string
without allocation from core::panic::Location::file().
readx_poll_timeout() can only be used in a nonatomic context. This
requirement is not checked by these abstractions, but it is
intended that klint [1] or a similar tool will be used to check it
in the future.
Link: https://rust-for-linux.com/klint [1]
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/helpers/helpers.c | 1 +
rust/helpers/kernel.c | 13 ++++++
rust/kernel/error.rs | 1 +
rust/kernel/io.rs | 5 +++
rust/kernel/io/poll.rs | 95 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
rust/kernel/processor.rs | 13 ++++++
7 files changed, 130 insertions(+)
create mode 100644 rust/helpers/kernel.c
create mode 100644 rust/kernel/io.rs
create mode 100644 rust/kernel/io/poll.rs
create mode 100644 rust/kernel/processor.rs
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index c274546bcf78..f9569ff1717e 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -12,6 +12,7 @@
#include "build_assert.c"
#include "build_bug.c"
#include "err.c"
+#include "kernel.c"
#include "kunit.c"
#include "mutex.c"
#include "page.c"
diff --git a/rust/helpers/kernel.c b/rust/helpers/kernel.c
new file mode 100644
index 000000000000..da847059260b
--- /dev/null
+++ b/rust/helpers/kernel.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+
+void rust_helper_cpu_relax(void)
+{
+ cpu_relax();
+}
+
+void rust_helper___might_sleep(const char *file, int line)
+{
+ __might_sleep(file, line);
+}
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 6f1587a2524e..d571b9587ed6 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -58,6 +58,7 @@ macro_rules! declare_err {
declare_err!(EPIPE, "Broken pipe.");
declare_err!(EDOM, "Math argument out of domain of func.");
declare_err!(ERANGE, "Math result not representable.");
+ declare_err!(ETIMEDOUT, "Connection timed out.");
declare_err!(ERESTARTSYS, "Restart the system call.");
declare_err!(ERESTARTNOINTR, "System call was interrupted by a signal and will be restarted.");
declare_err!(ERESTARTNOHAND, "Restart if no handler.");
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
new file mode 100644
index 000000000000..033f3c4e4adf
--- /dev/null
+++ b/rust/kernel/io.rs
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Input and Output.
+
+pub mod poll;
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
new file mode 100644
index 000000000000..a8caa08f86f2
--- /dev/null
+++ b/rust/kernel/io/poll.rs
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! IO polling.
+//!
+//! C header: [`include/linux/iopoll.h`](srctree/include/linux/iopoll.h).
+
+use crate::{
+ error::{code::*, Result},
+ processor::cpu_relax,
+ time::{delay::fsleep, Delta, Instant},
+};
+
+/// Polls periodically until a condition is met or a timeout is reached.
+///
+/// Public but hidden since it should only be used from public macros.
+#[doc(hidden)]
+pub fn read_poll_timeout<Op, Cond, T: Copy>(
+ mut op: Op,
+ cond: Cond,
+ sleep_delta: Delta,
+ timeout_delta: Delta,
+) -> Result<T>
+where
+ Op: FnMut() -> Result<T>,
+ Cond: Fn(T) -> bool,
+{
+ let start = Instant::now();
+ let sleep = !sleep_delta.is_zero();
+ let timeout = !timeout_delta.is_zero();
+
+ let val = loop {
+ let val = op()?;
+ if cond(val) {
+ // Unlike the C version, we immediately return.
+ // We know a condition is met so we don't need to check again.
+ return Ok(val);
+ }
+ if timeout && start.elapsed() > timeout_delta {
+ // Should we return Err(ETIMEDOUT) here instead of call op() again
+ // without a sleep between? But we follow the C version. op() could
+ // take some time so might be worth checking again.
+ break op()?;
+ }
+ if sleep {
+ fsleep(sleep_delta);
+ }
+ // fsleep() could be busy-wait loop so we always call cpu_relax().
+ cpu_relax();
+ };
+
+ if cond(val) {
+ Ok(val)
+ } else {
+ Err(ETIMEDOUT)
+ }
+}
+
+/// Print debug information if it's called inside atomic sections.
+///
+/// Equivalent to the kernel's [`__might_sleep`].
+#[macro_export]
+macro_rules! __might_sleep {
+ () => {
+ #[cfg(CONFIG_DEBUG_ATOMIC_SLEEP)]
+ // SAFETY: FFI call.
+ unsafe {
+ $crate::bindings::__might_sleep(
+ c_str!(::core::file!()).as_char_ptr(),
+ ::core::line!() as i32,
+ )
+ }
+ };
+}
+
+/// Polls periodically until a condition is met or a timeout is reached.
+///
+/// `op` is called repeatedly until `cond` returns `true` or the timeout is
+/// reached. The return value of `op` is passed to `cond`.
+///
+/// `sleep_delta` is the duration to sleep between calls to `op`.
+/// If `sleep_delta` is less than one microsecond, the function will busy-wait.
+///
+/// `timeout_delta` is the maximum time to wait for `cond` to return `true`.
+///
+/// This macro can only be used in a nonatomic context.
+#[macro_export]
+macro_rules! readx_poll_timeout {
+ ($op:expr, $cond:expr, $sleep_delta:expr, $timeout_delta:expr) => {{
+ if !$sleep_delta.is_zero() {
+ $crate::__might_sleep!();
+ }
+
+ $crate::io::poll::read_poll_timeout($op, $cond, $sleep_delta, $timeout_delta)
+ }};
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 22a3bfa5a9e9..b775fd1c9be0 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -35,6 +35,7 @@
#[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
pub mod firmware;
pub mod init;
+pub mod io;
pub mod ioctl;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
@@ -44,6 +45,7 @@
pub mod page;
pub mod prelude;
pub mod print;
+pub mod processor;
pub mod sizes;
pub mod rbtree;
mod static_assert;
diff --git a/rust/kernel/processor.rs b/rust/kernel/processor.rs
new file mode 100644
index 000000000000..eeeff4be84fa
--- /dev/null
+++ b/rust/kernel/processor.rs
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Processor related primitives.
+//!
+//! C header: [`include/linux/processor.h`](srctree/include/linux/processor.h).
+
+/// Lower CPU power consumption or yield to a hyperthreaded twin processor.
+///
+/// It also happens to serve as a compiler barrier.
+pub fn cpu_relax() {
+ // SAFETY: FFI call.
+ unsafe { bindings::cpu_relax() }
+}
--
2.43.0On Fri, Nov 01 2024, FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > For the proper debug info, readx_poll_timeout() and __might_sleep() > are implemented as a macro. We could implement them as a normal > function if there is a clean way to get a null-terminated string > without allocation from core::panic::Location::file(). Would it be too much to hope for either a compiler flag or simply default behaviour for having the backing, static store of the file!() &str being guaranteed to be followed by a nul character? (Of course that nul should not be counted in the slice's length). That would in general increase interop with C code. This is hardly the last place where Rust code would pass Location::file() into C, and having to pass that as a (ptr,len) pair always and updating the receiving C code to use %.*s seems like an uphill battle, especially when the C code passes the const char* pointer through a few layers before it is finally passed to a printf-like function. And creating the nul-terminated strings with c_str! needlessly doubles the storage needed for the file names (unless the rust compiler is smart enough to then re-use the c_str result for the backing store of the file!() &str). Rasmus
On Thu, Nov 7, 2024 at 1:50 PM Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> Would it be too much to hope for either a compiler flag or simply
> default behaviour for having the backing, static store of the file!()
> &str being guaranteed to be followed by a nul character? (Of course that
> nul should not be counted in the slice's length). That would in general
> increase interop with C code.
Definitely -- please see the "`c_stringify!`, `c_concat!`, `c_file!`
macros" item in:
https://github.com/Rust-for-Linux/linux/issues/514
Relatedly, for `Location`, there is "C string equivalents
(nul-terminated) for `core::panic::Location`.", with this ACP:
https://github.com/rust-lang/libs-team/issues/466
Cheers,
Miguel
On Thu, Nov 7, 2024 at 1:50 PM Rasmus Villemoes <ravi@prevas.dk> wrote: > > On Fri, Nov 01 2024, FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > > For the proper debug info, readx_poll_timeout() and __might_sleep() > > are implemented as a macro. We could implement them as a normal > > function if there is a clean way to get a null-terminated string > > without allocation from core::panic::Location::file(). > > Would it be too much to hope for either a compiler flag or simply > default behaviour for having the backing, static store of the file!() > &str being guaranteed to be followed by a nul character? (Of course that > nul should not be counted in the slice's length). That would in general > increase interop with C code. > > This is hardly the last place where Rust code would pass > Location::file() into C, and having to pass that as a (ptr,len) pair > always and updating the receiving C code to use %.*s seems like an > uphill battle, especially when the C code passes the const char* pointer > through a few layers before it is finally passed to a printf-like > function. This is actively being discussed at: https://github.com/rust-lang/libs-team/issues/466 > And creating the nul-terminated strings with c_str! needlessly doubles > the storage needed for the file names (unless the rust compiler is smart > enough to then re-use the c_str result for the backing store of the > file!() &str). For the case of c_str!(file!()), the compiler should do the right thing. Not via deduplication, but via removal of unused globals. Alice
On Fri, Nov 01, 2024 at 10:01:20AM +0900, FUJITA Tomonori wrote:
> Add read_poll_timeout functions which poll periodically until a
> condition is met or a timeout is reached.
>
> C's read_poll_timeout (include/linux/iopoll.h) is a complicated macro
> and a simple wrapper for Rust doesn't work. So this implements the
> same functionality in Rust.
>
> The C version uses usleep_range() while the Rust version uses
> fsleep(), which uses the best sleep method so it works with spans that
> usleep_range() doesn't work nicely with.
>
> Unlike the C version, __might_sleep() is used instead of might_sleep()
> to show proper debug info; the file name and line
> number. might_resched() could be added to match what the C version
> does but this function works without it.
>
> The sleep_before_read argument isn't supported since there is no user
> for now. It's rarely used in the C version.
>
> For the proper debug info, readx_poll_timeout() and __might_sleep()
> are implemented as a macro. We could implement them as a normal
> function if there is a clean way to get a null-terminated string
> without allocation from core::panic::Location::file().
>
So printk() actually support printing a string with a precison value,
that is: a format string "%.*s" would take two inputs, one for the length
and the other for the pointer to the string, for example you can do:
char *msg = "hello";
printk("%.*s\n", 5, msg);
This is similar to printf() in glibc [1].
If we add another __might_sleep_precision() which accepts a file name
length:
void __might_sleep_precision(const char *file, int len, int line)
then we don't need to use macro here, I've attached a diff based
on your whole patchset, and it seems working.
Cc printk folks to if they know any limitation on using precision.
Regards,
Boqun
[1]: https://www.gnu.org/software/libc/manual/html_node/Output-Conversion-Syntax.html#Output-Conversion-Syntax
> readx_poll_timeout() can only be used in a nonatomic context. This
> requirement is not checked by these abstractions, but it is
> intended that klint [1] or a similar tool will be used to check it
> in the future.
>
> Link: https://rust-for-linux.com/klint [1]
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
--------------------------------------------->8
diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs
index dabb772c468f..4d368ce80db6 100644
--- a/drivers/net/phy/qt2025.rs
+++ b/drivers/net/phy/qt2025.rs
@@ -18,7 +18,7 @@
Driver,
};
use kernel::prelude::*;
-use kernel::readx_poll_timeout;
+use kernel::read_poll_timeout;
use kernel::sizes::{SZ_16K, SZ_8K};
use kernel::time::Delta;
@@ -95,7 +95,7 @@ fn probe(dev: &mut phy::Device) -> Result<()> {
// The micro-controller will start running from SRAM.
dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?;
- readx_poll_timeout!(
+ read_poll_timeout(
|| dev.read(C45::new(Mmd::PCS, 0xd7fd)),
|val| val != 0x00 && val != 0x10,
Delta::from_millis(50),
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index be2e8c0a187e..b405b0d19bac 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -87,6 +87,8 @@ 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_resched_precision(const char *file, int len, int line, unsigned int offsets);
+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);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 43e453ab7e20..f872aa18eaf0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8543,7 +8543,7 @@ void __init sched_init(void)
#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
-void __might_sleep(const char *file, int line)
+void __might_sleep_precision(const char *file, int len, int line)
{
unsigned int state = get_current_state();
/*
@@ -8557,7 +8557,14 @@ void __might_sleep(const char *file, int line)
(void *)current->task_state_change,
(void *)current->task_state_change);
- __might_resched(file, line, 0);
+ __might_resched_precision(file, len, line, 0);
+}
+
+void __might_sleep(const char *file, int line)
+{
+ long len = strlen(file);
+
+ __might_sleep_precision(file, len, line);
}
EXPORT_SYMBOL(__might_sleep);
@@ -8582,7 +8589,7 @@ static inline bool resched_offsets_ok(unsigned int offsets)
return nested == offsets;
}
-void __might_resched(const char *file, int line, unsigned int offsets)
+void __might_resched_precision(const char *file, int len, int line, unsigned int offsets)
{
/* Ratelimiting timestamp: */
static unsigned long prev_jiffy;
@@ -8605,8 +8612,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",
+ 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);
@@ -8631,6 +8638,13 @@ void __might_resched(const char *file, int line, unsigned int offsets)
dump_stack();
add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
}
+
+void __might_resched(const char *file, int line, unsigned int offsets)
+{
+ long len = strlen(file);
+
+ __might_resched_precision(file, len, line, offsets);
+}
EXPORT_SYMBOL(__might_resched);
void __cant_sleep(const char *file, int line, int preempt_offset)
diff --git a/rust/helpers/kernel.c b/rust/helpers/kernel.c
index da847059260b..9dff28f4618e 100644
--- a/rust/helpers/kernel.c
+++ b/rust/helpers/kernel.c
@@ -7,7 +7,7 @@ void rust_helper_cpu_relax(void)
cpu_relax();
}
-void rust_helper___might_sleep(const char *file, int line)
+void rust_helper___might_sleep_precision(const char *file, int len, int line)
{
- __might_sleep(file, line);
+ __might_sleep_precision(file, len, line);
}
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
index a8caa08f86f2..d7e5be162b6e 100644
--- a/rust/kernel/io/poll.rs
+++ b/rust/kernel/io/poll.rs
@@ -10,10 +10,25 @@
time::{delay::fsleep, Delta, Instant},
};
+use core::panic::Location;
+
/// Polls periodically until a condition is met or a timeout is reached.
///
/// Public but hidden since it should only be used from public macros.
-#[doc(hidden)]
+///
+/// ```rust
+/// use kernel::io::poll::read_poll_timeout;
+/// use kernel::time::Delta;
+/// use kernel::sync::{SpinLock, new_spinlock};
+///
+/// let lock = KBox::pin_init(new_spinlock!(()), kernel::alloc::flags::GFP_KERNEL)?;
+/// let g = lock.lock();
+/// read_poll_timeout(|| Ok(()), |()| true, Delta::from_micros(42), Delta::from_micros(42));
+/// drop(g);
+///
+/// # Ok::<(), Error>(())
+/// ```
+#[track_caller]
pub fn read_poll_timeout<Op, Cond, T: Copy>(
mut op: Op,
cond: Cond,
@@ -28,6 +43,8 @@ pub fn read_poll_timeout<Op, Cond, T: Copy>(
let sleep = !sleep_delta.is_zero();
let timeout = !timeout_delta.is_zero();
+ might_sleep(Location::caller());
+
let val = loop {
let val = op()?;
if cond(val) {
@@ -55,41 +72,13 @@ pub fn read_poll_timeout<Op, Cond, T: Copy>(
}
}
-/// Print debug information if it's called inside atomic sections.
-///
-/// Equivalent to the kernel's [`__might_sleep`].
-#[macro_export]
-macro_rules! __might_sleep {
- () => {
- #[cfg(CONFIG_DEBUG_ATOMIC_SLEEP)]
- // SAFETY: FFI call.
- unsafe {
- $crate::bindings::__might_sleep(
- c_str!(::core::file!()).as_char_ptr(),
- ::core::line!() as i32,
- )
- }
- };
-}
-
-/// Polls periodically until a condition is met or a timeout is reached.
-///
-/// `op` is called repeatedly until `cond` returns `true` or the timeout is
-/// reached. The return value of `op` is passed to `cond`.
-///
-/// `sleep_delta` is the duration to sleep between calls to `op`.
-/// If `sleep_delta` is less than one microsecond, the function will busy-wait.
-///
-/// `timeout_delta` is the maximum time to wait for `cond` to return `true`.
-///
-/// This macro can only be used in a nonatomic context.
-#[macro_export]
-macro_rules! readx_poll_timeout {
- ($op:expr, $cond:expr, $sleep_delta:expr, $timeout_delta:expr) => {{
- if !$sleep_delta.is_zero() {
- $crate::__might_sleep!();
- }
-
- $crate::io::poll::read_poll_timeout($op, $cond, $sleep_delta, $timeout_delta)
- }};
+fn might_sleep(loc: &Location<'_>) {
+ // SAFETY: FFI call.
+ unsafe {
+ crate::bindings::__might_sleep_precision(
+ loc.file().as_ptr().cast(),
+ loc.file().len() as i32,
+ loc.line() as i32,
+ )
+ }
}
On Wed, 6 Nov 2024 13:35:09 -0800
Boqun Feng <boqun.feng@gmail.com> wrote:
> On Fri, Nov 01, 2024 at 10:01:20AM +0900, FUJITA Tomonori wrote:
>> Add read_poll_timeout functions which poll periodically until a
>> condition is met or a timeout is reached.
>>
>> C's read_poll_timeout (include/linux/iopoll.h) is a complicated macro
>> and a simple wrapper for Rust doesn't work. So this implements the
>> same functionality in Rust.
>>
>> The C version uses usleep_range() while the Rust version uses
>> fsleep(), which uses the best sleep method so it works with spans that
>> usleep_range() doesn't work nicely with.
>>
>> Unlike the C version, __might_sleep() is used instead of might_sleep()
>> to show proper debug info; the file name and line
>> number. might_resched() could be added to match what the C version
>> does but this function works without it.
>>
>> The sleep_before_read argument isn't supported since there is no user
>> for now. It's rarely used in the C version.
>>
>> For the proper debug info, readx_poll_timeout() and __might_sleep()
>> are implemented as a macro. We could implement them as a normal
>> function if there is a clean way to get a null-terminated string
>> without allocation from core::panic::Location::file().
>>
>
> So printk() actually support printing a string with a precison value,
> that is: a format string "%.*s" would take two inputs, one for the length
> and the other for the pointer to the string, for example you can do:
>
> char *msg = "hello";
>
> printk("%.*s\n", 5, msg);
>
> This is similar to printf() in glibc [1].
>
> If we add another __might_sleep_precision() which accepts a file name
> length:
>
> void __might_sleep_precision(const char *file, int len, int line)
>
> then we don't need to use macro here, I've attached a diff based
> on your whole patchset, and it seems working.
Ah, I didn't know this.
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index be2e8c0a187e..b405b0d19bac 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -87,6 +87,8 @@ 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_resched_precision(const char *file, int len, int line, unsigned int offsets);
> +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);
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 43e453ab7e20..f872aa18eaf0 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8543,7 +8543,7 @@ void __init sched_init(void)
>
> #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
>
> -void __might_sleep(const char *file, int line)
> +void __might_sleep_precision(const char *file, int len, int line)
> {
> unsigned int state = get_current_state();
> /*
> @@ -8557,7 +8557,14 @@ void __might_sleep(const char *file, int line)
> (void *)current->task_state_change,
> (void *)current->task_state_change);
>
> - __might_resched(file, line, 0);
> + __might_resched_precision(file, len, line, 0);
> +}
> +
> +void __might_sleep(const char *file, int line)
> +{
> + long len = strlen(file);
> +
> + __might_sleep_precision(file, len, line);
> }
> EXPORT_SYMBOL(__might_sleep);
>
> @@ -8582,7 +8589,7 @@ static inline bool resched_offsets_ok(unsigned int offsets)
> return nested == offsets;
> }
>
> -void __might_resched(const char *file, int line, unsigned int offsets)
> +void __might_resched_precision(const char *file, int len, int line, unsigned int offsets)
> {
> /* Ratelimiting timestamp: */
> static unsigned long prev_jiffy;
> @@ -8605,8 +8612,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",
> + 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);
> @@ -8631,6 +8638,13 @@ void __might_resched(const char *file, int line, unsigned int offsets)
> dump_stack();
> add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
> }
> +
> +void __might_resched(const char *file, int line, unsigned int offsets)
> +{
> + long len = strlen(file);
> +
> + __might_resched_precision(file, len, line, offsets);
> +}
> EXPORT_SYMBOL(__might_resched);
>
> void __cant_sleep(const char *file, int line, int preempt_offset)
Cc scheduler people to ask if they would accept the above change for
Rust or prefer that Rust itself handles the null-terminated string
issue.
On Wed 2024-11-06 13:35:09, Boqun Feng wrote:
> On Fri, Nov 01, 2024 at 10:01:20AM +0900, FUJITA Tomonori wrote:
> > Add read_poll_timeout functions which poll periodically until a
> > condition is met or a timeout is reached.
> >
> > C's read_poll_timeout (include/linux/iopoll.h) is a complicated macro
> > and a simple wrapper for Rust doesn't work. So this implements the
> > same functionality in Rust.
> >
> > The C version uses usleep_range() while the Rust version uses
> > fsleep(), which uses the best sleep method so it works with spans that
> > usleep_range() doesn't work nicely with.
> >
> > Unlike the C version, __might_sleep() is used instead of might_sleep()
> > to show proper debug info; the file name and line
> > number. might_resched() could be added to match what the C version
> > does but this function works without it.
> >
> > The sleep_before_read argument isn't supported since there is no user
> > for now. It's rarely used in the C version.
> >
> > For the proper debug info, readx_poll_timeout() and __might_sleep()
> > are implemented as a macro. We could implement them as a normal
> > function if there is a clean way to get a null-terminated string
> > without allocation from core::panic::Location::file().
> >
>
> So printk() actually support printing a string with a precison value,
> that is: a format string "%.*s" would take two inputs, one for the length
> and the other for the pointer to the string, for example you can do:
>
> char *msg = "hello";
>
> printk("%.*s\n", 5, msg);
>
> This is similar to printf() in glibc [1].
>
> If we add another __might_sleep_precision() which accepts a file name
> length:
>
> void __might_sleep_precision(const char *file, int len, int line)
>
> then we don't need to use macro here, I've attached a diff based
> on your whole patchset, and it seems working.
>
> Cc printk folks to if they know any limitation on using precision.
I am not aware of any printk() limitation here. The "%.*s" format
should work the same way as in printf() in the userspace.
Best Regards,
Petr
On Fri, Nov 01, 2024 at 10:01:20AM +0900, FUJITA Tomonori wrote:
[...]
> @@ -44,6 +45,7 @@
> pub mod page;
> pub mod prelude;
> pub mod print;
> +pub mod processor;
> pub mod sizes;
> pub mod rbtree;
> mod static_assert;
> diff --git a/rust/kernel/processor.rs b/rust/kernel/processor.rs
> new file mode 100644
> index 000000000000..eeeff4be84fa
> --- /dev/null
> +++ b/rust/kernel/processor.rs
What else would we put into this file? `smp_processor_id()` and IPI
functionality? If so, I would probably want to rename this to cpu.rs.
Regards,
Boqun
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Processor related primitives.
> +//!
> +//! C header: [`include/linux/processor.h`](srctree/include/linux/processor.h).
> +
> +/// Lower CPU power consumption or yield to a hyperthreaded twin processor.
> +///
> +/// It also happens to serve as a compiler barrier.
> +pub fn cpu_relax() {
> + // SAFETY: FFI call.
> + unsafe { bindings::cpu_relax() }
> +}
> --
> 2.43.0
>
>
On Wed, 6 Nov 2024 10:18:03 -0800 Boqun Feng <boqun.feng@gmail.com> wrote: > On Fri, Nov 01, 2024 at 10:01:20AM +0900, FUJITA Tomonori wrote: > [...] >> @@ -44,6 +45,7 @@ >> pub mod page; >> pub mod prelude; >> pub mod print; >> +pub mod processor; >> pub mod sizes; >> pub mod rbtree; >> mod static_assert; >> diff --git a/rust/kernel/processor.rs b/rust/kernel/processor.rs >> new file mode 100644 >> index 000000000000..eeeff4be84fa >> --- /dev/null >> +++ b/rust/kernel/processor.rs > > What else would we put into this file? `smp_processor_id()` and IPI > functionality? Yeah, we would need smp_processor_id() but not sure about the other functions. There aren't many processor-related functions that Rust drivers directly need to call, I guess. > If so, I would probably want to rename this to cpu.rs. Fine by me, I'll go with cpu.rs in the next version. I chose processor.rs just because the C side uses processor.h for cpu_relax() but cpu.rs also looks good.
© 2016 - 2024 Red Hat, Inc.