Hello,
Forgot to comment on this after your commit, but here we go..
David Young wrote:
> > > 'options SPLDEBUG' adds instrumentation to spllower() and splraise() as
> > > well as routines to start/stop debugging and to record IPL transitions:
> > > spldebug_start(), spldebug_stop(), spldebug_raise(), spldebug_lower().
> >
> > this seems too ad-hoc to me.
>
> If you have suggestions for providing a comparable function that is less
> ad-hoc, let me know.
>
> > - please don't put MD code like this in kern/. IPL_ values can't be
> > compared with >= in MI code. return_address.9 is in man.i386.
I agree with yamt that it should not belong to sys/kern. Also, that it is
quite ad-hoc. Perhaps it was better to teach LOCKDEBUG to do IPL tracking,
since most cases are via mutex(9) and in future there should not be many
direct spl() calls?
In fact, I am not convinced that such IPL tracking is very useful - we have
reduced amount of IPLs, which is much simpler model than we used to have, and
the problem you were recently debugging was miss-interpretation of certain
spin-mutex behaviour.
Also, since it tracks IPL and not direct spl() calls, it should have rather
been IPLDEBUG option, not SPLDEBUG. :)
> > - can you explain "cpu_index(ci) > MAXCPUS" and "cpu_index(ci) >=
> > MAXCPUS" ?
>
> Typo.
These cannot happen. Should be KASSERTs, at least. I think same with
if (ipl >= IPL_HIGH) check. Also, this code adds arrays of MAXCPUS size.
While it is not a problem at the moment, in the long term we should not
depend on MAXCPUS, or perhaps even look for a way to remove it.
--
Mindaugas
