> On 12 Feb 2026, at 07:11, Boris Brezillon <[email protected]>
> wrote:
>
>
>> +type LockedSeat<T, const MAX_SLOTS: usize> = LockedBy<Seat, SlotManager<T,
>> MAX_SLOTS>>;
>> +
>> +impl<T: SlotOperations, const MAX_SLOTS: usize> Unpin for SlotManager<T,
>> MAX_SLOTS> {}
>
> Do we really need to explicitly flag this type Unpin? I thought this
> was the default if the struct is not pinned (and it's not AFAICT).
We don’t.
Note that “Unpin” and “not pinned” are distinct concepts. Unpin
means, in more informal terms, “this type does not care if it is
moved”, but it does not preclude you from putting it into Pin<T> or
kernel::sync::Arc<T>. If by “not pinned” you mean no #[pin_data] or
#[pin], then also note that deriving this macro is distinct from "being
pinned".
The relevant rule here is that a type is Unpin if all its fields are Unpin.
This is the case for all non-generic types in SlotManager, so it is Unpin as
long as the generic types T and T::SlotData are also Unpin.
>> +
>> + // Checks and updates the seat state based on the slot it points to
>> + // (if any). Returns a Seat with a value matching the slot state.
>> + fn check_seat(&mut self, locked_seat: &LockedSeat<T, MAX_SLOTS>) ->
>> Seat {
>> + let new_seat = match locked_seat.access(self) {
>> + Seat::Active(seat_info) => {
>> + let old_slot_idx = seat_info.slot as usize;
>> + let slot = &self.slots[old_slot_idx];
>> +
>> + if kernel::warn_on!(
>> + !matches!(slot, Slot::Active(slot_info) if
>> slot_info.seqno == seat_info.seqno)
>> + ) {
>> + Seat::NoSeat
>> + } else {
>> + Seat::Active(SeatInfo {
>> + slot: seat_info.slot,
>> + seqno: seat_info.seqno,
>> + })
>> + }
>> + }
>> +
>> + Seat::Idle(seat_info) => {
>> + let old_slot_idx = seat_info.slot as usize;
>> + let slot = &self.slots[old_slot_idx];
>> +
>> + if !matches!(slot, Slot::Idle(slot_info) if slot_info.seqno
>> == seat_info.seqno) {
>> + Seat::NoSeat
>> + } else {
>> + Seat::Idle(SeatInfo {
>> + slot: seat_info.slot,
>> + seqno: seat_info.seqno,
>> + })
>> + }
>> + }
>> +
>> + _ => Seat::NoSeat,
>> + };
>> +
>> + // FIXME: Annoying manual copy. The original idea was to not add
>> Copy+Clone to SeatInfo,
>> + // so that only slot.rs can change the seat state, but there might
>> be better solutions
>> + // to prevent that.
>
> Okay, I guess we want some inputs from Daniel and/or Alice on that one.
Hm, I'd say we shouldn't implement Clone to avoid any possibility of holding on
to stale state by duplicating it.
Why do we need to return Seat from this function? Can't we simply write
locked_seat in place and not return anything?
>
>> + match &new_seat {
>> + Seat::Active(seat_info) => {
>> + *locked_seat.access_mut(self) = Seat::Active(SeatInfo {
>> + slot: seat_info.slot,
>> + seqno: seat_info.seqno,
>> + })
>> + }
>> + Seat::Idle(seat_info) => {
>> + *locked_seat.access_mut(self) = Seat::Idle(SeatInfo {
>> + slot: seat_info.slot,
>> + seqno: seat_info.seqno,
>> + })
>> + }
>> + _ => *locked_seat.access_mut(self) = Seat::NoSeat,
>> + }
>> +
>> + new_seat
>> + }
>> +
