From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9A8E212DDA1; Mon, 13 Oct 2025 14:47:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760366831; cv=none; b=dnB6Lh6B6jQhSdJX0dcq8NTe5I/TRm2V9aZ6zu/VRLdN6VPilIByVA8MpWxRfm5/f/mrzNkIDlklLZZdkNs6XMLirmbHeR8E524WpdAM7JBAhYwckGdxLvgks6lp81FPelewUsDoTUdFTfkNGmiwwRgQt8kPTRba0QrXaOb/nYI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760366831; c=relaxed/simple; bh=UszqnfPExzN/pXDJzghixK2YWh4rh+o850/Us1oA9dw=; h=Mime-Version:Content-Type:Date:Message-Id:From:To:Cc:Subject: References:In-Reply-To; b=soG3d5Cn9iKj4K8gQlJosrSUGiNfCz154HUjVViWOvlRKI4xSOJuJjoUui0ijBXQTI+kWlWQ2Td0h9rxZclYAZ5viLXTTMTdE6Zb89UR5Vv91bdmS7AX8yYuD22DtueIXvVNIAjXDjanTg8W1MXn5BiRy44bpH8CIRNZLT+IN+U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GNrFiN1A; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GNrFiN1A" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B802AC4CEE7; Mon, 13 Oct 2025 14:47:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1760366831; bh=UszqnfPExzN/pXDJzghixK2YWh4rh+o850/Us1oA9dw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=GNrFiN1AEOx+bZoxRfYZiZX1EMEvEqOA655+X++QtbuICKYQoQx73zLPiJyHbPYqh fYTpl4Q1GU/d/f/Q0nwHrV4yCqYlfogaKsLCNphTW6bajntgUqOXeybgv09Z5trv1y 8acIr9SzOceKudAYWhziAHQNN6sOKART3DEdH9AulyQh3i1ZOStHIFdUTxhct4Iiwt eVMB0Z1X7NYRtTVq9xZKzT2a/uyBerMGPHtfx4pDEpfEo+4PnjbG52uggioJukCRxz 5D4HAZepf4NyEWSfMJDIZgxLqEcC3Ar9Et/N7n9rvmAoAGLYP9+EVt2CX1ce86R12b Cer6ifyem1Idg== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 13 Oct 2025 16:47:05 +0200 Message-Id: From: "Benno Lossin" To: "Markus Probst" , "Danilo Krummrich" , "Miguel Ojeda" , "Alex Gaynor" , "Lee Jones" , "Pavel Machek" Cc: "Lorenzo Stoakes" , "Vlastimil Babka" , "Liam R. Howlett" , "Uladzislau Rezki" , "Boqun Feng" , "Gary Guo" , , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , , , Subject: Re: [PATCH v4 1/2] rust: add basic Pin> abstractions X-Mailer: aerc 0.21.0 References: <20251012145221.172116-1-markus.probst@posteo.de> <20251012145221.172116-2-markus.probst@posteo.de> <2a31fcd045582382987c8c1da7c4b7d58a1dff61.camel@posteo.de> <4bfba51907578cc0f4f25368240720f4148a2736.camel@posteo.de> In-Reply-To: <4bfba51907578cc0f4f25368240720f4148a2736.camel@posteo.de> On Mon Oct 13, 2025 at 3:43 PM CEST, Markus Probst wrote: > On Mon, 2025-10-13 at 10:03 +0200, Benno Lossin wrote: >> On Mon Oct 13, 2025 at 12:11 AM CEST, Markus Probst wrote: >> > On Sun, 2025-10-12 at 23:31 +0200, Benno Lossin wrote: >> > > 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> > > > A>>` >> > > > from a `&mut Pin>`. We can only get `Pin<&mut [T]>` >> > > > which >> > > > is >> > > > not usable in our case. >> > >=20 >> > > Hmm yeah that's true. >> > >=20 >> > > > If there is way, without the extension trait or an extra >> > > > struct, I >> > > > would be happy to implement it. >> > >=20 >> > > So I tried to look for the usage site of this and I found this >> > > usage >> > > in >> > > your v1: >> > >=20 >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 let m= ut leds =3D KPinnedVec::with_capacity( >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 Atmega1608LedAddress::VALUES.len() * >> > > Atmega1608LedId::VALUES.len(), >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 GFP_KERNEL, >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 )?; >> > > =C2=A0=C2=A0=C2=A0 + >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 let m= ut i =3D 0; >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 for a= ddr in Atmega1608LedAddress::VALUES { >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 let mode_lock =3D Arc::pin_init(new_mutex!(()), >> > > GFP_KERNEL)?; >> > > =C2=A0=C2=A0=C2=A0 + >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 for id in Atmega1608LedId::VALUES { >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 let Some(child) =3D >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 >> > > fwnode.get_child_by_name(&CString::try_from_fmt(fmt!("led@{i}"))? >> > > ) >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else { >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 continue= ; >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }; >> > > =C2=A0=C2=A0=C2=A0 + >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 let client =3D ARef::clone(&clie= nt); >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 let mode_lock =3D Arc::clone(&mo= de_lock); >> > > =C2=A0=C2=A0=C2=A0 + >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 leds.push_pin_init(LedClassDev::= new( >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Some(ide= v), >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 None, >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 LedInitD= ata::new().fwnode(&child), >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Atmega16= 08Led { >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 addr, >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 id, >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 client, >> > > =C2=A0=C2=A0=C2=A0 + >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 mode_lock, >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }, >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ))?; >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 i +=3D 1; >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 } >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> > > =C2=A0=C2=A0=C2=A0 +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Ok(KB= ox::new(Self { client, leds }, >> > > GFP_KERNEL)?.into()) >> > >=20 >> > > 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>` would suffice if it had proper >> > > support >> > > from pin-init? >> > As you can see in v1, the number of leds (or vec entries) depends >> > on >> > the fwnode (see the continue statement there). I don't think that >> > counts as fixed size. `Pin]>>` could >> > potentially be used instead of `Pin>` in my >> > scenario, >> > but that would require an extra byte of allocation for the max leds >> > of >> > 24 each and the code would look more ugly. At the point I use >> > Option in >> > the slice, its basically an unoptimized Vec (instead of storing the >> > length, it stores if an item in the buffer is present or not). >>=20 >> You can just make the length of the slice be the desired length? > That would work, but creates another allocation on the heap (Vec) > that could have been avoided. I don't think it would make `Pin A>>` obsolete. > > Or would you rather say, such allocations don't matter? No, but you're already allocating once per inner loop invocation, the `CString::try_from_fmt` function allocates :) I don't know the kind of application that you're writing, does performance matter? If yes, then just run your benchmark suite on both versions and look at the difference. If you don't have a benchmark suite, then perf probably isn't important enough. Also if you really want to avoid the allocation, then you probably could first query the length and store only that in a local var and then create the initializers on-demand. But then again to query that you're creating a string every loop iteration, which allocates :) >> (also, >> `i` is never incremented in the `continue` case, so it will act like >> a >> `break`?) > You just found a bug in v1. :) > Thanks > - Markus Probst > > [1] https://docs.rs/arrayvec/latest/arrayvec/struct.ArrayString.html Did you forget to put a reference to this? --- Cheers, Benno