Re: [PATCH v11 7/8] rust: Add read_poll_timeout functions

FUJITA Tomonori
2025-08-11T01:53:40Z

Sorry, I somehow missed this email.

On Sat, 22 Mar 2025 17:02:31 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

>> +/// 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.
>
> I don't think this safety comment is sufficient. There are two other
> similar comments further down.

Updated the comment.

>> +/// ```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), Some(Delta::from_micros(42)));
>> +/// drop(g);
>> +///
>> +/// # Ok::<(), Error>(())
>> +/// ```
>
> I am guessing this example is present to test the call to `might_sleep`.

I also guess so. Boqun wrote this test, IIRC.

> Could you document the reason for the test. As an example, this code is
> not really usable. `#[test]` was staged for 6.15, so perhaps move this
> to a unit test instead?
>
> The test throws this BUG, which is what I think is also your intention:

might_sleep() doesn't throw BUG(), just a warning. Can the test
infrastructure handle such?