Re: [PATCH v4 1/2] rust: add basic Pin<Vec<T, A>> abstractions

[Benno Lossin]

On Sun Oct 12, 2025 at 6:57 PM CEST, Markus Probst wrote:
> From what I can tell, there is no way to get a `Pin<&mut Vec<T, A>>`
> from a `&mut Pin<Vec<T, A>>`. We can only get `Pin<&mut [T]>` which is
> not usable in our case.

Hmm yeah that's true.

> If there is way, without the extension trait or an extra struct, I
> would be happy to implement it.

So I tried to look for the usage site of this and I found this usage in
your v1:

    +        let mut leds = KPinnedVec::with_capacity(
    +            Atmega1608LedAddress::VALUES.len() * Atmega1608LedId::VALUES.len(),
    +            GFP_KERNEL,
    +        )?;
    +
    +        let mut i = 0;
    +        for addr in Atmega1608LedAddress::VALUES {
    +            let mode_lock = Arc::pin_init(new_mutex!(()), GFP_KERNEL)?;
    +
    +            for id in Atmega1608LedId::VALUES {
    +                let Some(child) =
    +                    fwnode.get_child_by_name(&CString::try_from_fmt(fmt!("led@{i}"))?)
    +                else {
    +                    continue;
    +                };
    +
    +                let client = ARef::clone(&client);
    +                let mode_lock = Arc::clone(&mode_lock);
    +
    +                leds.push_pin_init(LedClassDev::new(
    +                    Some(idev),
    +                    None,
    +                    LedInitData::new().fwnode(&child),
    +                    Atmega1608Led {
    +                        addr,
    +                        id,
    +                        client,
    +
    +                        mode_lock,
    +                    },
    +                ))?;
    +                i += 1;
    +            }
    +        }
    +        Ok(KBox::new(Self { client, leds }, GFP_KERNEL)?.into())

And I think using `Vec` for this is just wrong. `Vec` is a data
structure that supports growing and shrinking the allocation. But you
just need a fixed size buffer that holds all your data. Do you think
that `Pin<Box<[LedClassDev]>>` would suffice if it had proper support
from pin-init?

Also, please don't top-post [1] and take a look at your mail client
configuration, it puts lots of extra `> ` at the end which looks pretty
strange [2].

[1]: https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
[2]: https://lore.kernel.org/all/e550b0862e9ea87e50688d1ec8f623638d170a3a.camel@xxxxxxxxx

---
Cheers,
Benno

> Thanks
> - Markus Probst
>
> On Sun, 2025-10-12 at 18:26 +0200, Benno Lossin wrote:
>> On Sun Oct 12, 2025 at 4:52 PM CEST, Markus Probst wrote:
>> > @@ -109,6 +111,21 @@ pub struct Vec<T, A: Allocator> {
>> >      _p: PhantomData<A>,
>> >  }
>> >  
>> > +/// Extension for Pin<Vec<T, A>>
>> > +pub trait PinnedVecExt<T> {
>> 
>> Why is this an extension trait? Couldn't we directly implement this
>> on
>> `Vec<T>` with `self: Pin<&mut Self>`?
>> 
>> ---
>> Cheers,
>> Benno
>> 
>> > +    /// Pin-initializes P and appends it to the back of the
>> > [`Vec`] instance without reallocating.
>> > +    fn push_pin_init<E: From<PushError<P>>, P: PinInit<T, E>>(&mut
>> > self, init: P) -> Result<(), E>;
>> > +
>> > +    /// Shortens the vector, setting the length to `len` and drops
>> > the removed values.
>> > +    /// If `len` is greater than or equal to the current length,
>> > this does nothing.
>> > +    ///
>> > +    /// This has no effect on the capacity and will not allocate.
>> > +    fn truncate(&mut self, len: usize);
>> > +
>> > +    /// Removes the last element from a vector and drops it
>> > returning true, or false if it is empty.
>> > +    fn pop(&mut self) -> bool;
>> > +}
>> > +
>> >  /// Type alias for [`Vec`] with a [`Kmalloc`] allocator.
>> >  ///
>> >  /// # Examples