Date: Fri, 9 Aug 2013 13:50:53 -0700 From: Alan Cox <alc@rice.edu> To: John Baldwin <jhb@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, "David E. O'Brien" <obrien@freebsd.org> Subject: Re: svn commit: r254150 - head/sys/vm Message-ID: <79BE43B3-382C-418B-8597-A599182228B3@rice.edu> In-Reply-To: <201308091645.26895.jhb@freebsd.org> References: <201308091643.r79GhoWx023884@svn.freebsd.org> <DA97389E-DAB5-4DB6-BB52-417CDBBB0BF9@rice.edu> <655524DF-4DE9-428C-8A6A-C1A395489EF5@rice.edu> <201308091645.26895.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Aug 9, 2013, at 1:45 PM, John Baldwin wrote: > On Friday, August 09, 2013 4:40:10 pm Alan Cox wrote: >>=20 >> On Aug 9, 2013, at 1:34 PM, Alan Cox wrote: >>=20 >>>=20 >>> On Aug 9, 2013, at 12:56 PM, John Baldwin wrote: >>>=20 >>>> On Friday, August 09, 2013 12:43:50 pm David E. O'Brien wrote: >>>>> Author: obrien >>>>> Date: Fri Aug 9 16:43:50 2013 >>>>> New Revision: 254150 >>>>> URL: http://svnweb.freebsd.org/changeset/base/254150 >>>>>=20 >>>>> Log: >>>>> Add missing 'VPO_BUSY' from r254141 to fix kernel build break. >>>>>=20 >>>>> Modified: >>>>> head/sys/vm/vm_page.h >>>>=20 >>>> This can't possibly be correct as r254138 just removed this flag. = If it=20 > isn't=20 >>>> obvious how to fix the uses added back in r254141, then r254141 = should be=20 >>>> reverted instead. >>>>=20 >>>> Hmm, looking at the relevant bits of r254141, it doesn't look = obvious: >>>>=20 >>>> + /* Detach the old page from the resident tailq. */ >>>> + TAILQ_REMOVE(&object->memq, mold, listq); >>>> + vm_page_lock(mold); >>>=20 >>> Replace the next four lines with >>>=20 >>> vm_page_xunbusy(mold); >>>=20 >>=20 >> On second thought, no, because it could lead to lock recursion. >=20 > What about this. I think this matches the common idiom I've seen in > other places. >=20 > Index: vm_page.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- vm_page.c (revision 254158) > +++ vm_page.c (working copy) > @@ -1202,12 +1202,9 @@ > /* Detach the old page from the resident tailq. */ > TAILQ_REMOVE(&object->memq, mold, listq); > vm_page_lock(mold); > - if (mold->oflags & VPO_BUSY) { > - mold->oflags &=3D ~VPO_BUSY; > - vm_page_flash(mold); > - } > mold->object =3D NULL; > vm_page_unlock(mold); > + vm_page_xunbusy(mold); >=20 > /* Insert the new page in the resident tailq. */ > if (mpred !=3D NULL) >=20 >=20 > --=20 Index: vm/vm_page.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- vm/vm_page.c (revision 254146) +++ vm/vm_page.c (working copy) @@ -1174,6 +1174,8 @@ vm_page_prev(vm_page_t m) /* * Uses the page mnew as a replacement for an existing page at index * pindex which must be already present in the object. + * + * The existing page must not be on a paging queue. */ vm_page_t vm_page_replace(vm_page_t mnew, vm_object_t object, vm_pindex_t pindex) @@ -1198,16 +1200,14 @@ vm_page_replace(vm_page_t mnew, vm_object_t = object mnew->object =3D object; mnew->pindex =3D pindex; mold =3D vm_radix_replace(&object->rtree, mnew, pindex); + KASSERT(mold->queue =3D=3D PQ_NONE, + ("vm_page_replace: mold is on a paging queue")); =20 /* Detach the old page from the resident tailq. */ TAILQ_REMOVE(&object->memq, mold, listq); - vm_page_lock(mold); - if (mold->oflags & VPO_BUSY) { - mold->oflags &=3D ~VPO_BUSY; - vm_page_flash(mold); - } + mold->object =3D NULL; - vm_page_unlock(mold); + vm_page_xunbusy(mold); =20 /* Insert the new page in the resident tailq. */ if (mpred !=3D NULL)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?79BE43B3-382C-418B-8597-A599182228B3>