Re: RFC: [0/2] Remove netloop by lazy copying in netback

[Herbert Xu <herbert@gondor.apana.org.au>]

On Wed, Mar 28, 2007 at 06:46:40AM +1000, Herbert Xu wrote:
> 
> I've checked again and the accessed bit is certainly present on ppc
> as well as ia64.  The only I don't know is if it's present on the
> nested page tables on ia64 but I see no reasons why it wouldn't be.

Here is a completely untested patch that demonstrates how this could
be done on x86.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff -r 3ac19fda0bc2 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c	Fri Mar 02 12:11:52 2007 +0000
+++ b/xen/arch/x86/mm.c	Wed Mar 28 22:58:45 2007 +1000
@@ -1262,6 +1262,36 @@ static void free_l4_table(struct page_in
 #endif
 
 
+static inline int cmpxchg_intpte(intpte_t *p,
+				 intpte_t *old,
+				 intpte_t new,
+				 unsigned long mfn,
+				 struct vcpu *v)
+{
+        int rv;
+
+        for ( ; ; )
+        {
+            intpte_t t = *old;
+
+            rv = paging_cmpxchg_guest_entry(v, p, old, new, _mfn(mfn));
+            if ( unlikely(rv == 0) )
+            {
+                MEM_LOG("Failed to update %" PRIpte " -> %" PRIpte
+                        ": saw %" PRIpte, t, new, t);
+                break;
+            }
+
+            if ( t == *old )
+                break;
+
+            /* Allowed to change in Accessed/Dirty flags only. */
+            BUG_ON((t ^ *old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
+        }
+
+        return rv;
+}
+
 /* How to write an entry to the guest pagetables.
  * Returns 0 for failure (pointer not valid), 1 for success. */
 static inline int update_intpte(intpte_t *p, 
@@ -1274,27 +1304,7 @@ static inline int update_intpte(intpte_t
 #ifndef PTE_UPDATE_WITH_CMPXCHG
     rv = paging_write_guest_entry(v, p, new, _mfn(mfn));
 #else
-    {
-        intpte_t t = old;
-        for ( ; ; )
-        {
-            rv = paging_cmpxchg_guest_entry(v, p, &t, new, _mfn(mfn));
-            if ( unlikely(rv == 0) )
-            {
-                MEM_LOG("Failed to update %" PRIpte " -> %" PRIpte
-                        ": saw %" PRIpte, old, new, t);
-                break;
-            }
-
-            if ( t == old )
-                break;
-
-            /* Allowed to change in Accessed/Dirty flags only. */
-            BUG_ON((t ^ old) & ~(intpte_t)(_PAGE_ACCESSED|_PAGE_DIRTY));
-
-            old = t;
-        }
-    }
+    rv = cmpxchg_intpte(p, &old, new, mfn, v);
 #endif
     return rv;
 }
@@ -2598,6 +2608,7 @@ static int destroy_grant_va_mapping(
     unsigned long addr, unsigned long frame, struct vcpu *v)
 {
     l1_pgentry_t *pl1e, ol1e;
+    intpte_t *ptep, opte, npte;
     unsigned long gl1mfn;
     int rc = 0;
     
@@ -2619,12 +2630,19 @@ static int destroy_grant_va_mapping(
     }
 
     /* Delete pagetable entry. */
-    if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, l1e_empty(), gl1mfn, v)) )
+    ptep = (intpte_t *)pl1e;
+    opte = l1e_get_intpte(ol1e);
+    npte = l1e_get_intpte(l1e_empty());
+
+    if ( unlikely(!cmpxchg_intpte(ptep, opte, npte, mfn, v)) )
     {
         MEM_LOG("Cannot delete PTE entry at %p", (unsigned long *)pl1e);
         rc = GNTST_general_error;
         goto out;
     }
+
+    if (!(opte & (intpte_t)_PAGE_ACCESSED))
+        rc = GNTST_unused;
 
  out:
     guest_unmap_l1e(v, pl1e);
diff -r 3ac19fda0bc2 xen/common/grant_table.c
--- a/xen/common/grant_table.c	Fri Mar 02 12:11:52 2007 +0000
+++ b/xen/common/grant_table.c	Wed Mar 28 22:58:45 2007 +1000
@@ -394,7 +394,7 @@ gnttab_map_grant_ref(
     return 0;
 }
 
-static void
+static int
 __gnttab_unmap_grant_ref(
     struct gnttab_unmap_grant_ref *op)
 {
@@ -416,7 +416,7 @@ __gnttab_unmap_grant_ref(
     {
         gdprintk(XENLOG_INFO, "Bad handle (%d).\n", op->handle);
         op->status = GNTST_bad_handle;
-        return;
+        return 0;
     }
 
     map = &maptrack_entry(ld->grant_table, op->handle);
@@ -425,7 +425,7 @@ __gnttab_unmap_grant_ref(
     {
         gdprintk(XENLOG_INFO, "Zero flags for handle (%d).\n", op->handle);
         op->status = GNTST_bad_handle;
-        return;
+        return 0;
     }
 
     dom   = map->domid;
@@ -437,7 +437,7 @@ __gnttab_unmap_grant_ref(
         /* This can happen when a grant is implicitly unmapped. */
         gdprintk(XENLOG_INFO, "Could not find domain %d\n", dom);
         domain_crash(ld); /* naughty... */
-        return;
+        return 0;
     }
 
     TRACE_1D(TRC_MEM_PAGE_GRANT_UNMAP, dom);
@@ -511,9 +511,17 @@ __gnttab_unmap_grant_ref(
         gnttab_clear_flag(_GTF_reading, &sha->flags);
 
  unmap_out:
-    op->status = rc;
     spin_unlock(&rd->grant_table->lock);
     rcu_unlock_domain(rd);
+
+    op->status = rc;
+
+    if (rc == GNTST_unused) {
+        op->status = GNTST_okay;
+        return 0;
+    }
+
+    return 1;
 }
 
 static long
@@ -521,23 +529,25 @@ gnttab_unmap_grant_ref(
     XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t) uop, unsigned int count)
 {
     int i;
+    int used = 0;
+    int err = -EFAULT;
     struct gnttab_unmap_grant_ref op;
 
     for ( i = 0; i < count; i++ )
     {
         if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
             goto fault;
-        __gnttab_unmap_grant_ref(&op);
+        used |= __gnttab_unmap_grant_ref(&op);
         if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) )
             goto fault;
     }
 
-    flush_tlb_mask(current->domain->domain_dirty_cpumask);
-    return 0;
+    err = 0;
 
 fault:
-    flush_tlb_mask(current->domain->domain_dirty_cpumask);
-    return -EFAULT;    
+    if (used)
+        flush_tlb_mask(current->domain->domain_dirty_cpumask);
+    return err;
 }
 
 int
diff -r 3ac19fda0bc2 xen/include/public/grant_table.h
--- a/xen/include/public/grant_table.h	Fri Mar 02 12:11:52 2007 +0000
+++ b/xen/include/public/grant_table.h	Wed Mar 28 22:58:45 2007 +1000
@@ -360,6 +360,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_query_siz
 /*
  * Values for error status returns. All errors are -ve.
  */
+#define GNTST_unused           (1)  /* Entry is unused (internal status).    */
 #define GNTST_okay             (0)  /* Normal return.                        */
 #define GNTST_general_error    (-1) /* General undefined error.              */
 #define GNTST_bad_domain       (-2) /* Unrecognsed domain id.                */