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:   Wed, 31 May 2023 09:00:32 +0000
In-Reply-To: <87leh69par.fsf@metaspace.dk>
Mime-Version: 1.0
References: <87leh69par.fsf@metaspace.dk>
Message-ID: <20230531090032.3667467-1-aliceryhl@google.com>
Subject: Re: [PATCH v1 5/7] rust: workqueue: add helper for defining
 work_struct fields
From:   Alice Ryhl <aliceryhl@google.com>
To:     nmi@metaspace.dk
Cc:     alex.gaynor@gmail.com, aliceryhl@google.com,
        benno.lossin@proton.me, bjorn3_gh@protonmail.com,
        boqun.feng@gmail.com, gary@garyguo.net, jiangshanlai@gmail.com,
        linux-kernel@vger.kernel.org, ojeda@kernel.org,
        patches@lists.linux.dev, rust-for-linux@vger.kernel.org,
        tj@kernel.org, wedsonaf@gmail.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Andreas Hindborg <nmi@metaspace.dk> writes:
> Alice Ryhl <aliceryhl@google.com> writes:
>> +/// Used to safely implement the [`HasWork<T>`] trait.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use kernel::sync::Arc;
>> +///
>> +/// struct MyStruct {
>> +///     work_field: Work<Arc<MyStruct>>,
>> +/// }
>> +///
>> +/// impl_has_work! {
>> +///     impl HasWork<Arc<MyStruct>> for MyStruct { self.work_field }
>> +/// }
>> +/// ```
>> +///
>> +/// [`HasWork<T>`]: HasWork
>> +#[macro_export]
>> +macro_rules! impl_has_work {
>> +    ($(impl$(<$($implarg:ident),*>)?
>> +       HasWork<$work_type:ty>
>> +       for $self:ident $(<$($selfarg:ident),*>)?
>> +       { self.$field:ident }
>> +    )*) => {$(
>> +        // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right
>> +        // type.
>> +        unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type> for $self $(<$($selfarg),*>)? {
>> +            const OFFSET: usize = $crate::offset_of!(Self, $field) as usize;
>> +
>> +            #[inline]
>> +            unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_type> {
>> +                // SAFETY: The caller promises that the pointer is not dangling.
>> +                unsafe {
>> +                    ::core::ptr::addr_of_mut!((*ptr).$field)
>> +                }
>> +            }
> 
> What is the reason for overriding the default implementation of `raw_get_work()`?
> 
> BR Andreas

That's how the macro checks that the field actually has the type you
claim it has. If you lie about the type, then `raw_get_work` will not
compile. (See the safety comment on the impl block.)

Alice