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

Andreas Hindborg
2025-08-11T09:42:56Z

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> 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?

As I wrote, kunit does not handle this. But I am confused about the
bug/warn comment. The trace I pasted clearly says "BUG"?

I think we should just remove this test for now.

Best regards,
Andreas Hindborg