> Date: Wed, 23 Jun 2021 15:32:03 +0000 > From: Visa Hankala <v...@hankala.org> > > On Wed, Jun 23, 2021 at 05:15:05PM +0200, Mark Kettenis wrote: > > > Date: Wed, 23 Jun 2021 14:56:45 +0000 > > > From: Visa Hankala <v...@hankala.org> > > > > > > On Tue, Jun 22, 2021 at 09:46:22AM -0500, Scott Cheloha wrote: > > > > On Mon, Jun 21, 2021 at 02:04:30PM +0000, Visa Hankala wrote: > > > > > On Thu, May 27, 2021 at 07:40:26PM -0500, Scott Cheloha wrote: > > > > > > On Sun, May 23, 2021 at 09:05:24AM +0000, Visa Hankala wrote: > > > > > > > When a CPU starts processing a soft interrupt, it reserves the > > > > > > > handler > > > > > > > to prevent concurrent execution. If the soft interrupt gets > > > > > > > rescheduled > > > > > > > during processing, the handler is run again by the same CPU. This > > > > > > > breaks > > > > > > > FIFO ordering, though. > > > > > > > > > > > > If you want to preserve FIFO you can reinsert the handler at the > > > > > > queue > > > > > > tail. That would be more fair. > > > > > > > > > > > > If FIFO is the current behavior I think we ought to keep it. > > > > > > > > > > I have updated the patch to preserve the FIFO order. > > > > > > > > > > > > +STAILQ_HEAD(x86_soft_intr_queue, x86_soft_intrhand); > > > > > > > + > > > > > > > +struct x86_soft_intr_queue softintr_queue[X86_NSOFTINTR]; > > > > > > > > > > > > Why did we switch to STAILQ? I know we don't have very many > > > > > > softintr_disestablish() calls but isn't O(1) removal worth the extra > > > > > > pointer? > > > > > > > > > > I used STAILQ because it avoids the hassle of updating the list nodes' > > > > > back pointers. softintr_disestablish() with multiple items pending in > > > > > the queue is very rare in comparison to the normal > > > > > softintr_schedule() / > > > > > softintr_dispatch() cycle. > > > > > > > > > > However, I have changed the code back to using TAILQ. > > > > > > > > This looks good to me. I mean, it looked good before, but it still > > > > looks good. > > > > > > > > I will run with it for a few days. > > > > > > > > Assuming I hit no issues I'll come back with an OK. > > > > > > > > Is there an easy way to exercise this code from userspace? There > > > > aren't many softintr users. > > > > > > > > Maybe audio drivers? > > > > > > audio(4) is one option with a relatively high rate of scheduling. > > > Serial communications drivers, such as com(4), might be useful for > > > testing too. > > > > > > softintr_disestablish() can be exercised with uaudio(4) and ucom(4) > > > for example. > > > > > > I am still uncertain whether the barrier in softintr_disestablish() > > > is fully safe. The typical detach-side users are audio_detach(), > > > com_detach() and usb_detach(). They should be fine because the > > > surrounding code may sleep. However, sbus(4) worries me because it > > > invokes softintr_disestablish() from PCMCIA intr_disestablish callback, > > > and I do not know how wild the usage contexts can be. sbus(4) is > > > specific to sparc64, though. > > > > Suprise-removal is a thing for PCI as well as PCMCIA and USB. And in > > the PCI case this will call com_detach() and therefore > > softintr_disestablish() from interrupt context, where you can't sleep. > > > > So I don't think that using some sort of barrier that sleeps is an > > option. > > Well, com_detach() does things that may sleep, so then the existing code > seems wrong.
Hmm, actually, it seems I misremembered and PCI hotplug remove runs in a task (see dev/pci/ppb.c). So maybe it is ok. > I will revise the diff so that it spins rather than sleeps when a handler > is active. That wouldn't work on non-MP kernels isn't it?
