On Fri, Jul 4, 2025 at 6:09 AM Benno Lossin lossin@kernel.org wrote:
On Fri Jul 4, 2025 at 1:23 AM CEST, Tamir Duberstein wrote:
On Thu, Jul 3, 2025 at 6:41 PM Tamir Duberstein tamird@gmail.com wrote:
On Thu, Jul 3, 2025 at 4:36 PM Benno Lossin lossin@kernel.org wrote:
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
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:
- 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.
- After all subsystems have merged the previous step, we change the implementations of `print_macro!` to use `fmt!` instead of `format_args!`.
- We remove all occurrences of `core::fmt` (& replace them with `kernel::fmt`), removing the `core::fmt::Display` impls.
That would probably work. We will probably see regressions because we can't just replace `core::fmt` imports with `kernel::fmt`, so new code may appear that uses the former.
I think this discussion would be productive on the next spin. The changes in other subsystems are now almost entirely changing of import paths -- perhaps that would be sufficiently uncontroversial for folks to give their Acked-bys.