[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.


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

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$/./


+     */
+    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



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