From patchwork Mon Aug 14 08:46:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benno Lossin X-Patchwork-Id: 135251 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b824:0:b0:3f2:4152:657d with SMTP id z4csp2623180vqi; Mon, 14 Aug 2023 02:30:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE4Mf9pJCdaEPg0p6bPL4M4rbTzNf/WkvhuSTAjBDvQ4YWVKc8ynvmkdB9ZhaI6u+pBgGEb X-Received: by 2002:a17:906:7489:b0:993:e752:1a71 with SMTP id e9-20020a170906748900b00993e7521a71mr7396528ejl.9.1692005400573; Mon, 14 Aug 2023 02:30:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692005400; cv=none; d=google.com; s=arc-20160816; b=FTr/DhCPOVfsuaBHp5NMJWbfEkBKjJcMFYBE+6Oda85ujhqZKUFQvzCu25Wmxj/gG2 t3wTGaYkRZy76M3xMuwIXOo/m6ixJeUgueOw9w1ei1L8Dl0lHZgEKibIJAylETqxmvrc C+c3UwSiIs+jSXzsV4tBOwJmts6bSIv2RYFMajWo8jyGnPiRJYjHtdtpEEi/vWay9qNz Gp8wka5uoUx7dy24245s7AI39Ykbhwpu1JXis2IfC3O4tF1e7313RgXamz10wHrojbkF SSYyXqSOzyGbqVacZ/n94wI7UQGmzsi34zweJce6Q+4kZ3idwNZN9UHMcYg6vXS5p9ui IMRA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :feedback-id:references:in-reply-to:message-id:subject:cc:from:to :dkim-signature:date; bh=r0rCJDgexbLd/PbKtXjKQNdC/yi+joahEDdPpIvT9F4=; fh=h3KS08NeBaYH5bCiXUF1yMk4NKDs2FHNIfHsq8atlgQ=; b=jMUiIaR2GorBN1nuSLRwhUUGocRkHk3AstG15HWy2KHGBUfKcc02USsCAIchqIDZKA fZQkZfMj9xPclNYmS8sgBkkTaEn+4t5Pvn6jZfVziTsm7JCountzrx++Ga/JN69ynF6b 3rgRBsUd4ffkGws+p/ZgwDQ/qceezpX3bElIkksO1V2LuGIQIL9s3oEjeoM+jlGWjq6v KzLSZ0FW0ug7SbB8zY8GpncQG4jAE2y1FlLNGEt4+A18q7VVKMjYg134oG+C5woPPHXy 6XVW4Ut/rgRtGSUDJ1uQcGMaxviKCI45XJY4EvME/dfC1n4KXcfIuxbfju0V3olTAEgF ikOA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@proton.me header.s=f4tuk7d6b5g5pm4hzkdqaeyym4.protonmail header.b=GuGi73VP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=proton.me Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id sd19-20020a170906ce3300b0098dafe075ccsi6696509ejb.97.2023.08.14.02.29.36; Mon, 14 Aug 2023 02:30:00 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@proton.me header.s=f4tuk7d6b5g5pm4hzkdqaeyym4.protonmail header.b=GuGi73VP; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=proton.me Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234994AbjHNIrU (ORCPT + 99 others); Mon, 14 Aug 2023 04:47:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38586 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235047AbjHNIrA (ORCPT ); Mon, 14 Aug 2023 04:47:00 -0400 Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 76F3210C for ; Mon, 14 Aug 2023 01:46:57 -0700 (PDT) Date: Mon, 14 Aug 2023 08:46:48 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=f4tuk7d6b5g5pm4hzkdqaeyym4.protonmail; t=1692002815; x=1692262015; bh=r0rCJDgexbLd/PbKtXjKQNdC/yi+joahEDdPpIvT9F4=; 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; b=GuGi73VPFHhqfKtnd2Cfr3GJXEI+uRjuLLuP+OCvBYdtmAKFphi6mgeHtuUzb0/AK 7l4crUPRsVWA07vpIUIvoRVtKOq1A8WSSB2xFCTM8TOZ0JXNuNNn2p5MmUZbaszlBk dGXhyLmYiCbFVMcXUEBDypGnu71XiZpW1igYvTI0hT+8N+DfBcscR+IrHM/hpLTiwZ TRGJjLWvaFcVZdSSjmoi9GzBXzQpCvM39LTUBnZMvq7/BYnvlNee63baGNUoFBD0N5 gGFNrW1SWNeBii3vqNRloCa1RSWqRUz/9QZMXppzmd3uoJnERAa91dEuOsY/TWeFog dD8PYFWqjSoBg== To: Miguel Ojeda , Wedson Almeida Filho , Alex Gaynor From: Benno Lossin Cc: Boqun Feng , Gary Guo , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Alice Ryhl , Andreas Hindborg , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, patches@lists.linux.dev, Asahi Lina , Martin Rodriguez Reboredo Subject: [PATCH v4 04/13] rust: init: make guards in the init macros hygienic Message-ID: <20230814084602.25699-5-benno.lossin@proton.me> In-Reply-To: <20230814084602.25699-1-benno.lossin@proton.me> References: <20230814084602.25699-1-benno.lossin@proton.me> Feedback-ID: 71780778:user:proton MIME-Version: 1.0 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_BLOCKED, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS autolearn=unavailable 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: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1774196254908341589 X-GMAIL-MSGID: 1774196254908341589 Use hygienic identifiers for the guards instead of the field names. This makes the init macros feel more like normal struct initializers, since assigning identifiers with the name of a field does not create conflicts. Also change the internals of the guards, no need to make the `forget` function `unsafe`, since users cannot access the guards anyways. Now the guards are carried directly on the stack and have no extra `Cell` field that marks if they have been forgotten or not, instead they are just forgotten via `mem::forget`. Suggested-by: Asahi Lina Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Reviewed-by: Gary Guo Signed-off-by: Benno Lossin --- v3 -> v4: - added Reviewed-by from Gary. v2 -> v3: - added Reviewed-by's from Martin and Alice. v1 -> v2: - use Gary's `paste!` macro to create the guard hygiene. rust/kernel/init.rs | 1 - rust/kernel/init/__internal.rs | 25 ++----- rust/kernel/init/macros.rs | 116 +++++++++++++++------------------ 3 files changed, 56 insertions(+), 86 deletions(-) diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index d431d0b153a2..0120674b451e 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -216,7 +216,6 @@ use alloc::boxed::Box; use core::{ alloc::AllocError, - cell::Cell, convert::Infallible, marker::PhantomData, mem::MaybeUninit, diff --git a/rust/kernel/init/__internal.rs b/rust/kernel/init/__internal.rs index 44751fb62b51..7abd1fb65e41 100644 --- a/rust/kernel/init/__internal.rs +++ b/rust/kernel/init/__internal.rs @@ -174,7 +174,6 @@ pub fn init(self: Pin<&mut Self>, init: impl PinInit) -> Result { ptr: *mut T, - do_drop: Cell, } impl DropGuard { @@ -190,32 +189,16 @@ impl DropGuard { /// - will not be dropped by any other means. #[inline] pub unsafe fn new(ptr: *mut T) -> Self { - Self { - ptr, - do_drop: Cell::new(true), - } - } - - /// Prevents this guard from dropping the supplied pointer. - /// - /// # Safety - /// - /// This function is unsafe in order to prevent safe code from forgetting this guard. It should - /// only be called by the macros in this module. - #[inline] - pub unsafe fn forget(&self) { - self.do_drop.set(false); + Self { ptr } } } impl Drop for DropGuard { #[inline] fn drop(&mut self) { - if self.do_drop.get() { - // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function - // ensuring that this operation is safe. - unsafe { ptr::drop_in_place(self.ptr) } - } + // SAFETY: A `DropGuard` can only be constructed using the unsafe `new` function + // ensuring that this operation is safe. + unsafe { ptr::drop_in_place(self.ptr) } } } diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs index 78091756dec0..454f31b8c614 100644 --- a/rust/kernel/init/macros.rs +++ b/rust/kernel/init/macros.rs @@ -994,7 +994,6 @@ impl<$($impl_generics)*> $pin_data<$($ty_generics)*> /// - `init_slot`: recursively creates the code that initializes all fields in `slot`. /// - `make_initializer`: recursively create the struct initializer that guarantees that every /// field has been initialized exactly once. -/// - `forget_guards`: recursively forget the drop guards for every field. #[doc(hidden)] #[macro_export] macro_rules! __init_internal { @@ -1034,6 +1033,7 @@ macro_rules! __init_internal { $crate::__init_internal!(init_slot($($use_data)?): @data(data), @slot(slot), + @guards(), @munch_fields($($fields)*,), ); // We use unreachable code to ensure that all fields have been mentioned exactly @@ -1048,10 +1048,6 @@ macro_rules! __init_internal { @acc(), ); } - // Forget all guards, since initialization was a success. - $crate::__init_internal!(forget_guards: - @munch_fields($($fields)*,), - ); } Ok(__InitOk) } @@ -1065,13 +1061,17 @@ macro_rules! __init_internal { (init_slot($($use_data:ident)?): @data($data:ident), @slot($slot:ident), + @guards($($guards:ident,)*), @munch_fields($(,)?), ) => { - // Endpoint of munching, no fields are left. + // Endpoint of munching, no fields are left. If execution reaches this point, all fields + // have been initialized. Therefore we can now dismiss the guards by forgetting them. + $(::core::mem::forget($guards);)* }; (init_slot($use_data:ident): // use_data is present, so we use the `data` to init fields. @data($data:ident), @slot($slot:ident), + @guards($($guards:ident,)*), // In-place initialization syntax. @munch_fields($field:ident <- $val:expr, $($rest:tt)*), ) => { @@ -1082,24 +1082,28 @@ macro_rules! __init_internal { // return when an error/panic occurs. // We also use the `data` to require the correct trait (`Init` or `PinInit`) for `$field`. unsafe { $data.$field(::core::ptr::addr_of_mut!((*$slot).$field), $field)? }; - // Create the drop guard. - // - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. + // Create the drop guard: // - // SAFETY: We forget the guard later when initialization has succeeded. - let $field = &unsafe { - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for $field. + ::kernel::macros::paste! { + // SAFETY: We forget the guard later when initialization has succeeded. + let [<$field>] = unsafe { + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) + }; - $crate::__init_internal!(init_slot($use_data): - @data($data), - @slot($slot), - @munch_fields($($rest)*), - ); + $crate::__init_internal!(init_slot($use_data): + @data($data), + @slot($slot), + @guards([<$field>], $($guards,)*), + @munch_fields($($rest)*), + ); + } }; (init_slot(): // no use_data, so we use `Init::__init` directly. @data($data:ident), @slot($slot:ident), + @guards($($guards:ident,)*), // In-place initialization syntax. @munch_fields($field:ident <- $val:expr, $($rest:tt)*), ) => { @@ -1109,24 +1113,28 @@ macro_rules! __init_internal { // SAFETY: `slot` is valid, because we are inside of an initializer closure, we // return when an error/panic occurs. unsafe { $crate::init::Init::__init($field, ::core::ptr::addr_of_mut!((*$slot).$field))? }; - // Create the drop guard. - // - // We only give access to `&DropGuard`, so it cannot be forgotten via safe code. + // Create the drop guard: // - // SAFETY: We forget the guard later when initialization has succeeded. - let $field = &unsafe { - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for $field. + ::kernel::macros::paste! { + // SAFETY: We forget the guard later when initialization has succeeded. + let [<$field>] = unsafe { + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) + }; - $crate::__init_internal!(init_slot(): - @data($data), - @slot($slot), - @munch_fields($($rest)*), - ); + $crate::__init_internal!(init_slot(): + @data($data), + @slot($slot), + @guards([<$field>], $($guards,)*), + @munch_fields($($rest)*), + ); + } }; (init_slot($($use_data:ident)?): @data($data:ident), @slot($slot:ident), + @guards($($guards:ident,)*), // Init by-value. @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*), ) => { @@ -1137,18 +1145,21 @@ macro_rules! __init_internal { unsafe { ::core::ptr::write(::core::ptr::addr_of_mut!((*$slot).$field), $field) }; // Create the drop guard: // - // We only give access to `&DropGuard`, so it cannot be accidentally forgotten. - // - // SAFETY: We forget the guard later when initialization has succeeded. - let $field = &unsafe { - $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) - }; + // We rely on macro hygiene to make it impossible for users to access this local variable. + // We use `paste!` to create new hygiene for $field. + ::kernel::macros::paste! { + // SAFETY: We forget the guard later when initialization has succeeded. + let [<$field>] = unsafe { + $crate::init::__internal::DropGuard::new(::core::ptr::addr_of_mut!((*$slot).$field)) + }; - $crate::__init_internal!(init_slot($($use_data)?): - @data($data), - @slot($slot), - @munch_fields($($rest)*), - ); + $crate::__init_internal!(init_slot($($use_data)?): + @data($data), + @slot($slot), + @guards([<$field>], $($guards,)*), + @munch_fields($($rest)*), + ); + } }; (make_initializer: @slot($slot:ident), @@ -1191,29 +1202,6 @@ macro_rules! __init_internal { @acc($($acc)* $field: ::core::panic!(),), ); }; - (forget_guards: - @munch_fields($(,)?), - ) => { - // Munching finished. - }; - (forget_guards: - @munch_fields($field:ident <- $val:expr, $($rest:tt)*), - ) => { - unsafe { $crate::init::__internal::DropGuard::forget($field) }; - - $crate::__init_internal!(forget_guards: - @munch_fields($($rest)*), - ); - }; - (forget_guards: - @munch_fields($field:ident $(: $val:expr)?, $($rest:tt)*), - ) => { - unsafe { $crate::init::__internal::DropGuard::forget($field) }; - - $crate::__init_internal!(forget_guards: - @munch_fields($($rest)*), - ); - }; } #[doc(hidden)]