On Fri Jul 4, 2025 at 1:23 AM CEST, Tamir Duberstein wrote:

On Thu, Jul 3, 2025 at 6:41 PM Tamir Duberstein [off-list ref] wrote:

quoted

On Thu, Jul 3, 2025 at 4:36 PM Benno Lossin [off-list ref] wrote:

quoted

I don't understand, can't you just do:

* add `rust/kernel/fmt.rs`,
* add `rust/macros/fmt.rs`,
* change all occurrences of `core::fmt` to `kernel::fmt` and
  `format_args!` to `fmt!`.

Yes, such a split could be done - I will do so in the next spin

quoted

The last one could be split by subsystem, no? Some subsystems might
interact and thus need simultaneous splitting, but there should be some
independent ones.

Yes, it probably can. As you say, some subsystems might interact - the
claimed benefit of doing this subsystem-by-subsystem split is that it
avoids conflicts with ongoing work that will conflict with a large
patch, but this is also the downside; if ongoing work changes the set
of interactions between subsystems then a maintainer may find
themselves unable to emit the log message they want (because one
subsystem is using kernel::fmt while another is still on core::fmt).

I gave this a try. I ran into the problem that `format_args!` (and,
after this patch, `fmt!`) is at the center of `print_macro!`, which
itself underpins various other formatting macros. This means we'd have
to bifurcate the formatting infrastructure to support an incremental
migration. That's quite a bit of code, and likely quite a mess in the
resulting git history -- and that's setting aside the toil required to
figure out the correct combinations of subsystems that must migrate
together.

So here is what we can do without duplicating the logic, though it
requires multiple cycles:

1. We merge the two `fmt.rs` files & each subsystem merges an
   implementation of `kernel::fmt::Display` for their types, but keeps
   the `core::fmt::Display` impl around.
2. After all subsystems have merged the previous step, we change the
   implementations of `print_macro!` to use `fmt!` instead of
   `format_args!`.
3. We remove all occurrences of `core::fmt` (& replace them with
   `kernel::fmt`), removing the `core::fmt::Display` impls.

---
Cheers,
Benno