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

Re: [Xen-devel] [PATCH v1 01/13] Export hypervisor symbols



On 09/11/2013 03:51 AM, Jan Beulich wrote:
On 10.09.13 at 17:20, Boris Ostrovsky<boris.ostrovsky@xxxxxxxxxx>  wrote:
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -102,11 +102,11 @@ $(BASEDIR)/common/symbols-dummy.o:
  $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
        $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
            $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
-       $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
+       $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols --all-symbols 
>$(@D)/.$(@F).0.S
For one I can't see what use data symbols have for performance
analysis.

They are used by perf, similarly to kallsyms (usually when debug symbols from
binary are not available)

And then I'm opposed to growing the symbol table size
unconditionally for no good reason.

I think I can remove --all-symbols, it is not strictly necessary for what I plan now for
perf. We may need to add it later, possibly with a config option.

--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -527,6 +527,26 @@ struct xenpf_core_parking {
  typedef struct xenpf_core_parking xenpf_core_parking_t;
  DEFINE_XEN_GUEST_HANDLE(xenpf_core_parking_t);
+#define XENPF_get_symbols 61
+
+#define XENSYMS_SZ         4096
This doesn't appear to belong into the public interface.

Linux driver needs to know size of the buffer that is passed from
the hypervisir. I suppose I can just use PAGE_SIZE.

+struct xenpf_symdata {
+    /*
+     * offset into Xen's symbol data and symbol number from
+     * last call. Used only by Xen.
+     */
+    uint64_t xen_offset;
+    uint64_t xen_symnum;
I wonder whether that's really a suitable mechanism.

Why do you think this is not suitable?

Linux needs to keep track of position in the symbol table while
it is walking over the file, otherwise we will need to keep the state
in hypervisor which is much less desirable.


+
+   /*
+    * Symbols data, formatted similar to /proc/kallsyms:
+    *   <address> <type> <name>
+    */
+    XEN_GUEST_HANDLE(char) buf;
This is too simplistic: Please use a proper structure here, to allow
switching the internal symbol table representation (which I have on
my todo list) without having to mimic old behavior.

I don't think I know what you are referring to here.

-boris


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


 


Rackspace

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