Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 23 May 2023 16:43:33 +0100
From:   Gary Guo <gary@garyguo.net>
To:     Alice Ryhl <aliceryhl@google.com>
Cc:     rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Tejun Heo <tj@kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        "=?UTF-8?B?QmrDtnJu?= Roy Baron" <bjorn3_gh@protonmail.com>,
        Benno Lossin <benno.lossin@proton.me>,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev
Subject: Re: [PATCH v1 3/7] rust: sync: add `Arc::{from_raw, into_raw}`
Message-ID: <20230523164333.2c4b0d85.gary@garyguo.net>
In-Reply-To: <20230517203119.3160435-4-aliceryhl@google.com>
References: <20230517203119.3160435-1-aliceryhl@google.com>
        <20230517203119.3160435-4-aliceryhl@google.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?03uJnsbIihydYbRLyR5qNuZmxcOdb6hCg9662nQr+UswVJoz5cUIRsyjbPt0?=
 =?us-ascii?Q?YoTKY+Vi+VmYT0alFUzmc7ru9iqH4QFdEmtDarPiTcBk0ArD6zCzU5sqw/sN?=
 =?us-ascii?Q?KkXeylS1K3HQwrPoxpk9PSaseeoOuAyr0j7ak4K7LjSw8LWhnMg5wqC/MiEu?=
 =?us-ascii?Q?fRZEvxN0YyGKpifbeeMBwsK7hn53IuoPrYkff7pmEhrusUF0bNij/srGh1qt?=
 =?us-ascii?Q?rC18Fq35ZfaC04XTEgkfWQeB6J/hnlCE0U8w03O5ntpe1gdqfa5B9so3u/Kh?=
 =?us-ascii?Q?YqoP6vlGxhoN2z+fPepiX1oMdKp2MEp/8e6ry/eipaSbXAwtL8p/iBHQbVXc?=
 =?us-ascii?Q?DpVaDvG8JdVHVWSSxmSJdfnKcpDy/4YmVoYRbaKWq1fTg5OY7IV637RcxmeB?=
 =?us-ascii?Q?AK52rU9AVHirvB1HQ9Klq52Cu+sN33zrDEJhyKp18xMSA/W2IM6yQSz4D/m0?=
 =?us-ascii?Q?uXzzMNyDKS/rdHolsN7MlEtGu0+578jmOS8yL2lIb2QOYgEYCFEamM+tXicq?=
 =?us-ascii?Q?lZjxtEcHWKwmJ1AQ3Zo/+4sKu4QvCgyPuQHdyWZEKG349pOfysmWc7fQIGCD?=
 =?us-ascii?Q?/2tx6O1sNOjk6Igv5r/OVIwVK9LCERi8y7UHI4ArTXAp683Y9gDt+ol7WfKG?=
 =?us-ascii?Q?drh1u8uchbeDD5PoqmgEZx2domIMnhQlJsQZk/JpJ0ORrDgB8HV2HU0ZUJsz?=
 =?us-ascii?Q?InRI3FaOotIUfk/YvdaThPtnMJgrNw1aIJDHzxF6/f+ShElPB7Uo2Kkv33nt?=
 =?us-ascii?Q?6O5IVVx2Iu6D+h4Hx42u32uhEnGQvmgpa/zYQzBTtchspMTxESfqw2nzsgU6?=
 =?us-ascii?Q?b3LS7eD7tPkHqMt4mXPwAdsSipFCJCsxZkvIEeVKu7o4g0X4CXff1Hg1ecE2?=
 =?us-ascii?Q?FtCXthDXmj6kpSM5E0sOZyO+beEve6pZW59KADFWZklsIJy0h5O0st023Z3d?=
 =?us-ascii?Q?M4FyStOOUatZz28C3TM4n3CYvp/a2F+kR6MU4kHvSdPvTHpRfWlXEfuRtrRB?=
 =?us-ascii?Q?SfAbVO0lkpkh75O/f7FtkQWMRZh48b7TnSWgZ85+cE8uMjNbxNg2iJf2p3EI?=
 =?us-ascii?Q?U13wnlWxrNyphswHEYgUHUFwi1KAz7aIvEVd0I/wdG5lRT2BttkAwRpSojAg?=
 =?us-ascii?Q?gzacthA91cmjhk0Qszdgz228jUW3WG8kiUPFq2zhkrJwjzatIzLAdqyD/a9c?=
 =?us-ascii?Q?Zjk7TBEZ4CcN5Qr+YkAA0Q/EbvxHII8TZhh2h8ZsoMeMzcF1Idqn26kstRk8?=
 =?us-ascii?Q?JeAdecUubDWrlegUbRnL3hvTWwx8wZLqMujUkEmMRw8sSTxDserAnfVMW4Vk?=
 =?us-ascii?Q?FMbceSeISikxHgfTUIpHPIVLC393YYKpzIzmIxp1dnc8CS5ZpYzLREHGKJOf?=
 =?us-ascii?Q?IxUqLHw9xUtUidc/UayT3wPF6vqyTa9pl68NP/pOEKRD/aklMrRRwJ85MYC2?=
 =?us-ascii?Q?8Xqyb7yvvfqHvqci3qP1VRYIXhB6wg/eOzhl1MQ22LokHMbFFwlcY1LxliGw?=
 =?us-ascii?Q?IgN7a6oi3Xd12Fm60LquIRFEe6+6syzw+r9n09XbPBZx3Hchoy3CEHIMJRms?=
 =?us-ascii?Q?o2HKPlvLa6OYMbpiD5vh17MOxKFD2wU8vvd0vwIX?=
