Date: Mon, 23 Jan 2006 13:18:52 -0500 From: John Baldwin <jhb@freebsd.org> To: David Xu <davidxu@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/kern syscalls.master vfs_aio.c Message-ID: <200601231318.54234.jhb@freebsd.org> In-Reply-To: <200601220559.k0M5xS5j026930@repoman.freebsd.org> References: <200601220559.k0M5xS5j026930@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_O4R1DmZf6RNy8LX Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Sunday 22 January 2006 00:59, David Xu wrote: > davidxu 2006-01-22 05:59:28 UTC > > FreeBSD src repository > > Modified files: > sys/kern syscalls.master vfs_aio.c > Log: > Make aio code MP safe. Thanks for doing this. This is something I had sort of been working on but hadn't gotten too in a while. (I don't think I had many uncommitted diffs, I think mostly I had just done the fget() and TAILQ_FOREACH_SAFE() cleanups I committed earlier.) One thing I would appreciate is if you could add some comments explaining the locking strategy and annotating which fields are protected by which locks similar to sys/proc.h, etc. I've attached my current set of diffs in case you wanted to look at them. -- John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org --Boundary-00=_O4R1DmZf6RNy8LX Content-Type: text/x-diff; charset="iso-8859-1"; name="aio.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="aio.patch" --- //depot/projects/smpng/sys/kern/vfs_aio.c 2006/01/17 17:16:05 +++ //depot/user/jhb/proc/kern/vfs_aio.c 2006/01/17 17:49:55 @@ -108,6 +108,8 @@ #define AIOD_LIFETIME_DEFAULT (30 * hz) #endif +static int aiod_creation_in_progress; + static SYSCTL_NODE(_vfs, OID_AUTO, aio, CTLFLAG_RW, 0, "Async IO management"); static int max_aio_procs = MAX_AIO_PROCS; @@ -266,7 +268,7 @@ static void aio_onceonly(void); static int aio_free_entry(struct aiocblist *aiocbe); static void aio_process(struct aiocblist *aiocbe); -static int aio_newproc(void); +static int aio_newproc(int always); static int aio_aqueue(struct thread *td, struct aiocb *job, int type, int osigev); static void aio_physwakeup(struct buf *bp); @@ -411,9 +413,17 @@ error = kqueue_del_filteropts(EVFILT_AIO); if (error) return error; + error = kqueue_del_filteropts(EVFILT_LIO); + if (error) + return error; async_io_version = 0; aio_swake = NULL; + uma_zdestroy(kaio_zone); + uma_zdestroy(aiop_zone); + uma_zdestroy(aiocb_zone); + uma_zdestroy(aiol_zone); + uma_zdestroy(aiolio_zone); EVENTHANDLER_DEREGISTER(process_exit, exit_tag); EVENTHANDLER_DEREGISTER(process_exec, exec_tag); mtx_destroy(&aio_freeproc_mtx); @@ -456,8 +466,10 @@ uma_zfree(kaio_zone, ki); } + mtx_lock(&aio_freeproc_mtx); while (num_aio_procs < target_aio_procs) - aio_newproc(); + aio_newproc(0); + mtx_unlock(&aio_freeproc_mtx); } static int @@ -901,8 +913,6 @@ struct proc *curcp, *mycp, *userp; struct vmspace *myvm, *tmpvm; struct thread *td = curthread; - struct pgrp *newpgrp; - struct session *newsess; /* * Local copies of curproc (cp) and vmspace (myvm) @@ -918,16 +928,8 @@ */ aiop = uma_zalloc(aiop_zone, M_WAITOK); aiop->aiothread = td; - aiop->aiothreadflags |= AIOP_FREE; /* - * Place thread (lightweight process) onto the AIO free thread list. - */ - mtx_lock(&aio_freeproc_mtx); - TAILQ_INSERT_HEAD(&aio_freeproc, aiop, list); - mtx_unlock(&aio_freeproc_mtx); - - /* * Get rid of our current filedescriptors. AIOD's don't need any * filedescriptors, except as temporarily inherited from the client. */ @@ -936,15 +938,16 @@ mtx_unlock(&Giant); /* The daemon resides in its own pgrp. */ - MALLOC(newpgrp, struct pgrp *, sizeof(struct pgrp), M_PGRP, - M_WAITOK | M_ZERO); - MALLOC(newsess, struct session *, sizeof(struct session), M_SESSION, - M_WAITOK | M_ZERO); + setsid(td, NULL); - sx_xlock(&proctree_lock); - enterpgrp(mycp, mycp->p_pid, newpgrp, newsess); - sx_xunlock(&proctree_lock); - mtx_lock(&Giant); + /* + * Place thread (lightweight process) onto the AIO free thread list. + */ + mtx_lock(&aio_freeproc_mtx); + num_aio_procs++; + aiop->aiothreadflags |= AIOP_FREE; + TAILQ_INSERT_HEAD(&aio_freeproc, aiop, list); + mtx_unlock(&aio_freeproc_mtx); /* * Wakeup parent process. (Parent sleeps to keep from blasting away @@ -952,6 +955,7 @@ */ wakeup(mycp); + mtx_lock(&aio_freeproc_mtx); for (;;) { /* * curcp is the current daemon process context. @@ -962,7 +966,6 @@ /* * Take daemon off of free queue */ - mtx_lock(&aio_freeproc_mtx); if (aiop->aiothreadflags & AIOP_FREE) { TAILQ_REMOVE(&aio_freeproc, aiop, list); aiop->aiothreadflags &= ~AIOP_FREE; @@ -972,6 +975,7 @@ /* * Check for jobs. */ + mtx_lock(&Giant); while ((aiocbe = aio_selectjob(aiop)) != NULL) { cb = &aiocbe->uaiocb; userp = aiocbe->userproc; @@ -1054,6 +1058,7 @@ curcp = mycp; } + mtx_unlock(&Giant); mtx_lock(&aio_freeproc_mtx); TAILQ_INSERT_HEAD(&aio_freeproc, aiop, list); @@ -1063,18 +1068,17 @@ * If daemon is inactive for a long time, allow it to exit, * thereby freeing resources. */ - if (msleep(aiop->aiothread, &aio_freeproc_mtx, PDROP | PRIBIO, + if (msleep(aiop->aiothread, &aio_freeproc_mtx, PRIBIO, "aiordy", aiod_lifetime)) { s = splnet(); if (TAILQ_EMPTY(&aio_jobs)) { - mtx_lock(&aio_freeproc_mtx); if ((aiop->aiothreadflags & AIOP_FREE) && (num_aio_procs > target_aio_procs)) { TAILQ_REMOVE(&aio_freeproc, aiop, list); + num_aio_procs--; mtx_unlock(&aio_freeproc_mtx); splx(s); uma_zfree(aiop_zone, aiop); - num_aio_procs--; #ifdef DIAGNOSTIC if (mycp->p_vmspace->vm_refcnt <= 1) { printf("AIOD: bad vm refcnt for" @@ -1084,7 +1088,6 @@ #endif kthread_exit(0); } - mtx_unlock(&aio_freeproc_mtx); } splx(s); } @@ -1096,24 +1099,36 @@ * AIO daemon modifies its environment itself. */ static int -aio_newproc(void) +aio_newproc(int always) { - int error; struct proc *p; + int error, n; + mtx_assert(&aio_freeproc_mtx, MA_OWNED); + n = num_aio_procs; + while (aiod_creation_in_progress) { + error = msleep(&aiod_creation_in_progress, &aio_freeproc_mtx, + PZERO, "aionew", aiod_timeout); + if (error || !always) + return (error); + } + aiod_creation_in_progress = 1; + mtx_unlock(&aio_freeproc_mtx); + error = kthread_create(aio_daemon, curproc, &p, RFNOWAIT, 0, "aiod%d", - num_aio_procs); - if (error) - return (error); + n); - /* - * Wait until daemon is started, but continue on just in case to - * handle error conditions. - */ - error = tsleep(p, PZERO, "aiosta", aiod_timeout); + mtx_lock(&aio_freeproc_mtx); + if (error == 0) + /* + * Wait until daemon is started, but continue on just in + * case to handle error conditions. + */ + error = msleep(p, &aio_freeproc_mtx, PZERO, "aiosta", + aiod_timeout); - num_aio_procs++; - + aiod_creation_in_progress = 0; + wakeup(&aiod_creation_in_progress); return (error); } @@ -1597,9 +1612,7 @@ ((ki->kaio_active_count + num_aio_resv_start) < ki->kaio_maxactive_count)) { num_aio_resv_start++; - mtx_unlock(&aio_freeproc_mtx); - error = aio_newproc(); - mtx_lock(&aio_freeproc_mtx); + error = aio_newproc(1); num_aio_resv_start--; if (error) goto retryproc; --Boundary-00=_O4R1DmZf6RNy8LX--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200601231318.54234.jhb>