From owner-svn-src-all@FreeBSD.ORG Sat May 1 15:41:54 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7A00C1065674; Sat, 1 May 2010 15:41:54 +0000 (UTC) (envelope-from alc@cs.rice.edu) Received: from mail.cs.rice.edu (mail.cs.rice.edu [128.42.1.31]) by mx1.freebsd.org (Postfix) with ESMTP id 3330E8FC0A; Sat, 1 May 2010 15:41:53 +0000 (UTC) Received: from mail.cs.rice.edu (localhost.localdomain [127.0.0.1]) by mail.cs.rice.edu (Postfix) with ESMTP id 5A18B2C2ADC; Sat, 1 May 2010 10:41:53 -0500 (CDT) X-Virus-Scanned: by amavis-2.4.0 at mail.cs.rice.edu Received: from mail.cs.rice.edu ([127.0.0.1]) by mail.cs.rice.edu (mail.cs.rice.edu [127.0.0.1]) (amavisd-new, port 10024) with LMTP id azEeEpg8B7CF; Sat, 1 May 2010 10:41:45 -0500 (CDT) Received: from adsl-216-63-78-18.dsl.hstntx.swbell.net (adsl-216-63-78-18.dsl.hstntx.swbell.net [216.63.78.18]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.cs.rice.edu (Postfix) with ESMTP id 478362C2AD0; Sat, 1 May 2010 10:41:45 -0500 (CDT) Message-ID: <4BDC4BB8.3000307@cs.rice.edu> Date: Sat, 01 May 2010 10:41:44 -0500 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100327) MIME-Version: 1.0 To: Kip Macy References: <201004302120.o3ULKEar083168@svn.freebsd.org> In-Reply-To: <201004302120.o3ULKEar083168@svn.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r207450 - head/sys/vm X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 May 2010 15:41:54 -0000 Kip Macy wrote: > Author: kmacy > Date: Fri Apr 30 21:20:14 2010 > New Revision: 207450 > URL: http://svn.freebsd.org/changeset/base/207450 > > Log: > - acquire the page lock in vm_contig_launder_page before checking page fields > - release page queue lock before calling vm_pageout_flush > > This change introduces a race condition. Between dropping the page queues lock in vm_contig_launder() and reacquiring it in vm_contig_launder_page(), the page may be removed from the inactive queue. It could be wired, freed, cached, etc. None of which vm_contig_launder_page() is prepared for. You need to call vm_contig_launder_page with the page queues lock held, like before, and deal with the lock order reversal just like the page daemon does in vm_pageout_scan(). In fact, notice that the lock order reversal with the object lock uses a function from vm_pageout.c. Regards, Alan > Modified: > head/sys/vm/vm_contig.c > > Modified: head/sys/vm/vm_contig.c > ============================================================================== > --- head/sys/vm/vm_contig.c Fri Apr 30 19:52:35 2010 (r207449) > +++ head/sys/vm/vm_contig.c Fri Apr 30 21:20:14 2010 (r207450) > @@ -96,30 +96,33 @@ vm_contig_launder_page(vm_page_t m, vm_p > vm_page_t m_tmp; > struct vnode *vp; > struct mount *mp; > - int vfslocked; > + int vfslocked, dirty; > > - mtx_assert(&vm_page_queue_mtx, MA_OWNED); > + vm_page_lock(m); > + vm_page_lock_queues(); > object = m->object; > if (!VM_OBJECT_TRYLOCK(object) && > !vm_pageout_fallback_object_lock(m, next)) { > VM_OBJECT_UNLOCK(object); > + vm_page_unlock_queues(); > + vm_page_unlock(m); > return (EAGAIN); > } > if (vm_page_sleep_if_busy(m, TRUE, "vpctw0")) { > VM_OBJECT_UNLOCK(object); > - vm_page_lock_queues(); > return (EBUSY); > } > vm_page_test_dirty(m); > if (m->dirty == 0 && m->hold_count == 0) > pmap_remove_all(m); > - if (m->dirty) { > + if ((dirty = m->dirty) != 0) { > + vm_page_unlock_queues(); > + vm_page_unlock(m); > if ((object->flags & OBJ_DEAD) != 0) { > VM_OBJECT_UNLOCK(object); > return (EAGAIN); > } > if (object->type == OBJT_VNODE) { > - vm_page_unlock_queues(); > vp = object->handle; > vm_object_reference_locked(object); > VM_OBJECT_UNLOCK(object); > @@ -133,7 +136,6 @@ vm_contig_launder_page(vm_page_t m, vm_p > VFS_UNLOCK_GIANT(vfslocked); > vm_object_deallocate(object); > vn_finished_write(mp); > - vm_page_lock_queues(); > return (0); > } else if (object->type == OBJT_SWAP || > object->type == OBJT_DEFAULT) { > @@ -144,6 +146,11 @@ vm_contig_launder_page(vm_page_t m, vm_p > } > } else if (m->hold_count == 0) > vm_page_cache(m); > + > + if (dirty == 0) { > + vm_page_unlock_queues(); > + vm_page_unlock(m); > + } > VM_OBJECT_UNLOCK(object); > return (0); > } > @@ -162,7 +169,9 @@ vm_contig_launder(int queue) > > KASSERT(VM_PAGE_INQUEUE2(m, queue), > ("vm_contig_launder: page %p's queue is not %d", m, queue)); > + vm_page_unlock_queues(); > error = vm_contig_launder_page(m, &next); > + vm_page_lock_queues(); > if (error == 0) > return (TRUE); > if (error == EBUSY) >