Re: [PATCH v3 04/13] rust: hrtimer: implement `TimerPointer` for `Arc`

[Andreas Hindborg <a.hindborg@kernel.org>]

"Lyude Paul" <lyude@redhat.com> writes:

> On Thu, 2024-10-17 at 15:04 +0200, Andreas Hindborg wrote:
>> This patch allows the use of intrusive `hrtimer` fields in structs that are
>> managed by an `Arc`.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>>  rust/kernel/hrtimer.rs     |  3 +-
>>  rust/kernel/hrtimer/arc.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 91 insertions(+), 1 deletion(-)
>>
>> diff --git a/rust/kernel/hrtimer.rs b/rust/kernel/hrtimer.rs
>> index 99058a690f2e18b5c26c94c71133407019aa4a26..6427b0450c694105190c8cddea0c768ab195aca2 100644
>> --- a/rust/kernel/hrtimer.rs
>> +++ b/rust/kernel/hrtimer.rs
>> @@ -107,7 +107,6 @@ unsafe fn raw_get(ptr: *const Self) -> *mut bindings::hrtimer {
>>      /// # Safety
>>      ///
>>      /// `self_ptr` must point to a valid `Self`.
>> -    #[allow(dead_code)]
>>      pub(crate) unsafe fn raw_cancel(self_ptr: *const Self) -> bool {
>>          // SAFETY: timer_ptr points to an allocation of at least `Timer` size.
>>          let c_timer_ptr = unsafe { Timer::raw_get(self_ptr) };
>> @@ -302,3 +301,5 @@ unsafe fn raw_get_timer(ptr: *const Self) ->
>>          }
>>      }
>>  }
>> +
>> +mod arc;
>> diff --git a/rust/kernel/hrtimer/arc.rs b/rust/kernel/hrtimer/arc.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..881de053ecad866a26e46a0123ec2bf38511c2bc
>> --- /dev/null
>> +++ b/rust/kernel/hrtimer/arc.rs
>> @@ -0,0 +1,89 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use super::HasTimer;
>> +use super::RawTimerCallback;
>> +use super::Timer;
>> +use super::TimerCallback;
>> +use super::TimerHandle;
>> +use super::TimerPointer;
>> +use crate::sync::Arc;
>> +use crate::sync::ArcBorrow;
>> +use crate::time::Ktime;
>
> Is there a reason you're using separate lines for each include instead of
> grouping them together by module?

No particular reason. It is often easier to rebase things around when
they are on their own line.

Are there any code guidelines to follow on this?


Best regards,
Andreas Hindborg