Xen 
 
Home About Xen.org Xen Xen Summit Wiki Mailing List Bug Tracker Xen Downloads
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] wrong accounting in direct_remap_pfn_range

To: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] wrong accounting in direct_remap_pfn_range
From: Steven Rostedt <srostedt@xxxxxxxxxx>
Date: Wed, 30 Aug 2006 21:03:07 -0400
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, quintela@xxxxxxxxxx
Delivery-date: Wed, 30 Aug 2006 18:02:43 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <C11BEF00.1935%Keir.Fraser@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <C11BEF00.1935%Keir.Fraser@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5.0.4 (X11/20060614)
Keir Fraser wrote:
On 31/8/06 1:37 am, "Steven Rostedt" <srostedt@xxxxxxxxxx> wrote:

grr, I take it back, I am the one that's confused :P

OK, this all happens because this whole blob of code is crazy because it
is missing a "if (size == 0)" check!

It's not really missing. We could have a size==0 check *or* we can have the
v!=u check. We don't need both and I think the latter is more obviously
correct, as the test is closer to the code that it 'protects'. Also it's a
fairly idiomatic way of generating and flushing batches of work.


So what is really wrong with this code? Or is the flushes need even on size == 0?

-- Steve

Signed-off-by: Steven Rostedt <srostedt@xxxxxxxxxx>


Index: linux-2.6-xen-sparse/arch/i386/mm/ioremap-xen.c
===================================================================
--- linux-2.6-xen-sparse.orig/arch/i386/mm/ioremap-xen.c
+++ linux-2.6-xen-sparse/arch/i386/mm/ioremap-xen.c
@@ -56,6 +56,9 @@ static int __direct_remap_pfn_range(stru
        unsigned long i, start_address;
        mmu_update_t *u, *v, *w;
 
+       if (unlikely(!size))
+               return 0;
+
        u = v = w = (mmu_update_t *)__get_free_page(GFP_KERNEL|__GFP_REPEAT);
        if (u == NULL)
                return -ENOMEM;
@@ -91,17 +94,15 @@ static int __direct_remap_pfn_range(stru
                v++;
        }
 
-       if (v != u) {
-               /* get the ptep's filled in */
-               rc = apply_to_page_range(mm, start_address,
-                                        address - start_address,
-                                        direct_remap_area_pte_fn, &w);
-               if (rc)
-                       goto out;
-               rc = -EFAULT;
-               if (unlikely(HYPERVISOR_mmu_update(u, v - u, NULL, domid) < 0))
-                       goto out;
-       }
+       /* get the ptep's filled in */
+       rc = apply_to_page_range(mm, start_address,
+                                address - start_address,
+                                direct_remap_area_pte_fn, &w);
+       if (rc)
+               goto out;
+       rc = -EFAULT;
+       if (unlikely(HYPERVISOR_mmu_update(u, v - u, NULL, domid) < 0))
+               goto out;
 
        rc = 0;
 
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel