From owner-freebsd-current@FreeBSD.ORG Thu Jan 26 21:32:03 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6DF6C106566C for ; Thu, 26 Jan 2012 21:32:03 +0000 (UTC) (envelope-from ambrisko@ambrisko.com) Received: from mail.ambrisko.com (mail.ambrisko.com [70.91.206.90]) by mx1.freebsd.org (Postfix) with ESMTP id 4EF9D8FC12 for ; Thu, 26 Jan 2012 21:32:03 +0000 (UTC) X-Ambrisko-Me: Yes Received: from server2.ambrisko.com (HELO internal.ambrisko.com) ([192.168.1.2]) by ironport.ambrisko.com with ESMTP; 26 Jan 2012 13:03:26 -0800 Received: from ambrisko.com (localhost [127.0.0.1]) by internal.ambrisko.com (8.14.4/8.14.4) with ESMTP id q0QL3Qv8083497 for ; Thu, 26 Jan 2012 13:03:26 -0800 (PST) (envelope-from ambrisko@ambrisko.com) Received: (from ambrisko@localhost) by ambrisko.com (8.14.4/8.14.4/Submit) id q0QL3QWr083496 for freebsd-current@freebsd.org; Thu, 26 Jan 2012 13:03:26 -0800 (PST) (envelope-from ambrisko) From: Doug Ambrisko Message-Id: <201201262103.q0QL3QWr083496@ambrisko.com> To: freebsd-current@freebsd.org Date: Thu, 26 Jan 2012 13:03:26 -0800 (PST) X-Mailer: ELM [version 2.4ME+ PL124d (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="US-ASCII" Subject: knlist_empty locking fix X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jan 2012 21:32:03 -0000 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.