Re: [PATCH] firmware: dmi_scan: Drop dmi_initialized

From: Jean Delvare
Date: Sat Sep 23 2017 - 11:29:48 EST


Hi Ingo,

On Sat, 23 Sep 2017 12:50:31 +0200, Ingo Molnar wrote:
> * Jean Delvare <jdelvare@xxxxxxx> wrote:
>
> > I don't think it makes sense to check for a possible bad
> > initialization order at run time on every system when it is all
> > decided at build time.
> >
> > A more efficient way to make sure developers do not introduce new
> > calls to dmi_check_system() too early in the initialization sequence
> > is to simply document the expected call order. That way, developers
> > have a chance to get it right immediately, without having to
> > test-boot their kernel, wonder why it does not work, and parse the
> > kernel logs for a warning message. And we get rid of the run-time
> > performance penalty as a nice side effect.
>
> Huh? Initialization ordering requirements are very opaque,

They were. Now they are very documented.

> and by removing the debug check any such bugs are actively hidden. How
> is documentation supposed to uncover such bugs once they happen?

You are looking at it the wrong way around. Documentation is how they
do not happen in the first place.

You hit this problem once, 9 years ago. You thought it would have been
easier to debug if there was a warning, and you added it. It was one
way to solve the problem but I claim it was not the best.

What I expect from developers calling a function they aren't familiar
with is to read its documentation first. That's the very reason why we
spend time writing the documentation. They should not just call the
function, boot and see if it works or not. Software engineering vs.
trial and error.

> So NAK.

This was FYI. I maintain this subsystem, and you did not convince me. I
also can't see a general trend of implementing what you suggest in the
rest of the kernel. Thankfully.

--
Jean Delvare
SUSE L3 Support