knlist_empty locking fix

Doug Ambrisko ambrisko at ambrisko.com
Thu Jan 26 21:32:03 UTC 2012


Ran into problems with running kqueue/aio with WITNESS etc.  Sometimes
things are locked sometimes not.  knlist_remove is called telling it
whether it is locked or not ie:
	 extern void    knlist_remove(struct knlist *knl, struct knote *kn, int islocked);
so I changed:
	extern int     knlist_empty(struct knlist *knl);
to:
	extern int     knlist_empty(struct knlist *knl, int islocked);

and then updated things to reflect that following what that state of the
lock for knlist_remove.  If it is not locked, it gets a lock and 
frees it after.

This now fixes a panic when a process using kqueue/aio is killed on
shutdown with WITNESS.

It changes an API/ABI so it probably can't merged back.  If there are
no objections then I'll commit it.

Here is the patch:

Index: fs/fifofs/fifo_vnops.c
===================================================================
RCS file: /cvs/src/sys/fs/fifofs/fifo_vnops.c,v
retrieving revision 1.157
diff -u -p -r1.157 fifo_vnops.c
--- fs/fifofs/fifo_vnops.c	16 Aug 2011 20:07:47 -0000	1.157
+++ fs/fifofs/fifo_vnops.c	26 Jan 2012 20:50:31 -0000
@@ -324,7 +324,7 @@ filt_fifordetach(struct knote *kn)
 
 	SOCKBUF_LOCK(&so->so_rcv);
 	knlist_remove(&so->so_rcv.sb_sel.si_note, kn, 1);
-	if (knlist_empty(&so->so_rcv.sb_sel.si_note))
+	if (knlist_empty(&so->so_rcv.sb_sel.si_note, 1))
 		so->so_rcv.sb_flags &= ~SB_KNOTE;
 	SOCKBUF_UNLOCK(&so->so_rcv);
 }
@@ -352,7 +352,7 @@ filt_fifowdetach(struct knote *kn)
 
 	SOCKBUF_LOCK(&so->so_snd);
 	knlist_remove(&so->so_snd.sb_sel.si_note, kn, 1);
-	if (knlist_empty(&so->so_snd.sb_sel.si_note))
+	if (knlist_empty(&so->so_snd.sb_sel.si_note, 1))
 		so->so_snd.sb_flags &= ~SB_KNOTE;
 	SOCKBUF_UNLOCK(&so->so_snd);
 }
