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

[Tej <bewith.tej@xxxxxxxxx>]

On Thu, Mar 26, 2009 at 4:27 PM, Gianluca Guida
<Gianluca.Guida@xxxxxxxxxxxxx> wrote:
> Hi,
>
> Sorry for the late reply.
>
> On Mar 26, 2009, at 7:59 AM, Tej wrote:
>
>> On Mon, Mar 2, 2009 at 9:36 PM, Tej <bewith.tej@xxxxxxxxx> wrote:
>>>
>>> On Sat, Feb 28, 2009 at 12:32 AM, Gianluca Guida
>>> <gianluca.guida@xxxxxxxxxxxxx> wrote:
>>>>
>>>> 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.
>>>>
>>>> - 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
>>>> multiple times? If so, it would be useful to remove them on 'make
>>>> clean'.
>>>> Also, it would be useful to make this feature enabled with a
>>>> compile-time
>>>> option.
>>>
>>> Attached patches address the four-spaces tabs and make clean issues.
>>> Hope the code is more clean and readable
>>
>> Hi
>>
>> Sorry for interrupting you guys from your busy schedule, as i just
>> want to know the current status for GCOV patches i submitted early
>> this month.
>> any feedback on "testing/coding/design/feature" will be appreciated.
>
> I've been testing it for a while, and it seems an useful and interesting
> feature.
>
> A few comments, though:
>
> - As I said, while the patch is minimal enough, the feature requires some
> intrusive changes in Xen, like changing the linker script (via the
> definition of CONSTRUCTORS) and in general it changes the compiler behavior,
> so I think it should be mandatory to make this feature explicitely enabled
> at compile time, e.g.  gcov=y.
> - I am talking about Xen code only, but I think a few part of the codes
> should be more clear. I find very confusing the difference between
> xen_gcov_info and gcov_info, and all the instructions that uses them. A
> better naming scheme for the variables and more comments would be
> appreciated.

Removed xen_gcov_info dependency all together. Complete implementation
is based on gcc "gcov_info" struct only.
We have also included some meaningful names and comments. I hope that
these changes makes code more clear/clean and readable.

I have tested code in amd-64 with gcov=y and gcov=n, all worked fine,

Are you looking for any other other specific feature to be included in this?
We are missing only modules implementation of gcov which is not
required for xen hypervisor.

Any further comments/suggestion are more than welcome.

thanks
-tej & team


>
> Thanks!
> Gianluca
>

Attachment: linux-2.6.18-gcov-v5.patch
Description: Text Data

Attachment: xen-3.3-gcov-v5.patch
Description: Text Data

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