From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) (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 D234C1537CB for ; Mon, 17 Mar 2025 17:22:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.70.40.133 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742232147; cv=none; b=HEQSnUVU1u8cdRTYElMR87nNjrRZyw5+SKKWlgxf1jUMvsonPHfBk5NSb3dI0dUPYiRJBBDfNWIC+uk9XlQTiYbfBYDzBJkrvXCihRpGdZ08K9qRW2/l1bDmh0b9Zy+MxPSxUAbQYn9XvhXDB16RIHk5o3Aciq0AjRvhFUEMFeM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742232147; c=relaxed/simple; bh=B+Up7d0Z8wa56e82ogZrDvmKDPW4RJZVDvjvFFZDNRw=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=tIXJJDwYr6CvFzaQefrRgVis6DLXQ/53STdzX/HsMG7cBts1OfpCHpoZ5izncQLiUrEZjEDMjXAei9dLKlziJf+1XLdixf4YHW+heEn8fYvrbL4Ra5JzjlrvP41A36WD9n1FZzrpQKcwOmo84D3K+4qwZwM6cldmiISf7FAXan8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me; spf=pass smtp.mailfrom=proton.me; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b=UU87RXA5; arc=none smtp.client-ip=185.70.40.133 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=proton.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=proton.me Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=proton.me header.i=@proton.me header.b="UU87RXA5" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1742232143; x=1742491343; bh=DESxcuoCBw+aq4zHzk9noLj49928I7AbU/o2hoK4cnM=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=UU87RXA5BRMvneWzmaI8Z8XZj1+ESi5u9or6emtjsj67lXFTS0hWiziPYJINU5p4H T5iydj69o9KIlYCe2+itcXl5SGbJhdyaK5U31SKO5RZ5xlFrUT22wFqqxKSxKE16ok f204eaw+S3Nra4ALii6BGP2cPTmics/IP6ge3dFUJ5sw5MibG8/2Xpy6j33lySOmJl wqguRR5IPzU6qjR64iN3EJXRBquXF5fRMs5IG/NZQTkneEAcFe3+Sqct+xPT25bU/M +2IHCEEt2sjbmxb3s9g1bscRPeoPfQIrqRhgaMrg2e5O243GOl19VbulBGl8JokbDU rAq8p1k7vsHcA== Date: Mon, 17 Mar 2025 17:22:15 +0000 To: Danilo Krummrich , Tamir Duberstein From: Benno Lossin Cc: Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?Q?Bj=C3=B6rn_Roy_Baron?= , Andreas Hindborg , Alice Ryhl , Trevor Gross , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] rust: alloc: use `spare_capacity_mut` to reduce unsafe Message-ID: In-Reply-To: References: <20250317-vec-push-use-spare-v1-1-7e025ef4ae14@gmail.com> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: e266c822ec3552c9807e853f5439c912b8c7816c Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Mon Mar 17, 2025 at 6:09 PM CET, Danilo Krummrich wrote: > On Mon, Mar 17, 2025 at 10:39:05AM -0400, Tamir Duberstein wrote: >> On Mon, Mar 17, 2025 at 10:34=E2=80=AFAM Benno Lossin wrote: >> > On Mon Mar 17, 2025 at 12:42 PM CET, Tamir Duberstein wrote: >> > > Use `spare_capacity_mut` in the implementation of `push` to reduce t= he >> > > use of `unsafe`. Both methods were added in commit 2aac4cd7dae3 ("ru= st: >> > > alloc: implement kernel `Vec` type"). >> > > >> > > Signed-off-by: Tamir Duberstein >> > > --- >> > > rust/kernel/alloc/kvec.rs | 11 ++--------- >> > > 1 file changed, 2 insertions(+), 9 deletions(-) >> > > >> > > diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs >> > > index ae9d072741ce..d2bc3d02179e 100644 >> > > --- a/rust/kernel/alloc/kvec.rs >> > > +++ b/rust/kernel/alloc/kvec.rs >> > > @@ -285,15 +285,8 @@ pub fn spare_capacity_mut(&mut self) -> &mut [M= aybeUninit] { >> > > pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocE= rror> { >> > > self.reserve(1, flags)?; >> > > >> > > - // SAFETY: >> > > - // - `self.len` is smaller than `self.capacity` and hence, = the resulting pointer is >> > > - // guaranteed to be part of the same allocated object. >> > > - // - `self.len` can not overflow `isize`. >> > > - let ptr =3D unsafe { self.as_mut_ptr().add(self.len) }; >> > > - >> > > - // SAFETY: >> > > - // - `ptr` is properly aligned and valid for writes. >> > > - unsafe { core::ptr::write(ptr, v) }; >> > > + // The call to `reserve` was successful so the spare capaci= ty is at least 1. >> > > + self.spare_capacity_mut()[0].write(v); >> > >> > I think the code uses unsafe to avoid a bounds check, but I'm not 100% >> > sure. Danilo might remember more info. > > Yes, that was the justification to use unsafe calls instead. > > (This may also justify keeping dec_len() unsafe, since otherwise it would > introduce an additional boundary check for pop().) If we use saturating_sub then we don't need a bounds check (at least on non-debug builds), right?=20 >> We could use `slice::get_unchecked_mut` here to retain the same >> guarantee of no bounds check. That would still be one fewer unsafe >> blocks. > > Sounds reasonable. +1