[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] trace: Fix incorrect number of pages used for trace metadata



On 30/09/16 15:46, George Dunlap wrote:
On 29/09/16 14:53, Igor Druzhinin wrote:
> As long as t_info_first_offset is calculated in uint32_t offsets we need to
> multiply it by sizeof(uint32_t) in order to get the right number of pages
> for trace metadata. Not doing that makes it impossible to read the trace
> buffer correctly from userspace for some corner cases.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
Hmm, looks like we actually want to revert this c/s fbf96e6, "xentrace:
correct formula to calculate t_info_pages".  But that one was presumably
written (and Acked by me) because the variable name there,
t_info_first_offset, is confusing.

The other mistake in fbf96e6 is that before t_info_words was actually
denominated in words; but after it's denominated in bytes (which is
again confusing).

What about something like the attached instead?  This should fix your
problem while making the code clearer.

 -George



From b0252cec4a96fea28faa85c47ab0f5d5997444ad Mon Sep 17 00:00:00 2001
From: George Dunlap <george.dunlap@xxxxxxxxxx>
Date: Fri, 30 Sep 2016 15:42:56 +0100
Subject: [PATCH] xen/trace: Fix trace metadata page count calculation (revert
 fbf96e6)

Changeset fbf96e6, "xentrace: correct formula to calculate
t_info_pages", broke the trace metadata page count calculation, by
mistaking t_info_first_offset as denominated in bytes, when in fact it
is denominated in words (uint32_t).

Effectively revert that change, and put a comment there to reduce the
chance that someone will make that mistake in the future.

Spotted-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
CC: Olaf Hering <olaf@xxxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/common/trace.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/common/trace.c b/xen/common/trace.c
index f651cf3..2f4ecca 100644
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -148,8 +148,12 @@ static int calculate_tbuf_size(unsigned int pages, uint16_t t_info_first_offset)
         pages = max_pages;
     }
 
-    t_info_words = num_online_cpus() * pages * sizeof(uint32_t);
-    t_info_pages = PFN_UP(t_info_first_offset + t_info_words);
+    /* 
+     * NB this calculation is correct, because t_info_first_offset is
+     * in words, not bytes, not bytes

s/, not bytes$/./

~Andrew

+     */
+    t_info_words = num_online_cpus() * pages + t_info_first_offset;
+    t_info_pages = PFN_UP(t_info_words * sizeof(uint32_t));
     printk(XENLOG_INFO "xentrace: requesting %u t_info pages "
            "for %u trace pages on %u cpus\n",
            t_info_pages, pages, num_online_cpus());
-- 2.1.4

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.