Re: [Xen-devel] [PATCH-0/2] Hypervisor profiling using GCOV (64bit Hypervisor)

[Tej <bewith.tej@xxxxxxxxx>]

On Sat, Feb 28, 2009 at 12:32 AM, Gianluca Guida
<gianluca.guida@xxxxxxxxxxxxx> wrote:
> Hi,
>
> Tej wrote:
>>
>> Sending Patches for the 64 bit Hypervisor, We have tested patches on
>> AMD-64 (Athlon(tm) 64 X2 Dual Core Processor 4600+) machine with
>> gcc-4.2 & gcc-3.4. Please refer the previous GCOV RFC in same mail for
>> more info on Hypervisor profiling.
>> We have added support to 32bit and 64bit Kernel.
>>
>> In addition to patches for hypervisor profiling, we did a little work
>> on *lcov* to work with hypervisor.
>> README could be useful, for naive lcov user. locv-diff.patch show our
>> change in lcov scripts.
>>
>> any comments, feedback and suggestion are more than welcome
>
> While I still need to test the patch (building 3.3 right now) and to
> understand gcov internals, I think that a few comments can be done, mostly
> aestethicals.
>
> - xen coding style: Using four-spaces tabs is generally the tradition. Also
> I do prefer to have brackets that start code blocks on a new line aligned to
> the previous line, but that's not followed everywhere in the code.

thanks for your comments and general feedback
Generally we uses Lindent script for tab/alignment problem...  i will
incorporate above said coding styles and resubmit
>
> - Makefiles: while the num=$*.c is still a mystery to me, my question is: do
> you really need to make links with different names to files compiled

num=$*.c changes mainly added to run lcov smoothly... Let me take an example

in hap directory Makefile generate guest_walk_%level.o and gcov option
generates guest_walk_%level.gcno... but there is no
guest_walk_%level.c files , so lcov script give me warning/error
saying guest_walk_%level.gcno file does not have correspoding *.c.. so
we have added some Makefile trick to generate *.c link at compile
time.

> multiple times? If so, it would be useful to remove them on 'make clean'.
nice point
thats absolutely needed,  so i will incorporated the changes

> Also, it would be useful to make this feature enabled with a compile-time
> option.

i will look into this also and resubmit the patches on Monday...
>
> More comments to follow as I test it.

thanks for your review and comments

>
> Thanks,
> Gianluca
>

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