[ltt-dev] [PATCH 07/13] implement fmt align
Lai Jiangshan
laijs at cn.fujitsu.com
Thu Jan 15 21:41:21 EST 2009
Mathieu Desnoyers wrote:
> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>> Mathieu Desnoyers wrote:
>>> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>>>> Mathieu Desnoyers wrote:
>>>>> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>>>>>> we need know the align for accessing the binary data from
>>>>>> event.(skip the padding data, calculate offset).
>>>>>>
>>>>>> It can also be used for write-side.
>>>>>>
>>>>> Yes, but I prefer not, because I like to do a single pass in the format
>>>>> string to calculate both the type sizes individually and the largest
>>>>> alignment. It makes sense to implement this for the read-side though.
>>>>>
>>>>> Please see comments below,
>>>>>
>>>>>> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
>>>>>> ---
>>>>>> b/ltt/ltt-serialize.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>> include/linux/ltt-tracer.h | 9 ++++++
>>>>>> 2 files changed, 69 insertions(+)
>>>>>> diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
>>>>>> index ece6819..b49cac2 100644
>>>>>> --- a/include/linux/ltt-tracer.h
>>>>>> +++ b/include/linux/ltt-tracer.h
>>>>>> @@ -311,6 +316,9 @@ static inline unsigned int ltt_align(size_t align_drift, size_t size_of_type)
>>>>>> size_t alignment = min(sizeof(void *), size_of_type);
>>>>>> return (alignment - align_drift) & (alignment - 1);
>>>>>> }
>>>>>> +
>>>>>> +extern unsigned int ltt_fmt_largest_align(size_t align_drift, const char *fmt);
>>>>>> +
>>>>>> /* Default arch alignment */
>>>>>> #define LTT_ALIGN
>>>>>>
>>>>>> @@ -327,6 +335,12 @@ static inline unsigned int ltt_align(size_t align_drift,
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> +static inline unsigned int ltt_fmt_largest_align(size_t align_drift,
>>>>>> + const char *fmt)
>>>>>> +{
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> #define LTT_ALIGN __attribute__((packed))
>>>>>>
>>>>>> static inline int ltt_get_alignment(void)
>>>>>> diff --git a/ltt/ltt-serialize.c b/ltt/ltt-serialize.c
>>>>>> index 5c34899..deb9106 100644
>>>>>> --- a/ltt/ltt-serialize.c
>>>>>> +++ b/ltt/ltt-serialize.c
>>>>>> @@ -531,6 +538,66 @@
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(ltt_serialize_printf);
>>>>>>
>>>>>> +#ifdef CONFIG_LTT_ALIGNMENT
>>>>>> +
>>>>>> +unsigned int ltt_fmt_largest_align(size_t align_drift, const char *fmt)
>>>>>> +{
>>>>>> + char trace_size = 0, c_size = 0;
>>>>>> + enum ltt_type trace_type = LTT_TYPE_NONE, c_type = LTT_TYPE_NONE;
>>>>>> + unsigned long attributes = 0;
>>>>>> + int largest_align = 1;
>>>>>> +
>>>>>> + for (; *fmt ; ++fmt) {
>>>>>> + switch (*fmt) {
>>>>>> + case '#':
>>>>>> + /* tracetypes (#) */
>>>>>> + ++fmt; /* skip first '#' */
>>>>>> + if (*fmt == '#') /* Escaped ## */
>>>>>> + break;
>>>>>> + attributes = 0;
>>>>>> + fmt = parse_trace_type(fmt, &trace_size, &trace_type,
>>>>>> + &attributes);
>>>>>> +
>>>>>> + largest_align = max_t(int, largest_align, trace_size);
>>>>>> + if (largest_align >= ltt_get_alignment())
>>>>>> + goto exit;
>>>>>> + break;
>>>>>> + case '%':
>>>>>> + /* c types (%) */
>>>>>> + ++fmt; /* skip first '%' */
>>>>>> + if (*fmt == '%') /* Escaped %% */
>>>>>> + break;
>>>>>> + fmt = parse_c_type(fmt, &c_size, &c_type, NULL);
>>>>>> + /*
>>>>>> + * Output c types if no trace types has been
>>>>>> + * specified.
>>>>>> + */
>>>>>> + if (!trace_size)
>>>>>> + trace_size = c_size;
>>>>>> + if (trace_type == LTT_TYPE_NONE)
>>>>>> + trace_type = c_type;
>>>>>> + if (c_type == LTT_TYPE_STRING)
>>>>>> + trace_type = LTT_TYPE_STRING;
>>>>>> +
>>>>>> + largest_align = max_t(int, largest_align, trace_size);
>>>>>> + if (largest_align >= ltt_get_alignment())
>>>>>> + goto exit;
>>>>>> +
>>>>>> + trace_size = 0;
>>>>>> + c_size = 0;
>>>>>> + trace_type = LTT_TYPE_NONE;
>>>>>> + c_size = LTT_TYPE_NONE;
>>>>>> + break;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> +exit:
>>>>>> + return (largest_align - align_drift) & (largest_align - 1);
>>>>> Hrm, I think there is something wrong here.
>>>>>
>>>>> On x86_32, the max alignment is sizeof(u32).
>>>>>
>>>>> However, if you get a u64 type here, the largest_align will become
>>>>> sizeof(u64). This is why, in ltt_vtrace(), I do a
>>>>>
>>>>> largest_align = 1; /* must be non-zero for ltt_align */
>>>>> data_size = ltt_get_data_size(&closure, serialize_private,
>>>>> &largest_align, fmt, &args_copy);
>>>>> largest_align = min_t(int, largest_align, sizeof(void *));
>>>>>
>>>>> So we should document in the ltt_fmt_largest_align header to always do a
>>>>> largest_align = min_t(int, largest_align, sizeof(void *)); on the return
>>>>> value (and check that we do it) or we simply do this operation before
>>>>> returning.
>>>>>
>>>>> Mathieu
>>>> I haven't seen this line:
>> *You haven't seen this line*:
>>
>
> Ok, this helps :)
>
>>>> if (largest_align >= ltt_get_alignment())
>>>> goto exit;
>>>>
>>>> We shouldn't hardcode "sizeof(void *)", we should use ltt_get_alignment().
>>>>
>>> My comment was that it's necessary to do a
>>>
>>> largest_align = min_t(int, largest_align, sizeof(void *));
>>>
>>> to make sure the alignment value is never bigger that sizeof(void *). I
>> I have insured value is never bigger that sizeof(void *)
>>
>
> Hrm, but when you goto exit, you actually do :
>
> +exit:
> + return (largest_align - align_drift) & (largest_align - 1);
>
> If largest_align == sizeof(u64) on a 32-bit machine, then we happen to
> return an alignment which will be on 64-bits and not 32.
>
> Or is there some magic I've missed ?
>
> Mathieu
Ouch! I did concern the things as you said, but I coded it with mistake.
Sorry.
>
>
>>> don't understand how your answer here relates to my concern. (maybe I
>>> have problem understanding your answer on my side..). Can you elaborate
>>> more please ?
>>>
>>> Mathieu
>>>
>>>
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(ltt_fmt_largest_align);
>>>>>> +
>>>>>> +#endif
>>>>>> +
>>>>>> /*
>>>>>> * Calculate data size
>>>>>> * Assume that the padding for alignment starts at a sizeof(void *) address.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> ltt-dev mailing list
>>>>>> ltt-dev at lists.casi.polymtl.ca
>>>>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>>>>>
>>>>
>>>> _______________________________________________
>>>> ltt-dev mailing list
>>>> ltt-dev at lists.casi.polymtl.ca
>>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>>>
>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
>
More information about the lttng-dev
mailing list