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 2B1D6199935 for ; Thu, 6 Mar 2025 09:42:55 +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=1741254176; cv=none; b=cUK6kAUCqBejKVLn+JPRp2iOqILMBXnY27f/UXsb96M5+OvpI+TD80OwOmOC3RubPiL2xJ/dXVDFX/O1DxMUDyD7kmD1b5YFgFGlNQl+nrs2gM7WNuNGvrDBTIZ85F5g36nr6OR6i9McNiL48zOBOGV6qr3zjZkMpXEkaSRVdTE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741254176; c=relaxed/simple; bh=Fq9SaQI/2OwWEr4YhcIiVOcqvPahsrSawmfqFeBO4iQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=AV8fvq2Nk7sAk0CCw9kd7X1LeB3QVLdG8jOqWNqnqxs+XKaWL5+LCy//VOuxi+NKjycWgtotRr4852k8QEMF10u3p+HLSrd608yfOEJrIO7TFieL+rQa0r/20S2onneCPHysTtsGaFurSyj6c2jB6C1392TaTPKqKTjk8nTJLY4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=m8vwI+h7; 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="m8vwI+h7" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1F8BFC4CEE0; Thu, 6 Mar 2025 09:42:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741254175; bh=Fq9SaQI/2OwWEr4YhcIiVOcqvPahsrSawmfqFeBO4iQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=m8vwI+h7T1Ym/gCpsabcvlO+ydhH/GIaiHmMr18ATW8bnrTRRN0e0zLCm+xzfKDYu Q6VQ8Gaaclu+5K6gRFIm13/kk5lJqW2VYSmDQQO8tB4Hmijn1nx5jvv9GEf3+i2QKE 6dkQZRMMnQFNo2KJZcpuylsQugY8+CAaqKMvpw3BtOLd8seu2xM5Fsjon5uIyFLIRN OS5006wc0f1QM4buXUjK5kXLrWdgLwdL5QVRH6JrQV5GS/C6zCSZ84EZ0yXQOS9Whr wTAtFX91fJ20Z/a4vSt2WzEyMsUZYpxxBB4We+REMUPqL6NUwQyCGL5v15KaguwGCA kqKhSo7mgMunw== From: Andreas Hindborg To: "Boqun Feng" Cc: "Dirk Behme" , , , , "Gary Guo" , "Alice Ryhl" , "Trevor Gross" Subject: Re: [PATCH v3 2/2] rust: types: `Opaque` doc: Add some destructor description In-Reply-To: (Boqun Feng's message of "Wed, 05 Mar 2025 22:01:11 -0800") References: <20250305053438.1532397-1-dirk.behme@de.bosch.com> <20250305053438.1532397-2-dirk.behme@de.bosch.com> <871pvbhqap.fsf@kernel.org> <87v7sn759g.fsf@kernel.org> <87zfhz5gq2.fsf@kernel.org> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Thu, 06 Mar 2025 10:42:47 +0100 Message-ID: <87o6ye5wbs.fsf@kernel.org> 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 "Boqun Feng" writes: > On Wed, Mar 05, 2025 at 10:07:33PM +0100, Andreas Hindborg wrote: >> "Boqun Feng" writes: >> > [...] >> >> >> > rust/kernel/types.rs | 26 +++++++++++++++++++++----- >> >> >> > 1 file changed, 21 insertions(+), 5 deletions(-) >> >> >> > >> >> >> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs >> >> >> > index af30e9c0ebccb..f370cdb48a648 100644 >> >> >> > --- a/rust/kernel/types.rs >> >> >> > +++ b/rust/kernel/types.rs >> >> >> > @@ -271,17 +271,33 @@ fn drop(&mut self) { >> >> >> > >> >> >> > /// Stores an opaque value. >> >> >> > /// >> >> >> > -/// [`Opaque`] is meant to be used with FFI objects that are never interpreted by Rust code. >> >> >> > +/// [`Opaque`] opts out of the following Rust language invariants for the contained `T` >> >> >> > +/// by using [`UnsafeCell`]: >> >> >> > /// >> >> >> > -/// It is used to wrap structs from the C side, like for example `Opaque`. >> >> >> > -/// It gets rid of all the usual assumptions that Rust has for a value: >> >> >> > +/// * Initialization invariant - the contained value is allowed to be uninitialized and >> >> >> > +/// contain invalid bit patterns. >> >> >> > +/// * Immutability invariant - [`Opaque`] allows interior mutability. >> >> >> > +/// * Uniqueness invariant - [`Opaque`] allows aliasing of shared references. >> >> >> >> >> >> This last one is wrong (I know it's probably my fault, sorry). It should be: >> >> >> >> >> >> /// * Uniqueness invariant - [`Opaque`] allows aliasing of **exclusive** references. >> >> >> >> >> > >> >> > Hmm... are we trying to say "`&mut Opaque` still cannot mean >> >> > noalias" here? If so I feel the wording might be confusing. I would >> >> > suggest we don't use "uniqueness" here. Maybe something like: >> >> > >> >> > * Always aliased invariant - `&mut` [`Opaque`] is not necessarily a >> >> > unique pointer, and thus the compiler cannot just make aliasing >> >> > assumptions. >> >> > >> >> > (I use the wording from UnsafePinned[1], maybe there could be better >> >> > wording) >> >> > >> >> > [1]: https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html#summary >> >> >> >> I like my wording better. It says the same with fewer words, and we have >> >> a more wordy section below anyway. We could combine: >> >> >> >> * Uniqueness invariant - [`Opaque`] allows aliasing of **exclusive** >> >> references. That is, `&mut` [`Opaque`] is not necessarily a unique >> >> pointer. >> >> >> >> >> >> How is that? >> >> >> > >> > I think my biggest problem is the word "allows", the phrase "allows >> > aliasing" sounds like Opaque provide something *optional*, however, >> > as I understand it, Opaque here just disallows certain optimization >> > from compiler, in other words, programmers want to use `Opaque` to >> > forbid something, hence "allows" may not be the good word to use here? >> > >> > How about: >> > >> > * Uniqueness invariant - [`Opaque`] disallow alias assumptions >> > made by compiler on an *exclusive* references. >> >> I don't object, but I feel like we should be able to define this within >> the context of the Rust language, without going into details about >> compiler internals. >> >> Without `Opaque` it is illegal to have aliased `&mut T`. With `Opaque`, >> it is legal to have aliased `&mut Opaque`. Or at least that is my >> understanding. >> > > Oh, I see. That's why you used "allows", however, allow me to explain > why it looks confusing to me. For `&mut T`, we can still have an > aliasing pointer (a pointer that points to the same variable as the > mutable reference): > > let mut x = ...; > > let mut_ref = &mut x; > let aliased_ptr = mut_ref as *mut _; > > it's just that if we dereference the pointer while the `&mut T` exists, > it's UB, e.g. > > unsafe { *aliased_ptr = ... }; Right. > > and `Opaque` allows above to be not a UB (there still are rules like > we cannot have data races, violating these rules would still be UB). Yes. > So to me "aliasing" can mean "having an aliasing pointer", and that as > shown above can be provided by both non-Opaque and Opaque cases. I > wonder whether "allow aliased/aliasing accesses" is better than "allow > aliasing" here. Yes, I think that would be better. I had another thing in mind when I wrote the text, and I am wondering if it is incorrect. As far as I know it is normally illegal to have more than one `&mut T` to the same `T`, even if no access occur through these references. With `Opaque`, is it still illegal to have two `&mut Opaque` to the same `T` in existence? I thought it was not illegal in this case. At any rate, your wording is probably the most accurate: * Uniqueness invariant - [`Opaque`] disallow alias assumptions made by compiler on an *exclusive* references. Alternatively, if we do not want to refer to compiler assumptions: * Uniqueness invariant - [`Opaque`] makes aliasing accesses to the underlying `T` well defined. Best regards, Andreas Hindborg