Re: [Xen-devel] [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.01.12 at 11:47, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>>>> On 23.01.12 at 11:16, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
>> On Mon, 2012-01-23 at 09:57 +0000, Jan Beulich wrote:
>>> >>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@xxxxxxxxxx> wrote:
>>> > --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
>>> > +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
>>> > @@ -68,7 +68,7 @@ struct event_log {
>>> >   };
>>> > 
>>> >   /* PC value that indicates a special code */
>>> > -#define XENOPROF_ESCAPE_CODE ~0UL
>>> > +#define XENOPROF_ESCAPE_CODE (~0ULL)
>>> 
>>> You cannot just go and change a public definition, as the consuming
>>> side will break. You need to introduce a new escape code, and
>>> provide a mechanism to negotiate (between hypervisor and kernel)
>>> which one to use.
>> 
>> This seems more like a bug fix to me than an interface change, the fact
>> that both producer and consumer had the bug notwithstanding.
>> 
>> Since this is a developer oriented interface I think we can be a little
>> more relaxed than we would be for other interface changes. In this case
>> we are fixing 32on64 (quite common) at the expense of 32on32 (very
>> uncommon these days, especially for the suybset of developers wanting to
>> do profiling) and leaving 64on64 (quite common) as it is . That seems
>> like a worthwhile tradeoff to me.
> 
> I see your point. However, I'd still like to be careful with this - what
> we think things ought to be used for doesn't necessarily cover all
> cases that they are in reality.

I see that it got applied unchanged. It introduces two warnings in
the 32-bit build, which not only fails that build, but also indicates
that the situation likely wasn't fully analyzed.

Jan


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