[PATCH] dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.

[PATCH] dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.

[Isaku Yamahata]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]


dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.
So far the order isn't specified and may be random in theory.
But sorted array is requested by crash utility for efficient looking up.
Fortunately it is the case except ia64 full virtualized domain.
Update document such that those array must be sorted and fix the ia64
full virtualized domain case.

-- 
yamahata

[-- Attachment #2: 14288_e93d1eaef5b4_dump_core_pfn_ascending.patch --]
[-- Type: text/x-diff, Size: 4419 bytes --]

# HG changeset patch
# User yamahata@valinux.co.jp
# Date 1173404772 -32400
# Node ID e93d1eaef5b44f59e1a2e0996f9caf61770666b9
# Parent  38513d22d23420a90f94e7e0f70c564100e83851
dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.
So far the order isn't specified and may be random in theory.
But sorted array is requested by crash utility for efficient looking up.
Fortunately it is the case except ia64 full virtualized domain.
Update document such that those array must be sorted and fix the ia64
full virtualized domain case.
PATCHNAME: dump_core_pfn_ascending

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff -r 38513d22d234 -r e93d1eaef5b4 docs/misc/dump-core-format.txt
--- a/docs/misc/dump-core-format.txt	Thu Mar 08 15:21:10 2007 +0000
+++ b/docs/misc/dump-core-format.txt	Fri Mar 09 10:46:12 2007 +0900
@@ -80,8 +80,7 @@ Currently the following sections are def
                         gmfn:   machine physical frame number
                 The size of arrays is stored in xch_nr_pages member of header
                 note descriptor in .note.Xen note section.
-                There is no rule about the order. Analysis tools must no rely
-                on its order.
+                The entries are stored in pfn-ascending order.
                 This section must exist when the domain is non auto
                 translated physmap mode. Currently x86 paravirtualized domain.
 
@@ -94,8 +93,7 @@ Currently the following sections are def
                 in .xen_pages section.
                 The size of arrays is stored in xch_nr_pages member of header
                 note descriptor in .note.Xen note section.
-                There is no rule about the order. Analysis tools must no rely
-                on its order.
+                The entries are stored in ascending order.
                 This section must exist when the domain is auto translated
                 physmap mode. Currently x86 full virtualized domain and
                 ia64 domain.
@@ -226,6 +224,8 @@ Currently only (major, minor) = (0, 1) i
 [When the format is changed, it would be described here.]
 
 (0, 1) update
+- .xen_p2m, .xen_pfn section
+  Arrays must be in pfn ascending order for efficient looking up.
 - EI_CLASS member of elf header was changed to ELFCLASS64 independent of
   architecture. This is mainly for x86_32pae.
   The format version isn't bumped because analysis tools can distinguish it.
diff -r 38513d22d234 -r e93d1eaef5b4 tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c	Thu Mar 08 15:21:10 2007 +0000
+++ b/tools/libxc/xc_core.c	Fri Mar 09 10:46:12 2007 +0900
@@ -54,6 +54,7 @@
 #include "xc_dom.h"
 #include <stdlib.h>
 #include <unistd.h>
+#include <inttypes.h>
 
 /* number of pages to write at a time */
 #define DUMP_INCREMENT (4 * 1024)
@@ -282,6 +283,27 @@ elfnote_fill_format_version(struct xen_d
                             *format_version)
 {
     format_version->version = XEN_DUMPCORE_FORMAT_VERSION_CURRENT;
+    return 0;
+}
+
+static int
+xc_memory_map_cmp(const void *lhs__, const void *rhs__)
+{
+    const struct xc_core_memory_map *lhs =
+        (const struct xc_core_memory_map *)lhs__;
+    const struct xc_core_memory_map *rhs =
+        (const struct xc_core_memory_map *)rhs__;
+
+    if (lhs->addr < rhs->addr)
+        return -1;
+    if (lhs->addr > rhs->addr)
+        return 1;
+
+    /* memory map overlap isn't allowed. complain */
+    DPRINTF("duplicated addresses are detected "
+            "(0x%" PRIx64 ", 0x%" PRIx64 "), "
+            "(0x%" PRIx64 ", 0x%" PRIx64 ")\n",
+            lhs->addr, lhs->size, rhs->addr, rhs->size);
     return 0;
 }
 
@@ -387,6 +409,8 @@ xc_domain_dumpcore_via_callback(int xc_h
                                       &memory_map, &nr_memory_map);
     if ( sts != 0 )
         goto out;
+    qsort(memory_map, nr_memory_map, sizeof(memory_map[0]),
+          &xc_memory_map_cmp);
 
     nr_pages = info.nr_pages;
     if ( !auto_translated_physmap )
diff -r 38513d22d234 -r e93d1eaef5b4 tools/libxc/xc_ptrace_core.c
--- a/tools/libxc/xc_ptrace_core.c	Thu Mar 08 15:21:10 2007 +0000
+++ b/tools/libxc/xc_ptrace_core.c	Fri Mar 09 10:46:12 2007 +0900
@@ -390,7 +390,6 @@ map_gmfn_to_offset_elf(unsigned long gmf
 {
     /* 
      * linear search
-     * There is no gurantee that those tables are sorted.
      */
     unsigned long i;
     if (current_is_auto_translated_physmap) {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.

[Keir Fraser]

On 9/3/07 01:52, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:

> dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.
> So far the order isn't specified and may be random in theory.
> But sorted array is requested by crash utility for efficient looking up.
> Fortunately it is the case except ia64 full virtualized domain.
> Update document such that those array must be sorted and fix the ia64
> full virtualized domain case.

Put the qsort() in the ia64 code if it's needed at all. But shouldn't it be
easy to generate the memory-area list in order in the first place?

 -- Keir

Re: [PATCH] dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.

[Isaku Yamahata]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Fri, Mar 09, 2007 at 09:59:40AM +0000, Keir Fraser wrote:
> On 9/3/07 01:52, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:
> 
> > dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.
> > So far the order isn't specified and may be random in theory.
> > But sorted array is requested by crash utility for efficient looking up.
> > Fortunately it is the case except ia64 full virtualized domain.
> > Update document such that those array must be sorted and fix the ia64
> > full virtualized domain case.
> 
> Put the qsort() in the ia64 code if it's needed at all. 

Moved.


> But shouldn't it be
> easy to generate the memory-area list in order in the first place?

Dave, do you have any opinion?

-- 
yamahata

[-- Attachment #2: 14288_12bc560784de_dump_core_pfn_ascending.patch --]
[-- Type: text/x-diff, Size: 4307 bytes --]

# HG changeset patch
# User yamahata@valinux.co.jp
# Date 1173439546 -32400
# Node ID 12bc560784de92e688226294cc213e3cd48a6d04
# Parent  38513d22d23420a90f94e7e0f70c564100e83851
dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.
So far the order isn't specified and may be random in theory.
But sorted array is requested by crash utility for efficient looking up.
Fortunately it is the case except ia64 full virtualized domain.
Update document such that those array must be sorted and fix the ia64
full virtualized domain case.
PATCHNAME: dump_core_pfn_ascending

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

diff -r 38513d22d234 -r 12bc560784de docs/misc/dump-core-format.txt
--- a/docs/misc/dump-core-format.txt	Thu Mar 08 15:21:10 2007 +0000
+++ b/docs/misc/dump-core-format.txt	Fri Mar 09 20:25:46 2007 +0900
@@ -80,8 +80,7 @@ Currently the following sections are def
                         gmfn:   machine physical frame number
                 The size of arrays is stored in xch_nr_pages member of header
                 note descriptor in .note.Xen note section.
-                There is no rule about the order. Analysis tools must no rely
-                on its order.
+                The entryies are stored in pfn-ascending order.
                 This section must exist when the domain is non auto
                 translated physmap mode. Currently x86 paravirtualized domain.
 
@@ -94,8 +93,7 @@ Currently the following sections are def
                 in .xen_pages section.
                 The size of arrays is stored in xch_nr_pages member of header
                 note descriptor in .note.Xen note section.
-                There is no rule about the order. Analysis tools must no rely
-                on its order.
+                The entries are stored in ascending order.
                 This section must exist when the domain is auto translated
                 physmap mode. Currently x86 full virtualized domain and
                 ia64 domain.
@@ -226,6 +224,8 @@ Currently only (major, minor) = (0, 1) i
 [When the format is changed, it would be described here.]
 
 (0, 1) update
+- .xen_p2m, .xen_pfn section
+  Arrays must be in pfn ascending order for efficient looking up.
 - EI_CLASS member of elf header was changed to ELFCLASS64 independent of
   architecture. This is mainly for x86_32pae.
   The format version isn't bumped because analysis tools can distinguish it.
diff -r 38513d22d234 -r 12bc560784de tools/libxc/xc_core_ia64.c
--- a/tools/libxc/xc_core_ia64.c	Thu Mar 08 15:21:10 2007 +0000
+++ b/tools/libxc/xc_core_ia64.c	Fri Mar 09 20:25:46 2007 +0900
@@ -22,6 +22,28 @@
 #include "xc_core.h"
 #include "xc_efi.h"
 #include "xc_dom.h"
+#include <inttypes.h>
+
+static int
+xc_memory_map_cmp(const void *lhs__, const void *rhs__)
+{
+    const struct xc_core_memory_map *lhs =
+        (const struct xc_core_memory_map *)lhs__;
+    const struct xc_core_memory_map *rhs =
+        (const struct xc_core_memory_map *)rhs__;
+
+    if (lhs->addr < rhs->addr)
+        return -1;
+    if (lhs->addr > rhs->addr)
+        return 1;
+
+    /* memory map overlap isn't allowed. complain */
+    DPRINTF("duplicated addresses are detected "
+            "(0x%" PRIx64 ", 0x%" PRIx64 "), "
+            "(0x%" PRIx64 ", 0x%" PRIx64 ")\n",
+            lhs->addr, lhs->size, rhs->addr, rhs->size);
+    return 0;
+}
 
 int
 xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
@@ -111,6 +133,7 @@ memory_map_get_old_hvm(int xc_handle, xc
     }
     *mapp = map;
     *nr_entries = i;
+    qsort(map, *nr_entries, sizeof(map[0]), &xc_memory_map_cmp);
     return 0;
 
 out:
@@ -196,6 +219,7 @@ xc_core_arch_memory_map_get(int xc_handl
     ret = 0;
 out:
     munmap(memmap_info, PAGE_SIZE);
+    qsort(map, *nr_entries, sizeof(map[0]), &xc_memory_map_cmp);
     return ret;
     
 old:
diff -r 38513d22d234 -r 12bc560784de tools/libxc/xc_ptrace_core.c
--- a/tools/libxc/xc_ptrace_core.c	Thu Mar 08 15:21:10 2007 +0000
+++ b/tools/libxc/xc_ptrace_core.c	Fri Mar 09 20:25:46 2007 +0900
@@ -390,7 +390,6 @@ map_gmfn_to_offset_elf(unsigned long gmf
 {
     /* 
      * linear search
-     * There is no gurantee that those tables are sorted.
      */
     unsigned long i;
     if (current_is_auto_translated_physmap) {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.

[Keir Fraser]

On 9/3/07 11:31, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:

>> But shouldn't it be
>> easy to generate the memory-area list in order in the first place?
> 
> Dave, do you have any opinion?

I'm just thinking, it looks like a static list to me. Maybe you don't want
to make assumptions based on the #define'd constants though? Either way, I
think requiring xc_core_arch_memory_map_get() to return the required
ordering makes sense.

 -- Keir

Re: [PATCH] dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.

[Dave Anderson]

Isaku Yamahata wrote:

> On Fri, Mar 09, 2007 at 09:59:40AM +0000, Keir Fraser wrote:
> > On 9/3/07 01:52, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:
> >
> > > dump-core: store .xen_p2m or .xen_pfn section in pfn ascending order.
> > > So far the order isn't specified and may be random in theory.
> > > But sorted array is requested by crash utility for efficient looking up.
> > > Fortunately it is the case except ia64 full virtualized domain.
> > > Update document such that those array must be sorted and fix the ia64
> > > full virtualized domain case.
> >
> > Put the qsort() in the ia64 code if it's needed at all.
>
> Moved.
>
> > But shouldn't it be
> > easy to generate the memory-area list in order in the first place?
>
> Dave, do you have any opinion?
>

Nope -- pfn-ascending order in the two arrays is all I'm requesting...

Thanks,
  Dave