Hello, Forgot to comment on this after your commit, but here we go..
David Young <dyo...@pobox.com> 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