Re: knlist_empty locking fix

From: Doug Ambrisko <ambrisko_at_ambrisko.com>
Date: Fri, 27 Jan 2012 09:42:58 -0800 (PST)
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
| > _at__at_ -2509,9 +2509,12 _at__at_ 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
_at__at_ -2508,10 +2508,12 _at__at_ 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 */
_at__at_ -2553,10 +2555,12 _at__at_ 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.
Received on Fri Jan 27 2012 - 16:42:57 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:23 UTC