svn commit: r254150 - head/sys/vm

[Alan Cox]

On Aug 9, 2013, at 1:45 PM, John Baldwin wrote:

> On Friday, August 09, 2013 4:40:10 pm Alan Cox wrote:
>> 
>> On Aug 9, 2013, at 1:34 PM, Alan Cox wrote:
>> 
>>> 
>>> On Aug 9, 2013, at 12:56 PM, John Baldwin wrote:
>>> 
>>>> 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
>>>>> 
>>>>> Log:
>>>>> Add missing 'VPO_BUSY' from r254141 to fix kernel build break.
>>>>> 
>>>>> Modified:
>>>>> head/sys/vm/vm_page.h
>>>> 
>>>> This can't possibly be correct as r254138 just removed this flag.  If it 
> isn't 
>>>> obvious how to fix the uses added back in r254141, then r254141 should be 
>>>> reverted instead.
>>>> 
>>>> Hmm, looking at the relevant bits of r254141, it doesn't look obvious:
>>>> 
>>>> +       /* Detach the old page from the resident tailq. */
>>>> +       TAILQ_REMOVE(&object->memq, mold, listq);
>>>> +       vm_page_lock(mold);
>>> 
>>> Replace the next four lines with
>>> 
>>> 	vm_page_xunbusy(mold);
>>> 
>> 
>> On second thought, no, because it could lead to lock recursion.
> 
> What about this.  I think this matches the common idiom I've seen in
> other places.
> 
> Index: vm_page.c
> ===================================================================
> --- 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 &= ~VPO_BUSY;
> -               vm_page_flash(mold);
> -       }
>        mold->object = NULL;
>        vm_page_unlock(mold);
> +       vm_page_xunbusy(mold);
> 
>        /* Insert the new page in the resident tailq. */
>        if (mpred != NULL)
> 
> 
> -- 

Index: vm/vm_page.c
===================================================================
--- 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 = object;
 	mnew->pindex = pindex;
 	mold = vm_radix_replace(&object->rtree, mnew, pindex);
+	KASSERT(mold->queue == PQ_NONE,
+	    ("vm_page_replace: mold is on a paging queue"));
 
 	/* 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 &= ~VPO_BUSY;
-		vm_page_flash(mold);
-	}
+
 	mold->object = NULL;
-	vm_page_unlock(mold);
+	vm_page_xunbusy(mold);
 
 	/* Insert the new page in the resident tailq. */
 	if (mpred != NULL)