On Fri 2021-12-10 16:09:31, Aaron Tomlin wrote:

On Fri 2021-12-10 11:00 +0100, Petr Mladek wrote:

quoted

quoted

If someone enables this feature I can't think of a reason why they
would want to limit this to some arbitrary number. So my preference
is to remove that limitation completely. I see no point to it.

I agree with Luis here. We could always add the limit later when
people report some real life problems with too long list. It is
always good to know that someone did some heavy lifting in
the system.

Fair enough.

quoted

It might be even interesting to remember timestamp of the removal
to match it with another events reported in the system log.

I'm not so sure about this. We could gather such details already via Ftrace
(e.g. see load_module()). Personally, I'd prefer to maintain a simple list.

Fair enough. It was just an idea. Simple list is a good start. We
could always add more details if people find it useful.

quoted

quoted

If you just bump the count then its not duplication, it just adds
more information that the same module name with the same taint flag
has been unloaded now more than once.

Please, do not remove records that a module was removed. IMHO, it
might be useful to track all removed module, including the non-tainted
ones. Module removal is always tricky and not much tested. The tain
flags might be just shown as extra information in the output.

This is an interesting suggestion. Albeit, as per the subject, I prefer to
just keep track of any module that tainted the kernel. That being said,
Petr, if you'd prefer to track each module unload/or deletion event, then I
would suggest for instance to remove a module once it has been reintroduced
or maintain an unload count as suggested by Luis.

I just have fresh in mind the patchset
https://lore.kernel.org/r/20211129034509.2646872-1-ming.lei@redhat.com (local)
It is about that removing sysfs interface is tricky and might lead to
use after free problems. I could imagine many other similar problems
that might happen with any module.

But I agree that the information about modules that tainted the kernel is
more important. I do not want to block the feature by requiring more
requirements.


Also we should keep in mind that the default panic() message should
be reasonably short. Only the last lines might be visible on screen.
Serial consoles might be really slow.

It is perfectly fine to add few lines, like the existing list of
loaded modules. Any potentially extensive output should be optional.
There already is support for optional info, see panic_print_sys_info().

Best Regards,
Petr