Rust-for-Linux / linux

Adding support for the Rust language to the Linux kernel.
https://rust-for-linux.com
Other
3.92k stars 417 forks source link

`pr_cont`: consider removing it #279

Open ojeda opened 3 years ago

ojeda commented 3 years ago

@nbdd0121

It seems that KERN_CONT is heavily discouraged: the comment in kern_levels.h says:

Only to be used by core/arch code during early bootup (a continued line is not SMP-safe otherwise).

Maybe we shouldn't expose it in Rust at all? With powerful formatting capability of core::fmt I don't see it being very useful anyway.

@ojeda

Agreed, even checkpatch.pl warns about it. And yet there are ~1k uses inside drivers/... :/ When I wrote this I was trying to produce the same functionality, more or less. But yeah, removing would simplify a few conditionals and one macro case.


Sample a few C drivers where pr_cont is used, and try to see what are their use cases. Then, taking that into account, decide and justify whether we should remove it from rust/kernel/print.rs (e.g. if it looks like all the C use cases can be easily done with Rust's formatting, or if it looks most of them should not be using pr_cont(), etc.).

obviyus commented 3 years ago

Some notes after perusing a couple of C drivers:

Apologies if I misunderstood anything. Please let me know if I should sample a couple more drivers.

nbdd0121 commented 3 years ago

L1707 of floppy.c

This isn't actually showing progress; it's only printing an array.

L298 of ssb/pci.c

This looks like the only case where pr_cont usage is inevitable, aka showing progress. But that could probably be replaced with multiple lines.