Re: amd64: softintr_dispatch: remove kernel lock
On Wed, Jun 23, 2021 at 05:39:05PM +0200, Mark Kettenis wrote: > > Date: Wed, 23 Jun 2021 15:32:03 + > > From: Visa Hankala > > > > On Wed, Jun 23, 2021 at 05:15:05PM +0200, Mark Kettenis wrote: > > > > Date: Wed, 23 Jun 2021 14:56:45 + > > > > From: Visa Hankala > > > > > > > > On Tue, Jun 22, 2021 at 09:46:22AM -0500, Scott Cheloha wrote: > > > > > On Mon, Jun 21, 2021 at 02:04:30PM +, 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 +, 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? Barrier-like removal is not reliable in interrupt context because an interrupt might have preempted the soft interrupt handler that the interrupt handler is about to remove. This applies to both MP and non-MP kernels. On a non-MP kernel, spinning, or sleeping, is not actually needed. Pending soft interrupts should run immediately when the system priority level allows them. If SPL is none on entry to softintr_disestablish(), the handler should have finished already. If SPL blocks soft interrupts on entry, the dequeueing clears any pending soft interrupt.
Re: amd64: softintr_dispatch: remove kernel lock
> Date: Wed, 23 Jun 2021 15:32:03 + > From: Visa Hankala > > On Wed, Jun 23, 2021 at 05:15:05PM +0200, Mark Kettenis wrote: > > > Date: Wed, 23 Jun 2021 14:56:45 + > > > From: Visa Hankala > > > > > > On Tue, Jun 22, 2021 at 09:46:22AM -0500, Scott Cheloha wrote: > > > > On Mon, Jun 21, 2021 at 02:04:30PM +, 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 +, 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?
Re: amd64: softintr_dispatch: remove kernel lock
On Wed, Jun 23, 2021 at 05:15:05PM +0200, Mark Kettenis wrote: > > Date: Wed, 23 Jun 2021 14:56:45 + > > From: Visa Hankala > > > > On Tue, Jun 22, 2021 at 09:46:22AM -0500, Scott Cheloha wrote: > > > On Mon, Jun 21, 2021 at 02:04:30PM +, 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 +, 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.
Re: amd64: softintr_dispatch: remove kernel lock
> Date: Wed, 23 Jun 2021 14:56:45 + > From: Visa Hankala > > On Tue, Jun 22, 2021 at 09:46:22AM -0500, Scott Cheloha wrote: > > On Mon, Jun 21, 2021 at 02:04:30PM +, 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 +, 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.
Re: amd64: softintr_dispatch: remove kernel lock
On Tue, Jun 22, 2021 at 09:46:22AM -0500, Scott Cheloha wrote: > On Mon, Jun 21, 2021 at 02:04:30PM +, 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 +, 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.
Re: amd64: softintr_dispatch: remove kernel lock
On Mon, Jun 21, 2021 at 02:04:30PM +, 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 +, 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?
Re: amd64: softintr_dispatch: remove kernel lock
On Thu, May 27, 2021 at 07:40:26PM -0500, Scott Cheloha wrote:
> On Sun, May 23, 2021 at 09:05:24AM +, 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.
Index: arch/amd64/amd64/softintr.c
===
RCS file: src/sys/arch/amd64/amd64/softintr.c,v
retrieving revision 1.10
diff -u -p -r1.10 softintr.c
--- arch/amd64/amd64/softintr.c 11 Sep 2020 09:27:09 - 1.10
+++ arch/amd64/amd64/softintr.c 21 Jun 2021 13:32:33 -
@@ -37,12 +37,33 @@
#include
#include
#include
+#include
+#include
#include
#include
-struct x86_soft_intr x86_soft_intrs[X86_NSOFTINTR];
+struct x86_soft_intrhand {
+ TAILQ_ENTRY(x86_soft_intrhand) sih_q;
+ void(*sih_fn)(void *);
+ void*sih_arg;
+ struct cpu_info *sih_runner;
+ int sih_which;
+ unsigned short sih_flags;
+ unsigned short sih_state;
+};
+
+#define SIF_MPSAFE 0x0001
+
+#define SIS_DYING 0x0001
+#define SIS_PENDING0x0002
+#define SIS_RESTART0x0004
+
+TAILQ_HEAD(x86_soft_intr_queue, x86_soft_intrhand);
+
+struct x86_soft_intr_queue softintr_queue[X86_NSOFTINTR];
+struct mutex softintr_lock = MUTEX_INITIALIZER(IPL_HIGH);
const int x86_soft_intr_to_ssir[X86_NSOFTINTR] = {
SIR_CLOCK,
@@ -58,15 +79,10 @@ const int x86_soft_intr_to_ssir[X86_NSOF
void
softintr_init(void)
{
- struct x86_soft_intr *si;
int i;
- for (i = 0; i < X86_NSOFTINTR; i++) {
- si = _soft_intrs[i];
- TAILQ_INIT(>softintr_q);
- mtx_init(>softintr_lock, IPL_HIGH);
- si->softintr_ssir = x86_soft_intr_to_ssir[i];
- }
+ for (i = 0; i < nitems(softintr_queue); i++)
+ TAILQ_INIT(_queue[i]);
}
/*
@@ -78,31 +94,44 @@ void
softintr_dispatch(int which)
{
struct cpu_info *ci = curcpu();
- struct x86_soft_intr *si = _soft_intrs[which];
+ struct x86_soft_intr_queue *queue = _queue[which];
struct x86_soft_intrhand *sih;
int floor;
floor = ci->ci_handled_intr_level;
ci->ci_handled_intr_level = ci->ci_ilevel;
- KERNEL_LOCK();
- for (;;) {
- mtx_enter(>softintr_lock);
- sih = TAILQ_FIRST(>softintr_q);
- if (sih == NULL) {
- mtx_leave(>softintr_lock);
- break;
+ mtx_enter(_lock);
+ while ((sih = TAILQ_FIRST(queue)) != NULL) {
+ KASSERT((sih->sih_state & (SIS_PENDING | SIS_RESTART)) ==
+ SIS_PENDING);
+ KASSERT(sih->sih_runner == NULL);
+
+ sih->sih_state &= ~SIS_PENDING;
+ TAILQ_REMOVE(queue, sih, sih_q);
+ sih->sih_runner = ci;
+ mtx_leave(_lock);
+
+ if (sih->sih_flags & SIF_MPSAFE) {
+ (*sih->sih_fn)(sih->sih_arg);
+ } else {
+ KERNEL_LOCK();
+ (*sih->sih_fn)(sih->sih_arg);
+ KERNEL_UNLOCK();
}
- TAILQ_REMOVE(>softintr_q, sih, sih_q);
- sih->sih_pending = 0;
- uvmexp.softs++;
-
- mtx_leave(>softintr_lock);
+ mtx_enter(_lock);
+ KASSERT((sih->sih_state & SIS_PENDING) == 0);
+ sih->sih_runner = NULL;
+ if (sih->sih_state & SIS_RESTART) {
+ TAILQ_INSERT_TAIL(queue, sih, sih_q);
+ sih->sih_state |= SIS_PENDING;
+ sih->sih_state &= ~SIS_RESTART;
+ }
- (*sih->sih_fn)(sih->sih_arg);
+ uvmexp.softs++;
}
- KERNEL_UNLOCK();
+ mtx_leave(_lock);
ci->ci_handled_intr_level = floor;
}
@@ -115,9 +144,13 @@
Re: amd64: softintr_dispatch: remove kernel lock
On Sun, May 23, 2021 at 09:05:24AM +, Visa Hankala wrote:
>
> [...]
>
> Here is a revised patch with a slightly altered approach.
>
> I have replaced the per-CPU queues with system-wide queues to make
> the code simpler.
I think that's fine to start with. We don't have many softintrs. We
can go per-CPU later if there is an issue with performance.
> 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.
> softintr_disestablish() now calls sched_barrier() to drain processing.
> This should also prevent the deadlock with the kernel lock.
>
> It appeared tricky to add witness(4) integration. witness(4) treats the
> kernel lock as a sleepable lock to avoid ordering issues with rwlocks.
> This creates a dilemma in softintr_dispatch() because of nesting.
> The natural lock class for the required soft interrupt pseudo-lock is
> mutex, but that does not work if an MP-safe soft interrupt got preempted
> by a higher IPL non-MP-safe soft interrupt.
>
> The dilemma could be worked around by preventing nesting when witness(4)
> is enabled, altering the system's behaviour. Another approach is to skip
> the pseudo-lock handling when processing is nested.
Altering the system's behavior sounds like a bad idea.
> However, as soft interrupt handlers cannot use rwlocks, for deadlock
> prevention it should be sufficient to ensure that the caller of
> softintr_disestablish() does not hold spinning locks (the kernel lock
> excluded). Hence softintr_disestablish() calls assertwaitok() at the
> start. The assertion is actually implied by sched_barrier().
I like your implementation a lot better than mine.
Misc. thoughts below, though I don't have any real complaints.
> Index: arch/amd64/amd64/softintr.c
> ===
> RCS file: src/sys/arch/amd64/amd64/softintr.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 softintr.c
> --- arch/amd64/amd64/softintr.c 11 Sep 2020 09:27:09 - 1.10
> +++ arch/amd64/amd64/softintr.c 23 May 2021 08:56:28 -
> @@ -37,12 +37,33 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
>
> #include
>
> -struct x86_soft_intr x86_soft_intrs[X86_NSOFTINTR];
> +struct x86_soft_intrhand {
> + STAILQ_ENTRY(x86_soft_intrhand) sih_q;
> + void(*sih_fn)(void *);
> + void*sih_arg;
> + struct cpu_info *sih_runner;
> + int sih_which;
> + unsigned short sih_flags;
> + unsigned short sih_state;
> +};
> +
> +#define SIF_MPSAFE 0x0001
> +
> +#define SIS_DYING0x0001
> +#define SIS_PENDING 0x0002
> +#define SIS_RESTART 0x0004
> +
> +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?
> +struct mutex softintr_lock = MUTEX_INITIALIZER(IPL_HIGH);
>
> const int x86_soft_intr_to_ssir[X86_NSOFTINTR] = {
> SIR_CLOCK,
> @@ -58,15 +79,10 @@ const int x86_soft_intr_to_ssir[X86_NSOF
> void
> softintr_init(void)
> {
> - struct x86_soft_intr *si;
> int i;
>
> - for (i = 0; i < X86_NSOFTINTR; i++) {
> - si = _soft_intrs[i];
> - TAILQ_INIT(>softintr_q);
> - mtx_init(>softintr_lock, IPL_HIGH);
> - si->softintr_ssir = x86_soft_intr_to_ssir[i];
> - }
> + for (i = 0; i < X86_NSOFTINTR; i++)
> + STAILQ_INIT(_queue[i]);
Nit: nitems(), if you want.
> }
>
> /*
> @@ -78,31 +94,45 @@ void
> softintr_dispatch(int which)
> {
> struct cpu_info *ci = curcpu();
> - struct x86_soft_intr *si = _soft_intrs[which];
> + struct x86_soft_intr_queue *queue = _queue[which];
> struct x86_soft_intrhand *sih;
> int floor;
>
> floor = ci->ci_handled_intr_level;
> ci->ci_handled_intr_level = ci->ci_ilevel;
>
> - KERNEL_LOCK();
> - for (;;) {
> - mtx_enter(>softintr_lock);
> - sih = TAILQ_FIRST(>softintr_q);
> - if (sih == NULL) {
> - mtx_leave(>softintr_lock);
> - break;
> + mtx_enter(_lock);
> + while ((sih = STAILQ_FIRST(queue)) != NULL) {
> + KASSERT((sih->sih_state & (SIS_PENDING | SIS_RESTART)) ==
> + SIS_PENDING);
> + KASSERT(sih->sih_runner == NULL);
> +
> + sih->sih_state &= ~SIS_PENDING;
Nit: you could use CLR(), ISSET(), etc., if you
Re: amd64: softintr_dispatch: remove kernel lock
On Sat, May 22, 2021 at 02:23:57PM +, Visa Hankala wrote: > On Sat, May 22, 2021 at 02:33:47PM +0200, Mark Kettenis wrote: > > > Date: Sat, 22 May 2021 11:11:38 + > > > From: Visa Hankala > > > > > > On Wed, May 19, 2021 at 05:11:09PM -0500, Scott Cheloha wrote: > > > > Hi, > > > > > > > > visa@ says I need to unlock softintr_dispatch() before I can > > > > unlock softclock(), so let's do that. > > > > > > > > Additionally, when we call softintr_disestablish() we want to wait for > > > > the underlying softintr handle to finish running if it is running. > > > > > > > > We can start with amd64. > > > > > > > > I think this approach will work: > > > > > > > > - Keep a pointer to the running softintr, if any, in the queue. NULL > > > > the pointer when we return from sih_func(). > > > > > > > > - Take/release the kernel lock if the SI_MPSAFE flag is present when > > > > we enter/leave sih_func(). > > > > > > > > - If the handle is running when you call softintr_disestablish(), spin > > > > until the handle isn't running anymore and retry. > > > > > > > > There is no softintr manpage but I think it is understood that > > > > softintr_disestablish() is only safe to call from a process context, > > > > otherwise you may deadlock. Maybe we should do splassert(IPL_NONE)? > > > > > > > > We could probably sleep here instead of spinning. We'd have to change > > > > every softintr_disestablish() implementation to do that, though. > > > > Otherwise you'd have different behavior on different platforms. > > > > > > I think your diff does not pay enough attention to the fact that soft > > > interrupts are handled by all CPUs. I think the diff that I posted > > > a while ago [1] is better in that respect. > > > > > > Two biggest things that I do not like in my original diff are > > > synchronization of handler execution, and use of the SMR barrier. > > > > > > [1] https://marc.info/?l=openbsd-tech=162092714911609 > > > > > > The kernel lock has guaranteed that at most one CPU is able to run > > > a given soft interrupt handler at a time. My diff used a mutex to > > > prevent concurrent execution. However, it is wasteful to spin. It would > > > be more economical to let the current runner of the handler re-execute > > > the code. > > > > > > The SMR barrier in softintr_disestablish() was a trick to drain any > > > pending activity. However, it made me feel uneasy because I have not > > > checked every caller of softintr_disestablish(). My main worry is not > > > the latency but unexpected side effects. > > > > > > Below is a revised diff that improves the above two points. > > > > > > When a soft interrupt handler is scheduled, it is assigned to a CPU. > > > That CPU will keep running the handler as long as there are pending > > > requests. Once all pending requests have been drained, the CPU > > > relinquishes its hold of the handler. This provides natural > > > serialization. > > > > > > Now softintr_disestablish() uses spinning for draining activity. > > > I still have slight qualms about this, though, because the feature > > > has not been so explicit before. Integration with witness(4) might be > > > in order. > > > > > > softintr_disestablish() uses READ_ONCE() to enforce reloading of the > > > value in the busy-wait loop. This way the variable does not need to be > > > volatile. (As yet another option, CPU_BUSY_CYCLE() could always > > > imply memory clobbering, which should make an optimizing compiler > > > redo the load.) For consistency with this READ_ONCE(), WRITE_ONCE() is > > > used whenever the variable is written, excluding the initialization. > > > > > > The patch uses a single mutex for access serialization. The old code > > > has used one mutex per each soft IPL level, but I am not sure how > > > useful that has been. I think it would be better to have a separate > > > mutex for each CPU. However, the increased code complexity might not > > > be worthwhile at the moment. Even having the per-CPU queues has > > > a whiff of being somewhat overkill. > > > > A few comments: > > > > * Looking at amd64 in isolation does not make sense. Like a lot of MD > > code in OpenBSD the softintr code was copied from whatever > > Net/FreeBSD had at the time, with no attempt at unification (it > > works, check it in, don't go back to clean it up). However, with > > powerpc64 and riscv64 we try to do things a little bit better in > > that area. So arm64, powerpc64 and riscv64 share the same softintr > > implementation that already implements softintr_establish_flags() > > with SOFTINTR_ESTABLISH_MPSAFE. Now we haven't used that flag > > anywhere in our tree yet, so the code might be completely busted. > > But it may make a lot of sense to migrate other architectures to the > > same codebase. > > I know that the implementations should be unified, or made machine > independent as far as possible. I have focused on amd64's code to get > the MP-safe processing
Re: amd64: softintr_dispatch: remove kernel lock
On Sat, May 22, 2021 at 02:33:47PM +0200, Mark Kettenis wrote: > > Date: Sat, 22 May 2021 11:11:38 + > > From: Visa Hankala > > > > On Wed, May 19, 2021 at 05:11:09PM -0500, Scott Cheloha wrote: > > > Hi, > > > > > > visa@ says I need to unlock softintr_dispatch() before I can > > > unlock softclock(), so let's do that. > > > > > > Additionally, when we call softintr_disestablish() we want to wait for > > > the underlying softintr handle to finish running if it is running. > > > > > > We can start with amd64. > > > > > > I think this approach will work: > > > > > > - Keep a pointer to the running softintr, if any, in the queue. NULL > > > the pointer when we return from sih_func(). > > > > > > - Take/release the kernel lock if the SI_MPSAFE flag is present when > > > we enter/leave sih_func(). > > > > > > - If the handle is running when you call softintr_disestablish(), spin > > > until the handle isn't running anymore and retry. > > > > > > There is no softintr manpage but I think it is understood that > > > softintr_disestablish() is only safe to call from a process context, > > > otherwise you may deadlock. Maybe we should do splassert(IPL_NONE)? > > > > > > We could probably sleep here instead of spinning. We'd have to change > > > every softintr_disestablish() implementation to do that, though. > > > Otherwise you'd have different behavior on different platforms. > > > > I think your diff does not pay enough attention to the fact that soft > > interrupts are handled by all CPUs. I think the diff that I posted > > a while ago [1] is better in that respect. > > > > Two biggest things that I do not like in my original diff are > > synchronization of handler execution, and use of the SMR barrier. > > > > [1] https://marc.info/?l=openbsd-tech=162092714911609 > > > > The kernel lock has guaranteed that at most one CPU is able to run > > a given soft interrupt handler at a time. My diff used a mutex to > > prevent concurrent execution. However, it is wasteful to spin. It would > > be more economical to let the current runner of the handler re-execute > > the code. > > > > The SMR barrier in softintr_disestablish() was a trick to drain any > > pending activity. However, it made me feel uneasy because I have not > > checked every caller of softintr_disestablish(). My main worry is not > > the latency but unexpected side effects. > > > > Below is a revised diff that improves the above two points. > > > > When a soft interrupt handler is scheduled, it is assigned to a CPU. > > That CPU will keep running the handler as long as there are pending > > requests. Once all pending requests have been drained, the CPU > > relinquishes its hold of the handler. This provides natural > > serialization. > > > > Now softintr_disestablish() uses spinning for draining activity. > > I still have slight qualms about this, though, because the feature > > has not been so explicit before. Integration with witness(4) might be > > in order. > > > > softintr_disestablish() uses READ_ONCE() to enforce reloading of the > > value in the busy-wait loop. This way the variable does not need to be > > volatile. (As yet another option, CPU_BUSY_CYCLE() could always > > imply memory clobbering, which should make an optimizing compiler > > redo the load.) For consistency with this READ_ONCE(), WRITE_ONCE() is > > used whenever the variable is written, excluding the initialization. > > > > The patch uses a single mutex for access serialization. The old code > > has used one mutex per each soft IPL level, but I am not sure how > > useful that has been. I think it would be better to have a separate > > mutex for each CPU. However, the increased code complexity might not > > be worthwhile at the moment. Even having the per-CPU queues has > > a whiff of being somewhat overkill. > > A few comments: > > * Looking at amd64 in isolation does not make sense. Like a lot of MD > code in OpenBSD the softintr code was copied from whatever > Net/FreeBSD had at the time, with no attempt at unification (it > works, check it in, don't go back to clean it up). However, with > powerpc64 and riscv64 we try to do things a little bit better in > that area. So arm64, powerpc64 and riscv64 share the same softintr > implementation that already implements softintr_establish_flags() > with SOFTINTR_ESTABLISH_MPSAFE. Now we haven't used that flag > anywhere in our tree yet, so the code might be completely busted. > But it may make a lot of sense to migrate other architectures to the > same codebase. I know that the implementations should be unified, or made machine independent as far as possible. I have focused on amd64's code to get the MP-safe processing working sooner on that platform - at the cost of increased code debt. > * The softintr_disestablish() function isn't used a lot in our tree. > It may make sense to postpone worrying about safely disestablishing > mpsafe soft interrupts for now and simply
Re: amd64: softintr_dispatch: remove kernel lock
Am Sat, May 22, 2021 at 02:33:47PM +0200 schrieb Mark Kettenis: > > Date: Sat, 22 May 2021 11:11:38 + > > From: Visa Hankala > > > > On Wed, May 19, 2021 at 05:11:09PM -0500, Scott Cheloha wrote: > > > Hi, > > > > > > visa@ says I need to unlock softintr_dispatch() before I can > > > unlock softclock(), so let's do that. > > > > > > Additionally, when we call softintr_disestablish() we want to wait for > > > the underlying softintr handle to finish running if it is running. > > > > > > We can start with amd64. > > > > > > I think this approach will work: > > > > > > - Keep a pointer to the running softintr, if any, in the queue. NULL > > > the pointer when we return from sih_func(). > > > > > > - Take/release the kernel lock if the SI_MPSAFE flag is present when > > > we enter/leave sih_func(). > > > > > > - If the handle is running when you call softintr_disestablish(), spin > > > until the handle isn't running anymore and retry. > > > > > > There is no softintr manpage but I think it is understood that > > > softintr_disestablish() is only safe to call from a process context, > > > otherwise you may deadlock. Maybe we should do splassert(IPL_NONE)? > > > > > > We could probably sleep here instead of spinning. We'd have to change > > > every softintr_disestablish() implementation to do that, though. > > > Otherwise you'd have different behavior on different platforms. > > > > I think your diff does not pay enough attention to the fact that soft > > interrupts are handled by all CPUs. I think the diff that I posted > > a while ago [1] is better in that respect. > > > > Two biggest things that I do not like in my original diff are > > synchronization of handler execution, and use of the SMR barrier. > > > > [1] https://marc.info/?l=openbsd-tech=162092714911609 > > > > The kernel lock has guaranteed that at most one CPU is able to run > > a given soft interrupt handler at a time. My diff used a mutex to > > prevent concurrent execution. However, it is wasteful to spin. It would > > be more economical to let the current runner of the handler re-execute > > the code. > > > > The SMR barrier in softintr_disestablish() was a trick to drain any > > pending activity. However, it made me feel uneasy because I have not > > checked every caller of softintr_disestablish(). My main worry is not > > the latency but unexpected side effects. > > > > Below is a revised diff that improves the above two points. > > > > When a soft interrupt handler is scheduled, it is assigned to a CPU. > > That CPU will keep running the handler as long as there are pending > > requests. Once all pending requests have been drained, the CPU > > relinquishes its hold of the handler. This provides natural > > serialization. > > > > Now softintr_disestablish() uses spinning for draining activity. > > I still have slight qualms about this, though, because the feature > > has not been so explicit before. Integration with witness(4) might be > > in order. > > > > softintr_disestablish() uses READ_ONCE() to enforce reloading of the > > value in the busy-wait loop. This way the variable does not need to be > > volatile. (As yet another option, CPU_BUSY_CYCLE() could always > > imply memory clobbering, which should make an optimizing compiler > > redo the load.) For consistency with this READ_ONCE(), WRITE_ONCE() is > > used whenever the variable is written, excluding the initialization. > > > > The patch uses a single mutex for access serialization. The old code > > has used one mutex per each soft IPL level, but I am not sure how > > useful that has been. I think it would be better to have a separate > > mutex for each CPU. However, the increased code complexity might not > > be worthwhile at the moment. Even having the per-CPU queues has > > a whiff of being somewhat overkill. > > A few comments: > > * Looking at amd64 in isolation does not make sense. Like a lot of MD > code in OpenBSD the softintr code was copied from whatever > Net/FreeBSD had at the time, with no attempt at unification (it > works, check it in, don't go back to clean it up). However, with > powerpc64 and riscv64 we try to do things a little bit better in > that area. So arm64, powerpc64 and riscv64 share the same softintr > implementation that already implements softintr_establish_flags() > with SOFTINTR_ESTABLISH_MPSAFE. Now we haven't used that flag > anywhere in our tree yet, so the code might be completely busted. > But it may make a lot of sense to migrate other architectures to the > same codebase. > > * The softintr_disestablish() function isn't used a lot in our tree. > It may make sense to postpone worrying about safely disestablishing > mpsafe soft interrupts for now and simply panic if someone tries to > do this. > > * Wouldn't it make sense for an mpsafe soft interrupt to protect > itself from running simultaniously on multiple CPUs? It probably > already needs some sort of lock to protect the
Re: amd64: softintr_dispatch: remove kernel lock
> Date: Sat, 22 May 2021 11:11:38 + > From: Visa Hankala > > On Wed, May 19, 2021 at 05:11:09PM -0500, Scott Cheloha wrote: > > Hi, > > > > visa@ says I need to unlock softintr_dispatch() before I can > > unlock softclock(), so let's do that. > > > > Additionally, when we call softintr_disestablish() we want to wait for > > the underlying softintr handle to finish running if it is running. > > > > We can start with amd64. > > > > I think this approach will work: > > > > - Keep a pointer to the running softintr, if any, in the queue. NULL > > the pointer when we return from sih_func(). > > > > - Take/release the kernel lock if the SI_MPSAFE flag is present when > > we enter/leave sih_func(). > > > > - If the handle is running when you call softintr_disestablish(), spin > > until the handle isn't running anymore and retry. > > > > There is no softintr manpage but I think it is understood that > > softintr_disestablish() is only safe to call from a process context, > > otherwise you may deadlock. Maybe we should do splassert(IPL_NONE)? > > > > We could probably sleep here instead of spinning. We'd have to change > > every softintr_disestablish() implementation to do that, though. > > Otherwise you'd have different behavior on different platforms. > > I think your diff does not pay enough attention to the fact that soft > interrupts are handled by all CPUs. I think the diff that I posted > a while ago [1] is better in that respect. > > Two biggest things that I do not like in my original diff are > synchronization of handler execution, and use of the SMR barrier. > > [1] https://marc.info/?l=openbsd-tech=162092714911609 > > The kernel lock has guaranteed that at most one CPU is able to run > a given soft interrupt handler at a time. My diff used a mutex to > prevent concurrent execution. However, it is wasteful to spin. It would > be more economical to let the current runner of the handler re-execute > the code. > > The SMR barrier in softintr_disestablish() was a trick to drain any > pending activity. However, it made me feel uneasy because I have not > checked every caller of softintr_disestablish(). My main worry is not > the latency but unexpected side effects. > > Below is a revised diff that improves the above two points. > > When a soft interrupt handler is scheduled, it is assigned to a CPU. > That CPU will keep running the handler as long as there are pending > requests. Once all pending requests have been drained, the CPU > relinquishes its hold of the handler. This provides natural > serialization. > > Now softintr_disestablish() uses spinning for draining activity. > I still have slight qualms about this, though, because the feature > has not been so explicit before. Integration with witness(4) might be > in order. > > softintr_disestablish() uses READ_ONCE() to enforce reloading of the > value in the busy-wait loop. This way the variable does not need to be > volatile. (As yet another option, CPU_BUSY_CYCLE() could always > imply memory clobbering, which should make an optimizing compiler > redo the load.) For consistency with this READ_ONCE(), WRITE_ONCE() is > used whenever the variable is written, excluding the initialization. > > The patch uses a single mutex for access serialization. The old code > has used one mutex per each soft IPL level, but I am not sure how > useful that has been. I think it would be better to have a separate > mutex for each CPU. However, the increased code complexity might not > be worthwhile at the moment. Even having the per-CPU queues has > a whiff of being somewhat overkill. A few comments: * Looking at amd64 in isolation does not make sense. Like a lot of MD code in OpenBSD the softintr code was copied from whatever Net/FreeBSD had at the time, with no attempt at unification (it works, check it in, don't go back to clean it up). However, with powerpc64 and riscv64 we try to do things a little bit better in that area. So arm64, powerpc64 and riscv64 share the same softintr implementation that already implements softintr_establish_flags() with SOFTINTR_ESTABLISH_MPSAFE. Now we haven't used that flag anywhere in our tree yet, so the code might be completely busted. But it may make a lot of sense to migrate other architectures to the same codebase. * The softintr_disestablish() function isn't used a lot in our tree. It may make sense to postpone worrying about safely disestablishing mpsafe soft interrupts for now and simply panic if someone tries to do this. * Wouldn't it make sense for an mpsafe soft interrupt to protect itself from running simultaniously on multiple CPUs? It probably already needs some sort of lock to protect the handler and other code running in process context on other CPUs. And some handlers may be safe to run simultaniously anyway. * I think we should avoid MAXCPU arrays if we can; adding stuff to struct cpu_info is probably a better approach here. Cheers, Mark
Re: amd64: softintr_dispatch: remove kernel lock
On Wed, May 19, 2021 at 05:11:09PM -0500, Scott Cheloha wrote:
> Hi,
>
> visa@ says I need to unlock softintr_dispatch() before I can
> unlock softclock(), so let's do that.
>
> Additionally, when we call softintr_disestablish() we want to wait for
> the underlying softintr handle to finish running if it is running.
>
> We can start with amd64.
>
> I think this approach will work:
>
> - Keep a pointer to the running softintr, if any, in the queue. NULL
> the pointer when we return from sih_func().
>
> - Take/release the kernel lock if the SI_MPSAFE flag is present when
> we enter/leave sih_func().
>
> - If the handle is running when you call softintr_disestablish(), spin
> until the handle isn't running anymore and retry.
>
> There is no softintr manpage but I think it is understood that
> softintr_disestablish() is only safe to call from a process context,
> otherwise you may deadlock. Maybe we should do splassert(IPL_NONE)?
>
> We could probably sleep here instead of spinning. We'd have to change
> every softintr_disestablish() implementation to do that, though.
> Otherwise you'd have different behavior on different platforms.
I think your diff does not pay enough attention to the fact that soft
interrupts are handled by all CPUs. I think the diff that I posted
a while ago [1] is better in that respect.
Two biggest things that I do not like in my original diff are
synchronization of handler execution, and use of the SMR barrier.
[1] https://marc.info/?l=openbsd-tech=162092714911609
The kernel lock has guaranteed that at most one CPU is able to run
a given soft interrupt handler at a time. My diff used a mutex to
prevent concurrent execution. However, it is wasteful to spin. It would
be more economical to let the current runner of the handler re-execute
the code.
The SMR barrier in softintr_disestablish() was a trick to drain any
pending activity. However, it made me feel uneasy because I have not
checked every caller of softintr_disestablish(). My main worry is not
the latency but unexpected side effects.
Below is a revised diff that improves the above two points.
When a soft interrupt handler is scheduled, it is assigned to a CPU.
That CPU will keep running the handler as long as there are pending
requests. Once all pending requests have been drained, the CPU
relinquishes its hold of the handler. This provides natural
serialization.
Now softintr_disestablish() uses spinning for draining activity.
I still have slight qualms about this, though, because the feature
has not been so explicit before. Integration with witness(4) might be
in order.
softintr_disestablish() uses READ_ONCE() to enforce reloading of the
value in the busy-wait loop. This way the variable does not need to be
volatile. (As yet another option, CPU_BUSY_CYCLE() could always
imply memory clobbering, which should make an optimizing compiler
redo the load.) For consistency with this READ_ONCE(), WRITE_ONCE() is
used whenever the variable is written, excluding the initialization.
The patch uses a single mutex for access serialization. The old code
has used one mutex per each soft IPL level, but I am not sure how
useful that has been. I think it would be better to have a separate
mutex for each CPU. However, the increased code complexity might not
be worthwhile at the moment. Even having the per-CPU queues has
a whiff of being somewhat overkill.
Index: arch/amd64/amd64/softintr.c
===
RCS file: src/sys/arch/amd64/amd64/softintr.c,v
retrieving revision 1.10
diff -u -p -r1.10 softintr.c
--- arch/amd64/amd64/softintr.c 11 Sep 2020 09:27:09 - 1.10
+++ arch/amd64/amd64/softintr.c 22 May 2021 11:07:23 -
@@ -36,13 +36,37 @@
#include
#include
+#include
#include
+#include
+#include
+#include
#include
#include
-struct x86_soft_intr x86_soft_intrs[X86_NSOFTINTR];
+struct x86_soft_intr_percpu;
+
+struct x86_soft_intrhand {
+ TAILQ_ENTRY(x86_soft_intrhand) sih_q;
+ void(*sih_fn)(void *);
+ void*sih_arg;
+ struct x86_soft_intr_percpu *sih_owner;
+ int sih_which;
+ unsigned short sih_flags;
+ unsigned short sih_pending;
+};
+
+#define SIF_DYING 0x0001
+#define SIF_MPSAFE 0x0002
+
+struct x86_soft_intr_percpu {
+ TAILQ_HEAD(, x86_soft_intrhand) sip_queue[X86_NSOFTINTR];
+} __aligned(CACHELINESIZE);
+
+struct x86_soft_intr_percpu x86_soft_intr_percpu[MAXCPUS];
+struct mutex softintr_lock = MUTEX_INITIALIZER(IPL_HIGH);
const int x86_soft_intr_to_ssir[X86_NSOFTINTR] = {
SIR_CLOCK,
@@ -58,14 +82,13 @@ const int x86_soft_intr_to_ssir[X86_NSOF
void
softintr_init(void)
{
- struct x86_soft_intr *si;
- int i;
+ struct x86_soft_intr_percpu *sip;
+ int i, j;
- for (i = 0; i < X86_NSOFTINTR; i++) {
-
