Messages in this thread | | | Date | Tue, 12 Dec 2023 17:05:09 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH 2/2] rust: sync: add `CondVar::wait_timeout` |
| |
On 12/12/23 10:51, Alice Ryhl wrote: > On Fri, Dec 8, 2023 at 8:04 PM Benno Lossin <benno.lossin@proton.me> wrote: >> >> On 12/6/23 11:09, Alice Ryhl wrote: >>> + /// Atomically releases the given lock (whose ownership is proven by the guard) and puts the >>> + /// thread to sleep. It wakes up when notified by [`CondVar::notify_one`] or >>> + /// [`CondVar::notify_all`], or when the thread receives a signal. >>> + /// >>> + /// Returns whether there is a signal pending. >>> + fn wait_internal_timeout<T, B>( >>> + &self, >>> + wait_state: u32, >>> + guard: &mut Guard<'_, T, B>, >>> + timeout: u64, >>> + ) -> u64 >>> + where >>> + T: ?Sized, >>> + B: Backend, >>> + { >>> + let wait = Opaque::<bindings::wait_queue_entry>::uninit(); >>> + >>> + // SAFETY: `wait` points to valid memory. >>> + unsafe { bindings::init_wait(wait.get()) }; >>> + >>> + // SAFETY: Both `wait` and `wait_list` point to valid memory. >>> + unsafe { >>> + bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state as _) >> >> Does `.into()` work here? If for some reason the type here changes, we >> probably want to know about it. > > I think we may be able to eliminate this cast by using c_int for the > integer type.
Sounds good.
>>> + }; >>> + >>> + // SAFETY: Switches to another thread. >>> + let timeout = >>> + guard.do_unlocked(|| unsafe { bindings::schedule_timeout(timeout as _) as _ }); >> >> Ditto. > > Here, we're casting u64->long and then long->u64. How about this? > > u64->long - Use timeout.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT), > since MAX_SCHEDULE_TIMEOUT is LONG_MAX. > > long->u64 - This value is guaranteed to be less than the argument > passed to schedule_timeout. Use .into() for infallible cast.
Also sounds good :)
-- Cheers, Benno
| |