X-OriginatorOrg: garyguo.net
X-MS-Exchange-CrossTenant-Network-Message-Id: dfbb8e11-9588-415a-59a7-08db5ba4787e
X-MS-Exchange-CrossTenant-AuthSource: LO2P265MB5183.GBRP265.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2023 15:43:35.8874
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bbc898ad-b10f-4e10-8552-d9377b823d45
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /ExKZgiEcP4jSSW/oKgbOCUgwov1rZzBu1N2n9p589O3RQHHw2LXgOyhhAnx/U4zZV5psDQahF6XbYJKFtSHnw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CWLP265MB6183
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, 17 May 2023 20:31:15 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> These methods can be used to turn an `Arc` into a raw pointer and back,
> in a way that preserves the metadata for fat pointers.
> 
> This is done using the unstable ptr_metadata feature [1]. However, it
> could also be done using the unstable pointer_byte_offsets feature [2],
> which is likely to have a shorter path to stabilization than
> ptr_metadata.
> 
> Link: https://github.com/rust-lang/rust/issues/81513 [1]
> Link: https://github.com/rust-lang/rust/issues/96283 [2]
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Co-developed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/lib.rs      |  1 +
>  rust/kernel/sync/arc.rs | 44 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index cdf9fe999328..82854c86e65d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -22,6 +22,7 @@
>  #![feature(generic_associated_types)]
>  #![feature(new_uninit)]
>  #![feature(pin_macro)]
> +#![feature(ptr_metadata)]
>  #![feature(receiver_trait)]
>  #![feature(unsize)]
>  
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e6d206242465..7c55a9178dfb 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -210,6 +210,50 @@ impl<T: ?Sized> Arc<T> {
>          }
>      }
>  
> +    /// Convert the [`Arc`] into a raw pointer.
> +    ///
> +    /// The raw pointer has ownership of the refcount that this Arc object owned.
> +    pub fn into_raw(self) -> *const T {
> +        let ptr = self.ptr.as_ptr();
> +        core::mem::forget(self);
> +        // SAFETY: The pointer is valid.
> +        unsafe { core::ptr::addr_of!((*ptr).data) }
> +    }
> +
> +    /// Recreates an [`Arc`] instance previously deconstructed via [`Arc::into_raw`].
> +    ///
> +    /// This code relies on the `repr(C)` layout of structs as described in
> +    /// <https://doc.rust-lang.org/reference/type-layout.html#reprc-structs>.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must have been returned by a previous call to [`Arc::into_raw`]. Additionally, it
> +    /// can only be called once for each previous call to [`Arc::into_raw`].
> +    pub unsafe fn from_raw(ptr: *const T) -> Self {
> +        // SAFETY: The safety requirement ensures that the pointer is valid.
> +        let val_align = core::mem::align_of_val(unsafe { &*ptr });
> +        let refcount_size = core::mem::size_of::<Opaque<bindings::refcount_t>>();
> +
> +        // Use the `repr(C)` algorithm to compute the offset of `data` in `ArcInner`.
> +        //
> +        // Pseudo-code for the `#[repr(C)]` algorithm can be found here:
> +        // <https://doc.rust-lang.org/reference/type-layout.html#reprc-structs>
> +        let mut val_offset = refcount_size;
> +        let val_misalign = val_offset % val_align;
> +        if val_misalign > 0 {
> +            val_offset += val_align - val_misalign;
> +        }

Given the layout of the whole ArcInner can be calculated as

	Layout::new::<bindings::refcount_t>().extend(Layout::for_value(&*ptr)).unwrap_unchecked().0.pad_to_align()

The offset of `data` could be more intuitively calculated by

	Layout::new::<bindings::refcount_t>().extend(Layout::for_value(&*ptr)).unwrap_unchecked().1

or

	Layout::new::<bindings::refcount_t>().align_to(val_align).unwrap_unchecked().pad_to_align().size()

Best,
Gary