Re: [PATCH v11 6/8] MAINTAINERS: rust: Add new sections for DELAY/SLEEP and TIMEKEEPING API

[Boqun Feng <boqun.feng@gmail.com>]

On Thu, Apr 03, 2025 at 08:03:34AM +0900, FUJITA Tomonori wrote:
> On Wed, 2 Apr 2025 09:29:18 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> > On Wed, Apr 02, 2025 at 11:16:27PM +0900, FUJITA Tomonori wrote:
> >> On Mon, 31 Mar 2025 07:03:15 -0700
> >> Boqun Feng <boqun.feng@gmail.com> wrote:
> >> 
> >> >> My recommendation would be to take all of `rust/kernel/time` under one
> >> >> entry for now. I suggest the following, folding in the hrtimer entry as
> >> >> well:
> >> >> 
> >> >> DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST]
> >> >> M:	Andreas Hindborg <a.hindborg@kernel.org>
> >> > 
> >> > Given you're the one who would handle the patches, I think this make
> >> > more sense.
> >> > 
> >> >> R:	Boqun Feng <boqun.feng@gmail.com>
> >> >> R:	FUJITA Tomonori <fujita.tomonori@gmail.com>
> >> > 
> >> > Tomo, does this look good to you?
> >> 
> >> Fine by me.
> >> 
> >> So a single entry for all the Rust time stuff, which isn't aligned
> >> with C's MAINTAINERS entries. It's just for now?
> >> 
> > 
> > Given Andreas is the one who's going to handle the PRs, and he will put
> > all the things in one branch. I think it's fine even for long term, and
> > we got all relevant reviewers covered. If the Rust timekeeping + hrtimer
> > community expands in the future, we can also add more entries. We don't
> > necessarily need to copy all maintainer structures from C ;-)
> 
> It seems I was mistaken. I had thought that the ideal goal was for the
> same team to maintain both the C code and the corresponding Rust code.
> 

Yeah, that was the ideal goal, but Frederic said in the hrtimer series:

	https://lore.kernel.org/rust-for-linux/Z8iLIyofy6KGOsq5@localhost.localdomain/

, to me it's clear that hrtimer maintainers want hrtimer Rust patches to
go to rust tree via Andreas, and given timekeeping maintainers are
basically the same group of people, and Thomas explicitly asked to be
added as reviewers:

	https://lore.kernel.org/rust-for-linux/87o6xu15m1.ffs@tglx/

It's a clear signal that timekeeping and hrtimer Rust patches are
preferred to go to rust tree, and Andreas had nicely accepted to handle
timekeeping and sleep/delay patches, so it makes sense to use one entry
if he prefers. Hope this clarifies things.

> 
> >> >> I assume patch 1 will go through the sched/core tree, and then Miguel
> >> >> can pick 7.
> >> >> 
> >> > 
> >> > Patch 1 & 7 probably should go together, but we can decide it later.
> >> 
> >> Since nothing has moved forward for quite a while, maybe it's time to
> >> drop patch 1.
> > 
> > No, I think we should keep it. Because otherwise we will use a macro
> 
> Yeah, I know. the first version of this uses a macro.
> 
> 
> > version of read_poll_timeout(), which is strictly worse. I'm happy to
> > collect patch #1 and the cpu_relax() patch of patch #7, and send an PR
> > to tip. Could you split them a bit:
> > 
> > * Move the Rust might_sleep() in patch #7 to patch #1 and put it at
> >   kernel::task, also if we EXPORT_SYMBOL(__might_sleep_precision), we
> >   don't need the rust_helper for it.
> > 
> > * Have a separate containing the cpu_relax() bit.
> > 
> > * Also you may want to put #[inline] at cpu_relax() and might_resched().
> > 
> > and we can start from there. Sounds good?
> 
> I can do whatever but I don't think these matters. The problem is that

Confused. I said I would do a PR, that means if no objection, the
patches will get merged. Isn't this a way to move forward? Or you're
against that I'm doing a PR?

> we haven't received a response from the scheduler maintainers for a
> long time. We don't even know if the implementation is actually an
> issue.
> 

If there's an issue, I can fix it. After all, printk confirmed that
".*s" format works for this case:

	https://lore.kernel.org/rust-for-linux/ZyyAsjsz05AlkOBd@pathway.suse.cz/

and Peter sort of confirmed he's not against the idea:

	https://lore.kernel.org/rust-for-linux/20250201121613.GC8256@noisy.programming.kicks-ass.net/

I don't see any major issue blocking this. But of course, I'm happy to
resolve if there is one.


Regards,
Boqun

> 
>