* [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
@ 2009-01-13 9:09 Lai Jiangshan
2009-01-15 18:48 ` Mathieu Desnoyers
0 siblings, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2009-01-13 9:09 UTC (permalink / raw)
we need function read the base type from ltt-relay buffer
in the kernel for text output
Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
ltt-serialize.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
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,77 @@
}
EXPORT_SYMBOL_GPL(ltt_serialize_data);
+static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
+ char trace_size, enum ltt_type trace_type)
+{
+ union {
+ uint8_t u_8;
+ uint16_t u_16;
+ uint32_t u_32;
+ uint64_t u_64;
+ int8_t s_8;
+ int16_t s_16;
+ int32_t s_32;
+ int64_t s_64;
+ } tmp;
+ uint64_t value;
+
+ if (ltt_get_alignment())
+ *ppos += ltt_align(*ppos, trace_size);
+
+ switch (trace_type) {
+ case LTT_TYPE_SIGNED_INT:
+ switch (trace_size) {
+ case 1:
+ ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
+ value = tmp.s_8;
+ break;
+ case 2:
+ ltt_relay_read(buf, *ppos, &tmp.s_16, trace_size);
+ value = tmp.s_16;
+ break;
+ case 4:
+ ltt_relay_read(buf, *ppos, &tmp.s_32, trace_size);
+ value = tmp.s_32;
+ break;
+ case 8:
+ ltt_relay_read(buf, *ppos, &tmp.s_64, trace_size);
+ value = tmp.s_64;
+ break;
+ default:
+ BUG();
+ }
+ break;
+ case LTT_TYPE_UNSIGNED_INT:
+ switch (trace_size) {
+ case 1:
+ ltt_relay_read(buf, *ppos, &tmp.u_8, trace_size);
+ value = tmp.u_8;
+ break;
+ case 2:
+ ltt_relay_read(buf, *ppos, &tmp.u_16, trace_size);
+ value = tmp.u_16;
+ break;
+ case 4:
+ ltt_relay_read(buf, *ppos, &tmp.u_32, trace_size);
+ value = tmp.u_32;
+ break;
+ case 8:
+ ltt_relay_read(buf, *ppos, &tmp.u_64, trace_size);
+ value = tmp.u_64;
+ break;
+ default:
+ BUG();
+ }
+ break;
+ default:
+ BUG();
+ }
+
+ *ppos += trace_size;
+ return value;
+}
+
/*
* Calculate data size
* Assume that the padding for alignment starts at a sizeof(void *) address.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
2009-01-13 9:09 [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer Lai Jiangshan
@ 2009-01-15 18:48 ` Mathieu Desnoyers
2009-01-17 3:24 ` Lai Jiangshan
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers @ 2009-01-15 18:48 UTC (permalink / raw)
* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>
> we need function read the base type from ltt-relay buffer
> in the kernel for text output
>
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
> ltt-serialize.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
> 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,77 @@
> }
> EXPORT_SYMBOL_GPL(ltt_serialize_data);
>
> +static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
Maybe the name "unserialize" would be more appropriate ?
> + char trace_size, enum ltt_type trace_type)
> +{
> + union {
> + uint8_t u_8;
> + uint16_t u_16;
> + uint32_t u_32;
> + uint64_t u_64;
> + int8_t s_8;
> + int16_t s_16;
> + int32_t s_32;
> + int64_t s_64;
> + } tmp;
> + uint64_t value;
> +
> + if (ltt_get_alignment())
This if () is unneeded. It's already in the ltt_align logic.
> + *ppos += ltt_align(*ppos, trace_size);
> +
> + switch (trace_type) {
> + case LTT_TYPE_SIGNED_INT:
> + switch (trace_size) {
> + case 1:
> + ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
> + value = tmp.s_8;
I think we could separate that in two parts to make the code smaller :
1 -
ltt_relay_read for u8, u16, u32, u64 (not caring about singedness)
2 - depending on the sign, cast to (s8, s16, s32) and then cast to a
64-bit signed value. Or if it was already 64-bits, no conversion is
needed.
The end result (64-bit signed or unsigned value) should in the end be
casted to a u64.
Mathieu
> + break;
> + case 2:
> + ltt_relay_read(buf, *ppos, &tmp.s_16, trace_size);
> + value = tmp.s_16;
> + break;
> + case 4:
> + ltt_relay_read(buf, *ppos, &tmp.s_32, trace_size);
> + value = tmp.s_32;
> + break;
> + case 8:
> + ltt_relay_read(buf, *ppos, &tmp.s_64, trace_size);
> + value = tmp.s_64;
> + break;
> + default:
> + BUG();
> + }
> + break;
> + case LTT_TYPE_UNSIGNED_INT:
> + switch (trace_size) {
> + case 1:
> + ltt_relay_read(buf, *ppos, &tmp.u_8, trace_size);
> + value = tmp.u_8;
> + break;
> + case 2:
> + ltt_relay_read(buf, *ppos, &tmp.u_16, trace_size);
> + value = tmp.u_16;
> + break;
> + case 4:
> + ltt_relay_read(buf, *ppos, &tmp.u_32, trace_size);
> + value = tmp.u_32;
> + break;
> + case 8:
> + ltt_relay_read(buf, *ppos, &tmp.u_64, trace_size);
> + value = tmp.u_64;
> + break;
> + default:
> + BUG();
> + }
> + break;
> + default:
> + BUG();
> + }
> +
> + *ppos += trace_size;
> + return value;
> +}
> +
> /*
> * 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
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
2009-01-15 18:48 ` Mathieu Desnoyers
@ 2009-01-17 3:24 ` Lai Jiangshan
2009-01-17 4:37 ` Mathieu Desnoyers
0 siblings, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2009-01-17 3:24 UTC (permalink / raw)
Mathieu Desnoyers wrote:
> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>> we need function read the base type from ltt-relay buffer
>> in the kernel for text output
>>
>> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
>> ---
>> ltt-serialize.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 71 insertions(+)
>> 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,77 @@
>> }
>> EXPORT_SYMBOL_GPL(ltt_serialize_data);
>>
>> +static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
>
> Maybe the name "unserialize" would be more appropriate ?
>
>> + char trace_size, enum ltt_type trace_type)
>> +{
>> + union {
>> + uint8_t u_8;
>> + uint16_t u_16;
>> + uint32_t u_32;
>> + uint64_t u_64;
>> + int8_t s_8;
>> + int16_t s_16;
>> + int32_t s_32;
>> + int64_t s_64;
>> + } tmp;
>> + uint64_t value;
>> +
>> + if (ltt_get_alignment())
>
> This if () is unneeded. It's already in the ltt_align logic.
>
>> + *ppos += ltt_align(*ppos, trace_size);
>> +
>> + switch (trace_type) {
>> + case LTT_TYPE_SIGNED_INT:
>> + switch (trace_size) {
>> + case 1:
>> + ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
>> + value = tmp.s_8;
>
> I think we could separate that in two parts to make the code smaller :
separate it into 2 functions?
>
> 1 -
> ltt_relay_read for u8, u16, u32, u64 (not caring about singedness)
>
> 2 - depending on the sign, cast to (s8, s16, s32) and then cast to a
> 64-bit signed value. Or if it was already 64-bits, no conversion is
> needed.
>
> The end result (64-bit signed or unsigned value) should in the end be
> casted to a u64.
>
> Mathieu
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
2009-01-17 3:24 ` Lai Jiangshan
@ 2009-01-17 4:37 ` Mathieu Desnoyers
2009-01-19 2:27 ` Lai Jiangshan
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers @ 2009-01-17 4:37 UTC (permalink / raw)
* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> > * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> >> we need function read the base type from ltt-relay buffer
> >> in the kernel for text output
> >>
> >> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >> ---
> >> ltt-serialize.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 71 insertions(+)
> >> 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,77 @@
> >> }
> >> EXPORT_SYMBOL_GPL(ltt_serialize_data);
> >>
> >> +static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
> >
> > Maybe the name "unserialize" would be more appropriate ?
> >
> >> + char trace_size, enum ltt_type trace_type)
> >> +{
> >> + union {
> >> + uint8_t u_8;
> >> + uint16_t u_16;
> >> + uint32_t u_32;
> >> + uint64_t u_64;
> >> + int8_t s_8;
> >> + int16_t s_16;
> >> + int32_t s_32;
> >> + int64_t s_64;
> >> + } tmp;
> >> + uint64_t value;
> >> +
> >> + if (ltt_get_alignment())
> >
> > This if () is unneeded. It's already in the ltt_align logic.
> >
> >> + *ppos += ltt_align(*ppos, trace_size);
> >> +
> >> + switch (trace_type) {
> >> + case LTT_TYPE_SIGNED_INT:
> >> + switch (trace_size) {
> >> + case 1:
> >> + ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
> >> + value = tmp.s_8;
> >
> > I think we could separate that in two parts to make the code smaller :
>
> separate it into 2 functions?
>
Not necessarily 2 functions. Just separating it in two switch () statements
would do the job.
Mathieu
> >
> > 1 -
> > ltt_relay_read for u8, u16, u32, u64 (not caring about singedness)
> >
> > 2 - depending on the sign, cast to (s8, s16, s32) and then cast to a
> > 64-bit signed value. Or if it was already 64-bits, no conversion is
> > needed.
> >
> > The end result (64-bit signed or unsigned value) should in the end be
> > casted to a u64.
> >
> > Mathieu
> >
> >
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
2009-01-17 4:37 ` Mathieu Desnoyers
@ 2009-01-19 2:27 ` Lai Jiangshan
2009-01-20 1:45 ` Mathieu Desnoyers
0 siblings, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2009-01-19 2:27 UTC (permalink / raw)
Mathieu Desnoyers wrote:
> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>> Mathieu Desnoyers wrote:
>>> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>>>> we need function read the base type from ltt-relay buffer
>>>> in the kernel for text output
>>>>
>>>> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
>>>> ---
>>>> ltt-serialize.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 71 insertions(+)
>>>> 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,77 @@
>>>> }
>>>> EXPORT_SYMBOL_GPL(ltt_serialize_data);
>>>>
>>>> +static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
>>> Maybe the name "unserialize" would be more appropriate ?
>>>
>>>> + char trace_size, enum ltt_type trace_type)
>>>> +{
>>>> + union {
>>>> + uint8_t u_8;
>>>> + uint16_t u_16;
>>>> + uint32_t u_32;
>>>> + uint64_t u_64;
>>>> + int8_t s_8;
>>>> + int16_t s_16;
>>>> + int32_t s_32;
>>>> + int64_t s_64;
>>>> + } tmp;
>>>> + uint64_t value;
>>>> +
>>>> + if (ltt_get_alignment())
>>> This if () is unneeded. It's already in the ltt_align logic.
>>>
>>>> + *ppos += ltt_align(*ppos, trace_size);
>>>> +
>>>> + switch (trace_type) {
>>>> + case LTT_TYPE_SIGNED_INT:
>>>> + switch (trace_size) {
>>>> + case 1:
>>>> + ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
>>>> + value = tmp.s_8;
>>> I think we could separate that in two parts to make the code smaller :
>> separate it into 2 functions?
>>
>
> Not necessarily 2 functions. Just separating it in two switch () statements
> would do the job.
>
I don't understand what you mean.
My code is the smallest, if we don't use macro.
> Mathieu
>
>>> 1 -
>>> ltt_relay_read for u8, u16, u32, u64 (not caring about singedness)
>>>
>>> 2 - depending on the sign, cast to (s8, s16, s32) and then cast to a
>>> 64-bit signed value. Or if it was already 64-bits, no conversion is
>>> needed.
>>>
>>> The end result (64-bit signed or unsigned value) should in the end be
>>> casted to a u64.
>>>
>>> Mathieu
>>>
>>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer
2009-01-19 2:27 ` Lai Jiangshan
@ 2009-01-20 1:45 ` Mathieu Desnoyers
0 siblings, 0 replies; 6+ messages in thread
From: Mathieu Desnoyers @ 2009-01-20 1:45 UTC (permalink / raw)
* 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 function read the base type from ltt-relay buffer
> >>>> in the kernel for text output
> >>>>
> >>>> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> >>>> ---
> >>>> ltt-serialize.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 71 insertions(+)
> >>>> 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,77 @@
> >>>> }
> >>>> EXPORT_SYMBOL_GPL(ltt_serialize_data);
> >>>>
> >>>> +static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
> >>> Maybe the name "unserialize" would be more appropriate ?
> >>>
> >>>> + char trace_size, enum ltt_type trace_type)
> >>>> +{
> >>>> + union {
> >>>> + uint8_t u_8;
> >>>> + uint16_t u_16;
> >>>> + uint32_t u_32;
> >>>> + uint64_t u_64;
> >>>> + int8_t s_8;
> >>>> + int16_t s_16;
> >>>> + int32_t s_32;
> >>>> + int64_t s_64;
> >>>> + } tmp;
> >>>> + uint64_t value;
> >>>> +
> >>>> + if (ltt_get_alignment())
> >>> This if () is unneeded. It's already in the ltt_align logic.
> >>>
> >>>> + *ppos += ltt_align(*ppos, trace_size);
> >>>> +
> >>>> + switch (trace_type) {
> >>>> + case LTT_TYPE_SIGNED_INT:
> >>>> + switch (trace_size) {
> >>>> + case 1:
> >>>> + ltt_relay_read(buf, *ppos, &tmp.s_8, trace_size);
> >>>> + value = tmp.s_8;
> >>> I think we could separate that in two parts to make the code smaller :
> >> separate it into 2 functions?
> >>
> >
> > Not necessarily 2 functions. Just separating it in two switch () statements
> > would do the job.
> >
>
> I don't understand what you mean.
> My code is the smallest, if we don't use macro.
>
> > Mathieu
> >
> >>> 1 -
> >>> ltt_relay_read for u8, u16, u32, u64 (not caring about singedness)
> >>>
> >>> 2 - depending on the sign, cast to (s8, s16, s32) and then cast to a
> >>> 64-bit signed value. Or if it was already 64-bits, no conversion is
> >>> needed.
> >>>
> >>> The end result (64-bit signed or unsigned value) should in the end be
> >>> casted to a u64.
> >>>
Here is a modified version. It may not be the smallest cost in terms of
LOC, but what I am trying to do here is to only have 4 "ltt_relay_read"
function calls, because all ltt_relay_read has to know is the size to
read, it does not care about types. Type management is made below in the
2nd switch() statement.
static inline u64 serialize_read_base_type(struct rchan_buf *buf, size_t *ppos,
char trace_size, enum ltt_type trace_type)
{
union {
u8 u_8;
u16 u_16;
u32 u_32;
u64 u_64;
} tmp;
uint64_t value;
if (ltt_get_alignment())
*ppos += ltt_align(*ppos, trace_size);
switch (trace_size) {
case 1:
ltt_relay_read(buf, *ppos, &tmp.u_8, trace_size);
break;
case 2:
ltt_relay_read(buf, *ppos, &tmp.u_16, trace_size);
break;
case 4:
ltt_relay_read(buf, *ppos, &tmp.u_32, trace_size);
break;
case 8:
ltt_relay_read(buf, *ppos, &tmp.u_64, trace_size);
break;
default:
BUG();
}
switch (trace_type) {
case LTT_TYPE_SIGNED_INT:
switch (trace_size) {
case 1:
value = (s64)(s8)tmp.u_8;
break;
case 2:
value = (s64)(s16)tmp.u_16;
break;
case 4:
value = (s64)(s32)tmp.u_32;
break;
case 8:
value = tmp.u_64;
break;
default:
BUG();
}
break;
case LTT_TYPE_UNSIGNED_INT:
switch (trace_size) {
case 1:
value = (u64)tmp.u_8;
break;
case 2:
value = (u64)tmp.u_16;
break;
case 4:
value = (u64)tmp.u_32;
break;
case 8:
value = tmp.u_64;
break;
default:
BUG();
}
break;
default:
BUG();
}
*ppos += trace_size;
return value;
}
/*
* Calculate data size
* Assume that the padding for alignment starts at a sizeof(void *) address.
Mathieu
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-01-20 1:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-13 9:09 [ltt-dev] [PATCH 05/13] read basic type from ltt-relay buffer Lai Jiangshan
2009-01-15 18:48 ` Mathieu Desnoyers
2009-01-17 3:24 ` Lai Jiangshan
2009-01-17 4:37 ` Mathieu Desnoyers
2009-01-19 2:27 ` Lai Jiangshan
2009-01-20 1:45 ` Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox