From owner-cvs-src@FreeBSD.ORG Mon Jan 23 18:55:56 2006 Return-Path: X-Original-To: cvs-src@freebsd.org Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4E70016A41F; Mon, 23 Jan 2006 18:55:56 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.FreeBSD.org (Postfix) with ESMTP id E5D6943D83; Mon, 23 Jan 2006 18:55:43 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.5b3) with ESMTP id 6780243 for multiple; Mon, 23 Jan 2006 13:56:50 -0500 Received: from localhost (john@localhost [127.0.0.1]) by server.baldwin.cx (8.13.4/8.13.4) with ESMTP id k0NIteXF002570; Mon, 23 Jan 2006 13:55:41 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: David Xu Date: Mon, 23 Jan 2006 13:18:52 -0500 User-Agent: KMail/1.9.1 References: <200601220559.k0M5xS5j026930@repoman.freebsd.org> In-Reply-To: <200601220559.k0M5xS5j026930@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_O4R1DmZf6RNy8LX" Message-Id: <200601231318.54234.jhb@freebsd.org> X-Virus-Scanned: ClamAV 0.87.1/1247/Sat Jan 21 05:24:51 2006 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.4 required=4.2 tests=ALL_TRUSTED autolearn=failed version=3.1.0 X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on server.baldwin.cx X-Server: High Performance Mail Server - http://surgemail.com r=1653887525 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 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Jan 2006 18:55:56 -0000 --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 <>< 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--