From owner-freebsd-current@FreeBSD.ORG Fri Jan 27 17:42:57 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 C72DB1065670; Fri, 27 Jan 2012 17:42:57 +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 9D7F78FC1D; Fri, 27 Jan 2012 17:42:57 +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; 27 Jan 2012 09:43:00 -0800 Received: from ambrisko.com (localhost [127.0.0.1]) by internal.ambrisko.com (8.14.4/8.14.4) with ESMTP id q0RHgxEt091010; Fri, 27 Jan 2012 09:42:59 -0800 (PST) (envelope-from ambrisko@ambrisko.com) Received: (from ambrisko@localhost) by ambrisko.com (8.14.4/8.14.4/Submit) id q0RHgwGS091009; Fri, 27 Jan 2012 09:42:58 -0800 (PST) (envelope-from ambrisko) From: Doug Ambrisko Message-Id: <201201271742.q0RHgwGS091009@ambrisko.com> In-Reply-To: <5EDB646F-513F-411A-ABBC-6DA555D2EAC6@averesystems.com> To: Andrew Boyer Date: Fri, 27 Jan 2012 09:42:58 -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" Cc: Kostik Belousov , freebsd-current@freebsd.org Subject: Re: 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: Fri, 27 Jan 2012 17:42:57 -0000 Andrew Boyer writes: | On Jan 27, 2012, at 12:15 PM, Doug Ambrisko wrote: | | > John Baldwin writes: | > | Agreed, I think the missing locking should just be added to the aio code. | > | > Okay so then just: | > | > Index: vfs_aio.c | > =================================================================== | > RCS file: /usr/local/cvsroot/freebsd/src/sys/kern/vfs_aio.c,v | > retrieving revision 1.243.2.3.4.1 | > diff -u -p -r1.243.2.3.4.1 vfs_aio.c | > --- vfs_aio.c 21 Dec 2010 17:09:25 -0000 1.243.2.3.4.1 | > +++ vfs_aio.c 27 Jan 2012 17:07:11 -0000 | > @@ -2509,9 +2509,12 @@ static void | > filt_aiodetach(struct knote *kn) | > { | > struct aiocblist *aiocbe = kn->kn_ptr.p_aio; | > + struct knlist *knl = &aiocbe->klist; | > | > - if (!knlist_empty(&aiocbe->klist)) | > - knlist_remove(&aiocbe->klist, kn, 0); | > + knl->kl_lock(knl->kl_lockarg); | > + if (!knlist_empty(knl)) | > + knlist_remove(knl, kn, 1); | > + knl->kl_unlock(knl->kl_lockarg); | > } | > | > /* kqueue filter function */ | > | > I was trying to be consistant with knlist_remove but this is a much | > smaller change that can be merge to older branches. | | Does filt_liodetach() need the same treatment? Yes, I had that in the original. I updated that and optimized the knl to just get the structure needed. Index: vfs_aio.c =================================================================== RCS file: /usr/local/cvsroot/freebsd/src/sys/kern/vfs_aio.c,v retrieving revision 1.243.2.3.4.1 diff -u -p -r1.243.2.3.4.1 vfs_aio.c --- vfs_aio.c 21 Dec 2010 17:09:25 -0000 1.243.2.3.4.1 +++ vfs_aio.c 27 Jan 2012 17:35:47 -0000 @@ -2508,10 +2508,12 @@ filt_aioattach(struct knote *kn) static void filt_aiodetach(struct knote *kn) { - struct aiocblist *aiocbe = kn->kn_ptr.p_aio; + struct knlist *knl = &kn->kn_ptr.p_aio->klist; - if (!knlist_empty(&aiocbe->klist)) - knlist_remove(&aiocbe->klist, kn, 0); + knl->kl_lock(knl->kl_lockarg); + if (!knlist_empty(knl)) + knlist_remove(knl, kn, 1); + knl->kl_unlock(knl->kl_lockarg); } /* kqueue filter function */ @@ -2553,10 +2555,12 @@ filt_lioattach(struct knote *kn) static void filt_liodetach(struct knote *kn) { - struct aioliojob * lj = kn->kn_ptr.p_lio; + struct knlist *knl = &kn->kn_ptr.p_lio->klist; - if (!knlist_empty(&lj->klist)) - knlist_remove(&lj->klist, kn, 0); + knl->kl_lock(knl->kl_lockarg); + if (!knlist_empty(knl)) + knlist_remove(knl, kn, 1); + knl->kl_unlock(knl->kl_lockarg); } /* kqueue filter function */ Thanks, Doug A.