Re: [Xen-ia64-devel] [PATCH 0/7 TAKE 2] xenoprof for xen/ia64

[Alex Williamson <alex.williamson@xxxxxx>]

On Wed, 2006-11-22 at 21:03 +0900, Isaku Yamahata wrote:
> Now xenoprof/common changes are committed.
> (At least they are in staging tree.)
> I attached updated xenoprof/ia64 patches as tar ball for convinience.

Hi Isaku,

   A few comments:

---- 12592_7aa7bb96eec7_xenoprof_ia64_xen_side.patch

$ grep "^+#if.*def XEN" 12592_7aa7bb96eec7_xenoprof_ia64_xen_side.patch | wc -l
126

Wow.  Makes perfmon.c a little difficult to read in places, but the file
is so big there are really only a few places where it gets in the way.
Some linux-null headers may clean up a few of the trivial ones towards
the top of the file.  I don't have any good suggestions for the others.


+       if ( !softirq_pending(smp_processor_id()) ) {
+               if (!can_do_pal_halt)
+                       safe_halt();
+               else
+                       cpu_relax();
+       }

Is this logic backwards?  It's opposite the kernel.

--- a/xen/include/public/arch-ia64.h    Thu Oct 26 14:27:25 2006 +0900
+++ b/xen/include/public/arch-ia64.h    Wed Nov 22 20:41:08 2006 +0900
@@ -388,6 +388,9 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_conte
 #define IA64_DOM0VP_tlb_untrack_page    10


+/* xen perfmon */
+#define IA64_DOM0VP_perfmon             11
+

We don't have IA64_DOM0VP_tlb_untrack_page in the upstream tree, should
this be IA64_DOM0VP op #8? (or are we missing a patch upstream)

---- 12593_12361c7cc046_xenoprof_ia64_linux_side.patch

Having two different definitions for __perfmon_init/exit() for the
non-Xen code path is confusing.  Consider accessing it through a macro
in the Xen path (ex. xen_permon_init).  Then __perfmon_init/exit() could
be static for the non-Xen case (not that I'm a fan of adding more
#ifdefs).  Perhaps we need a #define LINUX_STATIC (?)

Thanks,

        Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.


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