Date: Mon, 21 Aug 95 16:08:00 MDT From: terry@cs.weber.edu (Terry Lambert) To: bde@zeta.org.au (Bruce Evans) Cc: gibbs@freefall.FreeBSD.org, hackers@freefall.FreeBSD.org Subject: Re: Clock interrupts during probes? Message-ID: <9508212208.AA26967@cs.weber.edu> In-Reply-To: <199508211645.CAA01423@godzilla.zeta.org.au> from "Bruce Evans" at Aug 22, 95 02:45:34 am
next in thread | previous in thread | raw e-mail | index | archive | help
> Actually, the PCI code only allows previously enabled interrupts.
> Interrupts for previously configured devices are usually enabled at
> this point, but clock interrupts aren't enabled until quite late in
> init_main.c (by initclocks()).
>
> The PCI code and the EISA code shouldn't be allowing interrupts.
> Some of the ISA code called by the PCI and EISA code was written
> under the assumption that interrupts are disabled. E.g., INTREN().
> It's not obvious that INTREN() is safe to call at splhigh() like the
> ISA code does. Perhaps it isn't safe :-[.
>
> I don't see many reasons why clock interrupts need to be enabled so
> late. Perhaps the only reason is that curproc is bogusly initialized to
> &proc0 before proc0 is initialized. curproc is statically initialized
> and is reinitalized to the same thing near the start of init_main().
> This would cause hardclock() to follow a null pointer if it was called
> early:
>
> if (p) {
> ...
> pstats = p->p_stats;
> ...
> if (timerrisset(&pstats->p_timer[ITIMER_PROF].it_value) ...
> }
This is critically bogus. There is no reason for the timer code to need
to have knowledge of the process -- ESPECIALLY the initialization code.
> It would be more natural to leave curproc initialized to NULL until quite
> late, but init_main() says
>
> /*
> * Initialize the current process pointer (curproc) before
> * any possible traps/probes to simplify trap processing.
> */
>
> I think the idea is that traps should only occur in process context
> so trap handlers need not check for (curproc == NULL). However, at
> least the i386 pagefault handler checks curproc, and all trap handlers
> should probably check it, if only to panic as early as possible for
> traps from buggy interrupt handlers. Perhaps all traps that occur
> before proc0 is initialized should be fatal.
A trap can result in a wakeup, done on the process address.
This is actually also a bogus assumption, since it neglects both SMP
and the fact that the scheduler isn't running yet. Interrupts to the
devices prior to the scheduler/processor startup should be ignored
in all cases. The only "benefit" to aking them is that you get an
extra I/O when it's possible to process them.
This is the same order of bogosity that causes the init process 1 startup
to return to the main() caller to kick the machine alive rather than
a call down from additional assembly code. Someone was a bit lazy in
locore.s.
It's amusing to note that the other "kernel thread" type processes are
started up *after* init in any case -- there is a great deal of room
for potentially bogus behaviour as a result of interrupt reentrancy,
if it happened precisely as the scheduler was starting.
In the issue of SPL's, the spl0 is set in the forked processes prior to
them going into their tight loop. I can see no reasonable benefit from
this, specificall in vmdaemon().
Also in the neighborhood of the SPL's: the s = splimp() / stuff / splx(s)
trap around the car interfaces and domain initialization is all rather
silly, since it implies interrupts are enables prior to the cards being
active. Most likely, the probe routines should shutdown the interrupts
once it has been probed true, and the interrupts should be restarted as
part of the attach, which should take place after domain initialization.
This would cause the drivers to be architected in such a way as to allow
them to be loaded and unloaded, since an attach/detach registration
mechanism (instead of a silly loop in if.c) would be used to install the
drivers as they came online. One installed, interrupt processing could
be allowed, no problem. The initialization order in the current code
with regard to the domain registration is the sticking point.
> Not much easier - there would be no process context to sleep on. I
> think we need something using coroutines to probe and attach all (or
> large batches of) devices concurrently at boot time. Make the
> coroutine switch mechanism look like tsleep() and have the same
> semantics as tsleep() so that the same probe and attach code works
> correctly after the system is up. DELAY(n) would become
> csleep(&foo, PRIBIO, "foodelay", (n * hz) / 1000000).
As you point out, no sleep context, which implies no sleepers, which implies
the idea of wakeup is bogus.
I've made a healthy start at the modularization and a rewrite of the system
initialization code, per the discussion of initialization and ordering of
events, including a rewrite of the init_main.c code.
The patches are in my account on FreeFall (SYSINIT.tar.gz).
Note the mountroot() stuff in the README, if you plan on playing with
any of this.
Terry Lambert
terry@cs.weber.edu
---
Any opinions in this posting are my own and not those of my present
or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9508212208.AA26967>