Index: kern/kern_event.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.146
diff -u -p -r1.146 kern_event.c
--- kern/kern_event.c	16 Sep 2011 13:58:51 -0000	1.146
+++ kern/kern_event.c	26 Jan 2012 20:50:31 -0000
@@ -1650,7 +1650,7 @@ kqueue_close(struct file *fp, struct thr
 	KASSERT(kq->kq_refcnt == 1, ("other refs are out there!"));
 	fdp = kq->kq_fdp;
 
-	KASSERT(knlist_empty(&kq->kq_sel.si_note),
+	KASSERT(knlist_empty(&kq->kq_sel.si_note, 1),
 	    ("kqueue's knlist not empty"));
 
 	for (i = 0; i < kq->kq_knlistsize; i++) {
@@ -1735,7 +1735,7 @@ kqueue_wakeup(struct kqueue *kq)
 		if (!SEL_WAITING(&kq->kq_sel))
 			kq->kq_state &= ~KQ_SEL;
 	}
-	if (!knlist_empty(&kq->kq_sel.si_note))
+	if (!knlist_empty(&kq->kq_sel.si_note, 1))
 		kqueue_schedtask(kq);
 	if ((kq->kq_state & KQ_ASYNC) == KQ_ASYNC) {
 		pgsigio(&kq->kq_sigio, SIGIO, 0);
@@ -1867,10 +1867,18 @@ knlist_remove_inevent(struct knlist *knl
 }
 
 int
-knlist_empty(struct knlist *knl)
+knlist_empty(struct knlist *knl, int knlislocked)
 {
-	KNL_ASSERT_LOCKED(knl);
-	return SLIST_EMPTY(&knl->kl_list);
+	int error;
+
+	KNL_ASSERT_LOCK(knl, knlislocked);
+	if (!knlislocked)
+		knl->kl_lock(knl->kl_lockarg);
+	error = SLIST_EMPTY(&knl->kl_list);
+	if (!knlislocked)
+		knl->kl_unlock(knl->kl_lockarg);
+
+	return error;
 }
 
 static struct mtx	knlist_lock;
@@ -1931,7 +1939,9 @@ knlist_init(struct knlist *knl, void *lo
 	else
 		knl->kl_assert_unlocked = kl_assert_unlocked;
 
+	knl->kl_lock(knl->kl_lockarg);
 	SLIST_INIT(&knl->kl_list);
+	knl->kl_unlock(knl->kl_lockarg);
 }
 
 void
Index: kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.358
diff -u -p -r1.358 uipc_socket.c
--- kern/uipc_socket.c	25 Aug 2011 15:51:54 -0000	1.358
+++ kern/uipc_socket.c	26 Jan 2012 20:50:31 -0000
@@ -3188,7 +3188,7 @@ filt_sordetach(struct knote *kn)
 
 	SOCKBUF_LOCK(&so->so_rcv);
 	knlist_remove(&so->so_rcv.sb_sel.si_note, kn, 1);
-	if (knlist_empty(&so->so_rcv.sb_sel.si_note))
+	if (knlist_empty(&so->so_rcv.sb_sel.si_note, 1))
 		so->so_rcv.sb_flags &= ~SB_KNOTE;
 	SOCKBUF_UNLOCK(&so->so_rcv);
 }
@@ -3222,7 +3222,7 @@ filt_sowdetach(struct knote *kn)
 
 	SOCKBUF_LOCK(&so->so_snd);
 	knlist_remove(&so->so_snd.sb_sel.si_note, kn, 1);
-	if (knlist_empty(&so->so_snd.sb_sel.si_note))
+	if (knlist_empty(&so->so_snd.sb_sel.si_note, 1))
 		so->so_snd.sb_flags &= ~SB_KNOTE;
 	SOCKBUF_UNLOCK(&so->so_snd);
 }
Index: kern/vfs_aio.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_aio.c,v
retrieving revision 1.249
diff -u -p -r1.249 vfs_aio.c
--- kern/vfs_aio.c	16 Sep 2011 13:58:51 -0000	1.249
+++ kern/vfs_aio.c	26 Jan 2012 20:50:31 -0000
@@ -2531,7 +2533,7 @@ filt_aiodetach(struct knote *kn)
 {
 	struct aiocblist *aiocbe = kn->kn_ptr.p_aio;
 
-	if (!knlist_empty(&aiocbe->klist))
+	if (!knlist_empty(&aiocbe->klist, 0))
 		knlist_remove(&aiocbe->klist, kn, 0);
 }
 
@@ -2576,7 +2578,7 @@ filt_liodetach(struct knote *kn)
 {
 	struct aioliojob * lj = kn->kn_ptr.p_lio;
 
-	if (!knlist_empty(&lj->klist))
+	if (!knlist_empty(&lj->klist, 0))
 		knlist_remove(&lj->klist, kn, 0);
 }
 
Index: sys/event.h
===================================================================
RCS file: /cvs/src/sys/sys/event.h,v
retrieving revision 1.49
diff -u -p -r1.49 event.h
--- sys/event.h	31 Dec 2009 20:29:58 -0000	1.49
+++ sys/event.h	26 Jan 2012 20:50:37 -0000
@@ -244,7 +244,7 @@ extern void	knote_fork(struct knlist *li
 extern void	knlist_add(struct knlist *knl, struct knote *kn, int islocked);
 extern void	knlist_remove(struct knlist *knl, struct knote *kn, int islocked);
 extern void	knlist_remove_inevent(struct knlist *knl, struct knote *kn);
-extern int	knlist_empty(struct knlist *knl);
+extern int	knlist_empty(struct knlist *knl, int islocked);
 extern void	knlist_init(struct knlist *knl, void *lock,
     void (*kl_lock)(void *), void (*kl_unlock)(void *),
     void (*kl_assert_locked)(void *), void (*kl_assert_unlocked)(void *));

Thanks,

Doug A.


More information about the freebsd-current mailing list