RE: [Xen-ia64-devel] [PATCH] xentrace buffer alignment

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

Hi, Keir,
        IA64 compiler does promise 8-byte alignment for data segment compiled 
into binary if necessary, but not for dynamic allocated data. The problematic 
code is:
(alloc_trace_bufs)
                buf = t_bufs[i] = (struct t_buf 
*)&rawbuf[i*opt_tbuf_size*PAGE_SIZE];
        buf->cons = buf->prod = 0;
        buf->nr_recs = nr_recs;
        t_recs[i] = (struct t_rec *)(buf + 1);

        Here t_buf is aligned by 4 bytes as:
struct t_buf {
    unsigned int  cons;      /* Next item to be consumed by control tools. */
    unsigned int  prod;      /* Next item to be produced by Xen.           */
    unsigned int  nr_recs;   /* Number of records in this trace buffer.    */
    /* 'nr_recs' records follow immediately after the meta-data header.    */
};

        However t_rec should be aligned by 8 bytes as:
struct t_rec {
    uint64_t cycles;          /* cycle counter timestamp */
    uint32_t event;           /* event ID                */
    unsigned long data[5];    /* event data items        */
};

        So t_rec will be placed at middle of 8 bytes boundary by t_recs[i] = 
(struct t_rec *)(buf + 1). Then later access to 64bit field within t_rec is 
unaligned access.

Thanks,
Kevin

>-----Original Message-----
>From: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
>[mailto:xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Keir Fraser
>Sent: 2005年11月24日 21:34
>To: Masaki Kanno
>Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [Xen-ia64-devel] [PATCH] xentrace buffer alignment
>
>
>Why is explicit 8-byte alignment required? Does ia64 not guarantee to
>align 64-bit values on an 8-byte boundary? If not, then what
>instruction is it that requires this alignment -- there can't be very
>many, otherwise surely the compiler would enforce sufficient alignment?
>
>I'm not happy about adding alignment attributes to the header files, as
>it would be good to not make them gcc-specific.
>
>  -- Keir
>
>On 24 Nov 2005, at 12:05, Masaki Kanno wrote:
>
>> Hi, Kevin
>>
>> OK, I will send patch to the Xen-devel mailing list.
>>
>> Thanks,
>>  kan
>>
>>>> -----Original Message-----
>>>> From: Masaki Kanno
>>>> Sent: 2005定11??24?? 17:21
>>>> Hi, Rob, Kevin,
>>>>
>>>>> The alignment directive is necessary there since they're
>>>>> dynamically marked
>>>>> on an allocated buf. Or how about adding padding bytes to avoid
>>>>> using compiler
>>>>> directive and ifdef? Then, still no need for "t_rec".
>>>>
>>>> Sorry, "t_rec" alignment is mistake. I thought "sizeof(t_rec) = 52
>>>> bytes".
>>>>
>>>> The patch was made on Kevin's idea.
>>>> However, I'm worried. When someone adds other members to "t_buf",
>>>> isn't alignment
>>>> for ia64 forgotten?
>>>
>>> I meant to add padding bytes like "char padding[4]" with warning to
>>> developer that 8 bytes alignment should be promised.
>> But now I think your original ".align" approach may be easier without
>> concern how many padding bytes need to be there on
>> different architecture. So you can send out a patch with your original
>> ".align" approach (but remove "ifdef __ia64__" to
>> xen mailing list since it's a common code modification. Also please
>> keep a comment to warn alignment requirement here.
>> ;-)
>>>
>>> Thanks,
>>> Kevin
>>>>
>>>> Signed-off-by: Masaki Kanno <kanno.masaki@xxxxxxxxxxxxxx>
>>>>
>>>> Thanks,
>>>> kan
>>>>
>>>> diff -r 51f32d60536b xen/include/public/trace.h
>>>> --- a/xen/include/public/trace.h        Fri Nov 18 00:35:14 2005
>>>> +++ b/xen/include/public/trace.h        Thu Nov 24 18:04:31 2005
>>>> @@ -69,6 +69,7 @@
>>>>     unsigned int  prod;      /* Next item to be produced by Xen.
>>>>       */
>>>>     unsigned int  nr_recs;   /* Number of records in this trace
>>>> buffer.    */
>>>>     /* 'nr_recs' records follow immediately after the meta-data
>>>> header.    */
>>>> +    unsigned int  align_buf; /* 8 bytes alignment for ia64
>>>>        */
>>>> };
>>>>
>>>> #endif /* __XEN_PUBLIC_TRACE_H__ */
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-ia64-devel mailing list
>>>> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>>>> http://lists.xensource.com/xen-ia64-devel
>
>
>_______________________________________________
>Xen-ia64-devel mailing list
>Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
>http://lists.xensource.com/xen-ia64-devel

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