[Xen-devel] [PATCH 3/4] mm: Use put_old_guest_table for relinquish_pages

From: George Dunlap <george.dunlap@citrix.com>
To: <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>
Subject: [Xen-devel] [PATCH 3/4] mm: Use put_old_guest_table for relinquish_pages
Date: Mon, 23 Dec 2019 16:43:28 +0000	[thread overview]
Message-ID: <20191223164329.3113378-4-george.dunlap@citrix.com> (raw)
In-Reply-To: <20191223164329.3113378-1-george.dunlap@citrix.com>

relinquish_pages() deals with interrupted de-validation in a fairly
ad-hoc way, by either re-setting PGT_pinned (in the case of EINTR) or
letting the page "fall through" to the "force invalidate" loop below.
This requires an extensive comment describing what needs to happen to
the type and count in each case, and why each works.  Additionally, it
turns out that at this point, the "force invalidate" loop is only
required to handle this ad-hoc continuation.

Replace this with the 'standard' way of dealing with restarting pages,
old_guest_table.  Call put_old_guest_table(current) at the top of the
function, and set current->arch.old_guest_table* as appropriate.  This
code is simpler, and mirrors other old_guest_table code in mm.c.  It
will also allow us to remove the force-invalidate loop entirely in a
subsequent patch.

While here, make the refcounting logic a bit easier to follow: We
always drop the general reference held by PGT_pinned, regardless of
what happens to the type count.  Rather than manually re-dropping the
refcount if put_page_and_type_preemptible() fails, just drop the
refcount unconditionally, and call put_page_type_preemptible()
instead.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/domain.c | 50 +++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index d9c63379cd..b7968463cb 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1953,6 +1953,10 @@ static int relinquish_memory(
     unsigned long     x, y;
     int               ret = 0;
 
+    ret = put_old_guest_table(current);
+    if ( ret )
+        return ret;
+
     /* Use a recursive lock, as we may enter 'free_domheap_page'. */
     spin_lock_recursive(&d->page_alloc_lock);
 
@@ -1967,42 +1971,32 @@ static int relinquish_memory(
         }
 
         if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
-            ret = put_page_and_type_preemptible(page);
+        {
+            /* Always drop the page ref associated with PGT_pinned */
+            put_page(page);
+            ret = put_page_type_preemptible(page);
+        }
         switch ( ret )
         {
         case 0:
             break;
-        case -ERESTART:
         case -EINTR:
-            /*
-             * -EINTR means PGT_validated has been re-set; re-set
-             * PGT_pinned again so that it gets picked up next time
-             * around.
-             *
-             * -ERESTART, OTOH, means PGT_partial is set instead.  Put
-             * it back on the list, but don't set PGT_pinned; the
-             * section below will finish off de-validation.  But we do
-             * need to drop the general ref associated with
-             * PGT_pinned, since put_page_and_type_preemptible()
-             * didn't do it.
-             *
-             * NB we can do an ASSERT for PGT_validated, since we
-             * "own" the type ref; but theoretically, the PGT_partial
-             * could be cleared by someone else.
-             */
-            if ( ret == -EINTR )
-            {
-                ASSERT(page->u.inuse.type_info & PGT_validated);
-                set_bit(_PGT_pinned, &page->u.inuse.type_info);
-            }
-            else
-                put_page(page);
+            ASSERT(page->u.inuse.type_info & PGT_validated);
+            /* Fallthrough */
+        case -ERESTART:
+            current->arch.old_guest_ptpg = NULL;
+            current->arch.old_guest_table = page;
+            current->arch.old_guest_table_partial = (ret == -ERESTART);
 
             ret = -ERESTART;
 
-            /* Put the page back on the list and drop the ref we grabbed above */
-            page_list_add(page, list);
-            put_page(page);
+            /* Make sure we don't lose track of the page */
+            page_list_add_tail(page, &d->arch.relmem_list);
+
+            /*
+             * NB that we've transferred the general ref acquired at
+             * the top of the loop to old_guest_table.
+             */
             goto out;
         default:
             BUG();
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
