svn commit: r303225 - head/sys/dev/fb

[Bruce Evans]

On Sat, 23 Jul 2016, John Baldwin wrote:

> Log:
>  Use MTX_SYSINIT for the VESA lock.

I wouldn't trust this either.

>  vesa_init_done isn't a reliable guard for the mutex init.  If
>  vesa_configure() doesn't find valid VESA info it will not set
>  vesa_init_done, but the lock will remain initialized.  Revert r303076
>  and use MTX_SYSINIT to deterministically init the lock.

I'm fixing consoles and needed very delicate modifications for the
sio_inited guard that you added in sio.  That basically works, but
it needs complicated atomic locking.  Early initialization is better,
but MTX_SYSINIT()'s obfuscated order is wrong for console drivers:

X enum sysinit_sub_id {
X 	SI_SUB_DUMMY		= 0x0000000,	/* not executed; for linker*/
X 	SI_SUB_DONE		= 0x0000001,	/* processed*/
X 	SI_SUB_TUNABLES		= 0x0700000,	/* establish tunable values */
X 	SI_SUB_COPYRIGHT	= 0x0800001,	/* first use of console*/
X 	SI_SUB_VM		= 0x1000000,	/* virtual memory system init*/
X ...
X 	SI_SUB_WITNESS		= 0x1A80000,	/* witness initialization */
X 	SI_SUB_MTX_POOL_DYNAMIC	= 0x1AC0000,	/* dynamic mutex pool */
X 	SI_SUB_LOCK		= 0x1B00000,	/* various locks */

MTX_SYSINIT() is in SI_SUB_LOCK, but console drivers want mutexes in
(actually before) SI_SUB_COPYRIGHT, to avoid special cases.

There are many other ordering bugs near here.  The copyright never was
the first use use of the console, at least on x86.  kdb was.  The
careful ordering to get consoles initialized as early as practical so
that kdb could be used is very broken now.  In my i386 configuration,
crashes or hangs for vm86 being used uninitialized just before kdb can
be run to debug it.  printf()s to consoles in getmemsize() are also
broken.  printf()s to the message buffer always were broken until after
getmemsize().  amd64 doesn't use vm86 so it doesn't crash.  The order
in init386() is not as obfuscated as the above.

syscons initializes its main lock (video_mtx) using mtx_init() before
SI_SUB_COPYRIGHT, and that seems to work right.  It guards this with
'cold' but that is bogus so I removed it.  'cold' is true until much
later and is not needed early.

Uninitialization is more broken for consoles than initialization, but is
less of a problem since it is usually unreachable.  It is only obviously
reachable for dcons, and is obviously wrong there (cnremove() doesn't
actually remove the console, but dcons tries it; no other driver even
tries it).

The SCHEDULER_STOPPED() hack caused me problems.  My fixed console
drivers know a little more about when it is safe to call mutex code
than the mutex system does, except they don't want to know about
this hack.  They do a check like mtx_unowned() (but more fragile).
mtx_unowned() also doesn't know about SCHEDULER_STOPPED().  mtx_unowned()
works accidentally in more cases without INVARIANTS because the inlined
mutex functions also don't know about SCHEDULER_STOPPED().  Usually they
work normally.  But when they would normally block, they call the public
functions that don't block and do leave the mutex in an inconsistent
state.  Then without INVARIANTS, mtx_owned() in KASSERT() is annulled
but direct mtx_owned() finds inconsistencies.

Bruce