Re: [PATCH net-next v2 0/6] rust: Add IO polling

[FUJITA Tomonori]

On Sat, 12 Oct 2024 20:16:44 -0700
Boqun Feng <boqun.feng@xxxxxxxxx> wrote:

> On Sun, Oct 13, 2024 at 11:50:33AM +0900, FUJITA Tomonori wrote:
>> On Sun, 13 Oct 2024 10:15:05 +0900 (JST)
>> FUJITA Tomonori <fujita.tomonori@xxxxxxxxx> wrote:
>> 
>> > On Sat, 12 Oct 2024 08:29:06 -0700
>> > Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>> > 
>> >> While, we are at it, I want to suggest that we also add
>> >> rust/kernel/time{.rs, /} into the "F:" entries of TIME subsystem like:
>> >> 
>> >> diff --git a/MAINTAINERS b/MAINTAINERS
>> >> index b77f4495dcf4..09e46a214333 100644
>> >> --- a/MAINTAINERS
>> >> +++ b/MAINTAINERS
>> >> @@ -23376,6 +23376,8 @@ F:      kernel/time/timeconv.c
>> >>  F:     kernel/time/timecounter.c
>> >>  F:     kernel/time/timekeeping*
>> >>  F:     kernel/time/time_test.c
>> >> +F:     rust/kernel/time.rs
>> >> +F:     rust/kernel/time/
>> >>  F:     tools/testing/selftests/timers/
>> >> 
>> >>  TIPC NETWORK LAYER
>> >> 
>> >> This will help future contributers copy the correct people while
>> >> submission. Could you maybe add a patch of this in your series if this
>> >> sounds reasonable to you? Thanks!
>> > 
>> > Agreed that it's better to have Rust time abstractions in
>> > MAINTAINERS. You add it into the time entry but there are two options
>> > in the file; time and timer?
>> > 
>> > TIMEKEEPING, CLOCKSOURCE CORE, NTP, ALARMTIMER
>> > M:      John Stultz <jstultz@xxxxxxxxxx>
>> > M:      Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> > R:      Stephen Boyd <sboyd@xxxxxxxxxx>
>> > 
>> > HIGH-RESOLUTION TIMERS, TIMER WHEEL, CLOCKEVENTS
>> > M:      Anna-Maria Behnsen <anna-maria@xxxxxxxxxxxxx>
>> > M:      Frederic Weisbecker <frederic@xxxxxxxxxx>
>> > M:      Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> > 
>> > The current Rust abstractions which play mainly with ktimer.h. it's 
>> > not time, timer stuff, I think.
>> 
>> Oops, s/ktimer.h/ktime.h/
>> 
>> No entry for ktime.h in MAINTAINERS; used by both time and timer
>> stuff.
>> 
> 
> I think ktime.h belongs to TIMEKEEPING, since ktime_get() is defined in
> kernel/time/timekeeping.c and that's a core function for ktime_t, but

Sounds reasonable.

This patchset adds Delta (also belongs to time, I guess) and fsleep to
rust/kernel/time.rs. I think that fsleep belongs to timer (because
sleep functions in kernel/time/timer.c). It's better to add
rust/kerne/time/timer.rs for fsleep() rather than putting both time
and timer stuff to rust/kernel/time.rs?


> you're not wrong that there is no entry of ktime.h in MAINTAINERS, so if
> you want to wait for or check with Thomas, feel free.
> 
>> > As planned, we'll move *.rs files from rust/kernel in the future,
>> > how we handle time and timer abstractions?
> 
> I don't think core stuffs will be moved from rust/kernel, i.e. anything
> correspond to the concepts defined in kernel/ directory probably stays
> in rust/kernel, so time and timer fall into that category.

Seems that I misunderstood. The plan for the future layout is
documented somewhere?


>> Looks like that we'll add rust/kernel/hrtimer/ soon. I feel that it's
>> better to decide on the layout of time and timer abstractions now
>> rather than later.
> 
> I already suggested we move hrtimer.rs into rust/kernel/time to match
> the hierarchy of the counterpart in C (kernel/time/hrtimer.c):
> 
> 	https://lore.kernel.org/rust-for-linux/ZwqTf-6xaASnIn9l@boqun-archlinux/

Sounds good.