From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Rostedt Subject: [PATCH] wrong accounting in direct_remap_pfn_range Date: Tue, 29 Aug 2006 12:25:34 -0400 Message-ID: <44F46A7E.9010906@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------080401010008080503000809" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: quintela@redhat.com List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------080401010008080503000809 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Looking into the code in linux-2.6-xen-sparse/arch/i386/mm/ioremap-xen.c I found some logic that did not make sense. We have a loop than updates the page tables in __direct_remap_pfn_range, and in the beginning of that loop, there is a test that if we finished the page (v-u == PAGE_SIZE/sizeof(mmu_update_t)) we call the hypervisor to do our update. This is all fine and dandy but, but the code right after the loop seems to be wrong. There is a check if (v != u) then do some more work. I'm assuming that this code is there in case we didn't reach the if statement at the top of the loop. But what is wrong is that this check is invalid. Although the loop if statement sets v = u, the following if statement ignores the fact that v++ is done at the bottom of the loop. So if we really want to do this extra work if we didn't finish the loop, then the test really needs to be. if ((v - u) != 1) { .... } Unless I'm missing something here, I've attached a patch. -- Steve Signed-off-by: Steven Rostedt --------------080401010008080503000809 Content-Type: text/x-patch; name="ioredirect.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="ioredirect.patch" 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 @@ -91,7 +91,13 @@ static int __direct_remap_pfn_range(stru v++; } - if (v != u) { + /* + * If we didn't finish the page in the previous loop then we + * need to process it now. We take into account the v++ + * at the end of the loop, so the test to know if we finished + * or not is really a +1 difference and not an equal. + */ + if ((v - u) != 1) { /* get the ptep's filled in */ rc = apply_to_page_range(mm, start_address, address - start_address, --------------080401010008080503000809 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------080401010008080503000809--