Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/10/16 14:26, George Dunlap wrote:
> On 12/10/16 14:24, George Dunlap wrote:
>> On 12/10/16 14:06, Wei Liu wrote:
>>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>>>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/common/gcov/gcc_4_7.c
>>>>> @@ -0,0 +1,205 @@
>>>>> +/*
>>>>> + *  This code provides functions to handle gcc's profiling data format
>>>>> + *  introduced with gcc 4.7.
>>>>> + *
>>>>> + *  This file is based heavily on gcc_3_4.c file.
>>>>> + *
>>>>> + *  For a better understanding, refer to gcc source:
>>>>> + *  gcc/gcov-io.h
>>>>> + *  libgcc/libgcov.c
>>>>> + *
>>>>> + *  Uses gcc-internal data definitions.
>>>>> + *
>>>>> + *  Imported from Linux and modified for Xen by
>>>>> + *    Wei Liu <wei.liu2@citrix.com>
>>>>> + */
>>>>> +
>>>>> +#include <xen/string.h>
>>>>> +
>>>>> +#include "gcov.h"
>>>>> +
>>>>> +#if GCC_VERSION < 40700
>>>>> +#error "Wrong version of GCC used to compile gcov"
>>>>> +#endif
>>>>> +
>>>>> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
>>>>> +#define GCOV_COUNTERS                   10
>>>>> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
>>>>> +#define GCOV_COUNTERS                   9
>>>>> +#else
>>>>> +#define GCOV_COUNTERS                   8
>>>>> +#endif
>>>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>>>> but then didn't finish analyzing the situation), but I'm afraid this
>>>> together with ...
>>>>
>>>>> +struct gcov_info {
>>>>> +    unsigned int version;
>>>>> +    struct gcov_info *next;
>>>>> +    unsigned int stamp;
>>>>> +    const char *filename;
>>>>> +    void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>>>> +    unsigned int n_functions;
>>>>> +    struct gcov_fn_info **functions;
>>>>> +};
>>>> ... this structure's trailing fields actually getting used by the code
>>>> won't work well when changing compiler versions without cleaning
>>>> the tree.  I think instead you need thin gcc_5.c and gcc_4_9.c
>>>> #define-ing their GCOV_COUNTERS and then #include-ing this
>>>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>>>> development variant of 5.x) would use anything different from
>>>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>>>> normally be necessary anymore with gcc 5+.
>>>>
>>> Right. I will do something about this. Thanks for catching this.
>>>
>>>> And then - how is all of this supposed to be working in conjucntion
>>>> with live patching, where the patch may have been created by yet
>>>> another compiler version?
>>>>
>>> There is a version field in gcov_info, so we can compare that and reject
>>> incompatible version.
>>>
>>> We need to use hooks in livepatching to call the constructor /
>>> destructor when applying / reverting a live-patch.  We might need to be
>>> cautious about locks or whatever, but I'm sure it can be figured out.
>>>
>>> But I have no idea how useful it would be to use gcov and livepatching
>>> together.  For now the easiest thing to do is to
>>>
>>>    depends on !LIVEPATCH
>>>
>>> in Kconfig.
>> Wouldn't it be just as easy, and more useful, to set a "has been
>> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>>
>> I would expect most users to want to build a single hypervisor that can
>> be used for both gcov testing and live patching (under different
>> circumstances).
> I mean software provider, not user, of course.  That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.

GCOV is majority invasive, both in terms of performance (every basic
block needs to do a locked increment of a counter) and data size
(metadata for all basic blocks).

No software vendor is ever going to have it enabled in a production
scenario.

~Andrew

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