[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@xxxxxxxxxx> wrote:
> xenoprof: Make the escape code consistent across 32 and 64-bit xen
> 
> At the moment, the xenoprof escape code is defined as "~0UL".
> Unfortunately, this expands to 0xffffffff on 32-bit systems
> and 0xffffffffffffffff on 64-bit systems; with the result that
> while 32-on-32 and 64-in-64 work fine, 32-on-64 (also known as
> "compat mode") is broken.
> 
> This patch makes the definition consistent across architectures.
> In so doing, it will break old-32-bit-on-new-Xen, and vice versa;
> but this was seen as an acceptable thing to do.
> 
> Signed-off-by: Marcus Granado <marcus.granado@xxxxxxxxxxxxx>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

NAK.

> diff -r 870db1ebc0df -r bcd2c4f0c502 xen/include/public/xenoprof.h
> --- 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.

I'd also be curious to see the kernel side change accompanying this
(if trivial, it might be a good idea to commit this to the old 2.6.18
tree for future reference).

Jan

>   /* Transient events for the xenoprof->oprofile cpu buf */
>   #define XENOPROF_TRACE_BEGIN 1




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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.