Date: Sat, 01 May 2010 10:41:44 -0500 From: Alan Cox <alc@cs.rice.edu> To: Kip Macy <kmacy@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r207450 - head/sys/vm Message-ID: <4BDC4BB8.3000307@cs.rice.edu> In-Reply-To: <201004302120.o3ULKEar083168@svn.freebsd.org> References: <201004302120.o3ULKEar083168@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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) >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4BDC4BB8.3000307>