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 E06C92BEC42; Mon, 29 Sep 2025 07:24:26 +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=1759130667; cv=none; b=qzaQR2BRYfEXAEzJwpOZdDMPkQ/yRQLuItREzkjg/usIDwcn0T2dV57/pzvpnCHy49D49r8m4QLuXeFp85kmcfF80lg8Yfi8cPnj66x3YA9smjKZ4pUNY8u+QlyJkcLVNw/oIHJA+EAHp4qNdjojG1eNNk0vdwNT7lfcNavIjuI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759130667; c=relaxed/simple; bh=1qzNnpu3NUoqi58w/V80ugREMmoNYeEvOtzP7t+zvkI=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=U2zcMS1CXpFKIPEh1co7y8hQ4oktgSKhukbmgyE0U2tQQo0sMqTYoEBTHxB/1RLlfspNaggnPthUFCV6fI5oQsoz04JhPWe1jgB+cDm/FmmzVpvsEsufSNHa0TCnHBVpcdv+vrOWFzpAD4d7xU6sGHmxvVhkuIybBgNk53SuXoY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=AwgzNJRu; 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="AwgzNJRu" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E1C08C4CEF4; Mon, 29 Sep 2025 07:24:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1759130666; bh=1qzNnpu3NUoqi58w/V80ugREMmoNYeEvOtzP7t+zvkI=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=AwgzNJRuocgjAGZ1GURK4UCtFvmTUaMRB+hol6d5Rk+/bvyCaocUKVrJF3iQ0UWLd 3Qc6mE5XX+a/dsG1lBnayQyoJuooAqF7qQfSLq/VfolhbDVh4DokgIMIwNU6hFdWe2 QqVn2li1Pox+IYgNPjwxGnkERpplwjciHOTFUto93ULc6qNb5ZE2JDtvsMTorjUDBF ZI1Bkk2BEW7PVGHpTuq0tG3bMJFwjBIQjHxLMAIruWYvk0TAZ2Tv6W6qg0vlONCTBe tsxJUnslbgBZCCJy+WtpXjvguHBMappH5ZE8nDsKgnCerpI3y+N3TY4nx5BR1Ukyn/ MCAbmC8FRzHVg== 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, 29 Sep 2025 09:24:19 +0200 Message-Id: Subject: Re: [PATCH v2 05/10] gpu: nova-core: gsp: Add GSP command queue handling Cc: "Alexandre Courbot" , "Lyude Paul" , , , "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Benno Lossin" , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "David Airlie" , "Simona Vetter" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "John Hubbard" , "Joel Fernandes" , "Timur Tabi" , , To: "Alistair Popple" From: "Danilo Krummrich" References: <20250922113026.3083103-1-apopple@nvidia.com> <20250922113026.3083103-6-apopple@nvidia.com> In-Reply-To: On Mon Sep 29, 2025 at 3:06 AM CEST, Alistair Popple wrote: > On 2025-09-26 at 12:20 +1000, Alexandre Courbot wro= te... >> On Thu Sep 25, 2025 at 3:32 PM JST, Alistair Popple wrote: >> >> >> > + #[expect(unused)] >> >> > + pub(crate) fn receive_msg_from_gsp( >> >> > + &mut self, >> >> > + timeout: Delta, >> >> > + init: impl FnOnce(&M, SBuffer>) -> Result, >> >> > + ) -> Result { >> >> > + let (driver_area, msg_header, slice_1) =3D wait_on(timeout= , || { >> >> > + let driver_area =3D self.gsp_mem.driver_read_area(); >> >> > + // TODO: find an alternative to as_flattened() >> >> > + #[allow(clippy::incompatible_msrv)] >> >> > + let (msg_header_slice, slice_1) =3D driver_area >> >> > + .0 >> >> > + .as_flattened() >> >> > + .split_at(size_of::()); >> >> > + >> >> > + // Can't fail because msg_slice will always be >> >> > + // size_of::() bytes long by the above = split. >> >> > + let msg_header =3D GspMsgElement::from_bytes(msg_heade= r_slice).unwrap(); >> >>=20 >> >> Any reason we're not just using unwrap_unchecked() here then? >> > >> > Because whilst my assertions about the code are currently correct if i= t ever >> > changes I figured it would be better to explicitly panic than end up w= ith >> > undefined behaviour. Is there some other advantage to using unwrap_unc= hecked()? >> > I can't imagine there'd be much of a performance difference. >>=20 >> Here I think we should just use the `?` operator. The function already >> returns a `Result` so it would fit. > > Actually note quite true - this is in a closure that must return `Option<= _>` > so returning `Result` doesn't fit. However it still fits because I just n= oticed > `::from_bytes()` returns an `Option` so `?` will still work. More in general, as by now, unwrap() panics the kernel, which is an absolut= e last resort, that should only be considered if there's really no other opti= on. Please also see [1] and [2]. [1] https://docs.kernel.org/process/coding-style.html#do-not-crash-the-kern= el [2] https://github.com/Rust-for-Linux/linux/issues/1191