svn commit: r336635 - in head/sys: kern sys
Andriy Gapon
avg at FreeBSD.org
Mon Jul 23 12:51:24 UTC 2018
Author: avg
Date: Mon Jul 23 12:51:23 2018
New Revision: 336635
URL: https://svnweb.freebsd.org/changeset/base/336635
Log:
change interrupt event's list of handlers from TAILQ to CK_SLIST
The primary reason for this commit is to separate mechanical and nearly
mechanical code changes from an upcoming fix for unsafe teardown of
shared interrupt handlers that have only filters (see D15905).
The technical rationale is that SLIST is sufficient. The only operation
that gets worse performance -- O(n) instead of O(1) is a removal of a
handler, but it is not a critical operation and the list is expected to
be rather short.
Additionally, it is easier to reason about SLIST when considering the
concurrent lock-free access to the list from the interrupt context and
the interrupt thread.
CK_SLIST is used because the upcoming change depends on the memory order
provided by CK_SLIST insert and the fact that CL_SLIST remove does not
trash the linkage in a removed element.
While here, I also fixed a couple of whitespace issues, made code under
ifdef notyet compilable, added a lock assertion to ithread_update() and
made intr_event_execute_handlers() static as it had no external callers.
Reviewed by: cem (earlier version)
MFC after: 4 weeks
Differential Revision: https://reviews.freebsd.org/D16016
Modified:
head/sys/kern/kern_intr.c
head/sys/sys/interrupt.h
Modified: head/sys/kern/kern_intr.c
==============================================================================
--- head/sys/kern/kern_intr.c Mon Jul 23 11:21:43 2018 (r336634)
+++ head/sys/kern/kern_intr.c Mon Jul 23 12:51:23 2018 (r336635)
@@ -160,12 +160,13 @@ ithread_update(struct intr_thread *ithd)
ie = ithd->it_event;
td = ithd->it_thread;
+ mtx_assert(&ie->ie_lock, MA_OWNED);
/* Determine the overall priority of this event. */
- if (TAILQ_EMPTY(&ie->ie_handlers))
+ if (CK_SLIST_EMPTY(&ie->ie_handlers))
pri = PRI_MAX_ITHD;
else
- pri = TAILQ_FIRST(&ie->ie_handlers)->ih_pri;
+ pri = CK_SLIST_FIRST(&ie->ie_handlers)->ih_pri;
/* Update name and priority. */
strlcpy(td->td_name, ie->ie_fullname, sizeof(td->td_name));
@@ -195,7 +196,7 @@ intr_event_update(struct intr_event *ie)
space = 1;
/* Run through all the handlers updating values. */
- TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
+ CK_SLIST_FOREACH(ih, &ie->ie_handlers, ih_next) {
if (strlen(ie->ie_fullname) + strlen(ih->ih_name) + 1 <
sizeof(ie->ie_fullname)) {
strcat(ie->ie_fullname, " ");
@@ -257,7 +258,7 @@ intr_event_create(struct intr_event **event, void *sou
ie->ie_flags = flags;
ie->ie_irq = irq;
ie->ie_cpu = NOCPU;
- TAILQ_INIT(&ie->ie_handlers);
+ CK_SLIST_INIT(&ie->ie_handlers);
mtx_init(&ie->ie_lock, "intr event", NULL, MTX_DEF);
va_start(ap, fmt);
@@ -378,7 +379,7 @@ intr_lookup(int irq)
TAILQ_FOREACH(ie, &event_list, ie_list)
if (ie->ie_irq == irq &&
(ie->ie_flags & IE_SOFT) == 0 &&
- TAILQ_FIRST(&ie->ie_handlers) != NULL)
+ CK_SLIST_FIRST(&ie->ie_handlers) != NULL)
break;
mtx_unlock(&event_lock);
return (ie);
@@ -474,7 +475,7 @@ intr_event_destroy(struct intr_event *ie)
mtx_lock(&event_lock);
mtx_lock(&ie->ie_lock);
- if (!TAILQ_EMPTY(&ie->ie_handlers)) {
+ if (!CK_SLIST_EMPTY(&ie->ie_handlers)) {
mtx_unlock(&ie->ie_lock);
mtx_unlock(&event_lock);
return (EBUSY);
@@ -504,7 +505,7 @@ ithread_create(const char *name)
error = kproc_kthread_add(ithread_loop, ithd, &intrproc,
&td, RFSTOPPED | RFHIGHPID,
- 0, "intr", "%s", name);
+ 0, "intr", "%s", name);
if (error)
panic("kproc_create() failed with %d", error);
thread_lock(td);
@@ -539,6 +540,7 @@ intr_event_add_handler(struct intr_event *ie, const ch
enum intr_type flags, void **cookiep)
{
struct intr_handler *ih, *temp_ih;
+ struct intr_handler **prevptr;
struct intr_thread *it;
if (ie == NULL || name == NULL || (handler == NULL && filter == NULL))
@@ -561,9 +563,9 @@ intr_event_add_handler(struct intr_event *ie, const ch
/* We can only have one exclusive handler in a event. */
mtx_lock(&ie->ie_lock);
- if (!TAILQ_EMPTY(&ie->ie_handlers)) {
+ if (!CK_SLIST_EMPTY(&ie->ie_handlers)) {
if ((flags & INTR_EXCL) ||
- (TAILQ_FIRST(&ie->ie_handlers)->ih_flags & IH_EXCLUSIVE)) {
+ (CK_SLIST_FIRST(&ie->ie_handlers)->ih_flags & IH_EXCLUSIVE)) {
mtx_unlock(&ie->ie_lock);
free(ih, M_ITHREAD);
return (EINVAL);
@@ -588,14 +590,12 @@ intr_event_add_handler(struct intr_event *ie, const ch
}
/* Add the new handler to the event in priority order. */
- TAILQ_FOREACH(temp_ih, &ie->ie_handlers, ih_next) {
+ CK_SLIST_FOREACH_PREVPTR(temp_ih, prevptr, &ie->ie_handlers, ih_next) {
if (temp_ih->ih_pri > ih->ih_pri)
break;
}
- if (temp_ih == NULL)
- TAILQ_INSERT_TAIL(&ie->ie_handlers, ih, ih_next);
- else
- TAILQ_INSERT_BEFORE(temp_ih, ih, ih_next);
+ CK_SLIST_INSERT_PREVPTR(prevptr, temp_ih, ih, ih_next);
+
intr_event_update(ie);
CTR3(KTR_INTR, "%s: added %s to %s", __func__, ih->ih_name,
@@ -621,7 +621,7 @@ intr_event_describe_handler(struct intr_event *ie, voi
mtx_lock(&ie->ie_lock);
#ifdef INVARIANTS
- TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
+ CK_SLIST_FOREACH(ih, &ie->ie_handlers, ih_next) {
if (ih == cookie)
break;
}
@@ -721,15 +721,13 @@ _intr_drain(int irq)
return;
}
-
int
intr_event_remove_handler(void *cookie)
{
struct intr_handler *handler = (struct intr_handler *)cookie;
struct intr_event *ie;
-#ifdef INVARIANTS
struct intr_handler *ih;
-#endif
+ struct intr_handler **prevptr;
#ifdef notyet
int dead;
#endif
@@ -740,25 +738,26 @@ intr_event_remove_handler(void *cookie)
KASSERT(ie != NULL,
("interrupt handler \"%s\" has a NULL interrupt event",
handler->ih_name));
+
mtx_lock(&ie->ie_lock);
CTR3(KTR_INTR, "%s: removing %s from %s", __func__, handler->ih_name,
ie->ie_name);
-#ifdef INVARIANTS
- TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next)
+ CK_SLIST_FOREACH_PREVPTR(ih, prevptr, &ie->ie_handlers, ih_next) {
if (ih == handler)
- goto ok;
- mtx_unlock(&ie->ie_lock);
- panic("interrupt handler \"%s\" not found in interrupt event \"%s\"",
- ih->ih_name, ie->ie_name);
-ok:
-#endif
+ break;
+ }
+ if (ih == NULL) {
+ panic("interrupt handler \"%s\" not found in "
+ "interrupt event \"%s\"", handler->ih_name, ie->ie_name);
+ }
+
/*
* If there is no ithread, then just remove the handler and return.
* XXX: Note that an INTR_FAST handler might be running on another
* CPU!
*/
if (ie->ie_thread == NULL) {
- TAILQ_REMOVE(&ie->ie_handlers, handler, ih_next);
+ CK_SLIST_REMOVE_PREVPTR(prevptr, ih, ih_next);
mtx_unlock(&ie->ie_lock);
free(handler, M_ITHREAD);
return (0);
@@ -789,11 +788,12 @@ ok:
*/
atomic_store_rel_int(&ie->ie_thread->it_need, 1);
} else
- TAILQ_REMOVE(&ie->ie_handlers, handler, ih_next);
+ CK_SLIST_REMOVE_PREVPTR(prevptr, ih, ih_next);
thread_unlock(ie->ie_thread->it_thread);
while (handler->ih_flags & IH_DEAD)
msleep(handler, &ie->ie_lock, 0, "iev_rmh", 0);
intr_event_update(ie);
+
#ifdef notyet
/*
* XXX: This could be bad in the case of ppbus(8). Also, I think
@@ -801,8 +801,8 @@ ok:
* interrupt.
*/
dead = 1;
- TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
- if (!(ih->ih_flags & IH_FAST)) {
+ CK_SLIST_FOREACH(ih, &ie->ie_handlers, ih_next) {
+ if (ih->ih_handler != NULL) {
dead = 0;
break;
}
@@ -828,7 +828,7 @@ intr_event_schedule_thread(struct intr_event *ie)
/*
* If no ithread or no handlers, then we have a stray interrupt.
*/
- if (ie == NULL || TAILQ_EMPTY(&ie->ie_handlers) ||
+ if (ie == NULL || CK_SLIST_EMPTY(&ie->ie_handlers) ||
ie->ie_thread == NULL)
return (EINVAL);
@@ -962,30 +962,35 @@ swi_remove(void *cookie)
return (intr_event_remove_handler(cookie));
}
-
-/*
- * This is a public function for use by drivers that mux interrupt
- * handlers for child devices from their interrupt handler.
- */
-void
+static void
intr_event_execute_handlers(struct proc *p, struct intr_event *ie)
{
- struct intr_handler *ih, *ihn;
+ struct intr_handler *ih, *ihn, *ihp;
- TAILQ_FOREACH_SAFE(ih, &ie->ie_handlers, ih_next, ihn) {
+ ihp = NULL;
+ CK_SLIST_FOREACH_SAFE(ih, &ie->ie_handlers, ih_next, ihn) {
/*
* If this handler is marked for death, remove it from
* the list of handlers and wake up the sleeper.
*/
if (ih->ih_flags & IH_DEAD) {
mtx_lock(&ie->ie_lock);
- TAILQ_REMOVE(&ie->ie_handlers, ih, ih_next);
+ if (ihp == NULL)
+ CK_SLIST_REMOVE_HEAD(&ie->ie_handlers, ih_next);
+ else
+ CK_SLIST_REMOVE_AFTER(ihp, ih_next);
ih->ih_flags &= ~IH_DEAD;
wakeup(ih);
mtx_unlock(&ie->ie_lock);
continue;
}
+ /*
+ * Now that we know that the current element won't be removed
+ * update the previous element.
+ */
+ ihp = ih;
+
/* Skip filter only handlers */
if (ih->ih_handler == NULL)
continue;
@@ -1157,7 +1162,7 @@ intr_event_handle(struct intr_event *ie, struct trapfr
#endif
/* An interrupt with no event or handlers is a stray interrupt. */
- if (ie == NULL || TAILQ_EMPTY(&ie->ie_handlers))
+ if (ie == NULL || CK_SLIST_EMPTY(&ie->ie_handlers))
return (EINVAL);
/*
@@ -1172,7 +1177,8 @@ intr_event_handle(struct intr_event *ie, struct trapfr
critical_enter();
oldframe = td->td_intr_frame;
td->td_intr_frame = frame;
- TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next) {
+
+ CK_SLIST_FOREACH(ih, &ie->ie_handlers, ih_next) {
if (ih->ih_filter == NULL) {
thread = 1;
continue;
@@ -1218,7 +1224,7 @@ intr_event_handle(struct intr_event *ie, struct trapfr
if (ie->ie_post_filter != NULL)
ie->ie_post_filter(ie->ie_source);
}
-
+
/* Schedule the ithread if needed. */
if (thread) {
int error __unused;
@@ -1364,7 +1370,7 @@ db_dump_intr_event(struct intr_event *ie, int handlers
db_printf("\n");
if (handlers)
- TAILQ_FOREACH(ih, &ie->ie_handlers, ih_next)
+ CK_SLIST_FOREACH(ih, &ie->ie_handlers, ih_next)
db_dump_intrhand(ih);
}
@@ -1379,7 +1385,7 @@ DB_SHOW_COMMAND(intr, db_show_intr)
verbose = strchr(modif, 'v') != NULL;
all = strchr(modif, 'a') != NULL;
TAILQ_FOREACH(ie, &event_list, ie_list) {
- if (!all && TAILQ_EMPTY(&ie->ie_handlers))
+ if (!all && CK_SLIST_EMPTY(&ie->ie_handlers))
continue;
db_dump_intr_event(ie, verbose);
if (db_pager_quit)
Modified: head/sys/sys/interrupt.h
==============================================================================
--- head/sys/sys/interrupt.h Mon Jul 23 11:21:43 2018 (r336634)
+++ head/sys/sys/interrupt.h Mon Jul 23 12:51:23 2018 (r336635)
@@ -33,6 +33,7 @@
#include <sys/_lock.h>
#include <sys/_mutex.h>
+#include <sys/ck.h>
struct intr_event;
struct intr_thread;
@@ -52,7 +53,7 @@ struct intr_handler {
char ih_name[MAXCOMLEN + 1]; /* Name of handler. */
struct intr_event *ih_event; /* Event we are connected to. */
int ih_need; /* Needs service. */
- TAILQ_ENTRY(intr_handler) ih_next; /* Next handler for this event. */
+ CK_SLIST_ENTRY(intr_handler) ih_next; /* Next handler for this event. */
u_char ih_pri; /* Priority of this handler. */
struct intr_thread *ih_thread; /* Ithread for filtered handler. */
};
@@ -105,7 +106,7 @@ struct intr_handler {
*/
struct intr_event {
TAILQ_ENTRY(intr_event) ie_list;
- TAILQ_HEAD(, intr_handler) ie_handlers; /* Interrupt handlers. */
+ CK_SLIST_HEAD(, intr_handler) ie_handlers; /* Interrupt handlers. */
char ie_name[MAXCOMLEN + 1]; /* Individual event name. */
char ie_fullname[MAXCOMLEN + 1];
struct mtx ie_lock;
@@ -174,7 +175,6 @@ int intr_event_create(struct intr_event **event, void
int intr_event_describe_handler(struct intr_event *ie, void *cookie,
const char *descr);
int intr_event_destroy(struct intr_event *ie);
-void intr_event_execute_handlers(struct proc *p, struct intr_event *ie);
int intr_event_handle(struct intr_event *ie, struct trapframe *frame);
int intr_event_remove_handler(void *cookie);
int intr_getaffinity(int irq, int mode, void *mask);
More information about the svn-src-head
mailing list