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 <[email protected]>
> > 
> > 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.

I will revise the diff so that it spins rather than sleeps when a handler
is active.

Reply via email to