Date:	Fri, 17 Apr 2009 12:16:01 +0200
From:	Frédéric Weisbecker <fweisbec@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>, Zhaolei <zhaolei@...fujitsu.com>,
	Tom Zanussi <tzanussi@...il.com>,
	Li Zefan <lizf@...fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2 v2] tracing/events: provide string with undefined size 
	support

2009/4/17 Steven Rostedt <rostedt@...dmis.org>:
>
> [ /me can't sleep, trying to get tired again ]
>
>
> On Fri, 17 Apr 2009, Peter Zijlstra wrote:
>
>> On Fri, 2009-04-17 at 10:59 +0200, Fr?d?ric Weisbecker wrote:
>>
>> > struct foo {
>> >    field1;
>> >    field2;
>> >    ...
>> > };
>> >
>> > struct foo *f;
>> >
>> > event = ring_buffer_lock_reserve(sizeof(*f), ...);
>> > f = ring_buffer_event_data(event);
>> >
>> > I can't use a kmalloc here. We are tracing a random event which can happen
>> > at a random frequency, random context, etc...
>>
>> Can't you add a bit to that ring_buffer_lock_reserve() thing?
>>
>> The thing I do for perf_counters is I first iterate all the output, then
>> make the reserve large enough to fit all the output in, then copy the
>> bits into the output buffer.
>>
>> The result is that the output cannot be interpreted as a fixed offset
>> struct, but that's not much of an issue anyway.
>>
>> Another possibility is using relative pointers for strings that point
>> beyond the tail of the fixed offset struct.
>>
>> So something like:
>>
>>   __field(int, foo);
>>   __string(bar);
>>   __field(int, foo2);
>>   __string(bar2);
>>   __field(int, foo3);
>>
>> would look like:
>>
>>  struct plop {
>>    int foo;
>>    char *bar;
>>    int foo2;
>>    char *bar2;
>>    int foo3;
>>
>>    char data[0];
>>  }
>>
>> and you'd do something like:
>>
>>  size = sizeof(struct plop);
>>  size += strlen(bar) + 1;
>>  size += strlen(bar2) + 1;
>>
>>  event = ring_buffer_lock_reserve(size);
>>  offset = sizeof(struct plop);
>>  my_plop.bar = (char *)offset;
>>  offset += strlen(bar) + 1;
>>  my_plop.bar2 = (char *)offset;
>>  memcpy(&event, &my_plop, sizeof(struct plop));
>>  memcpy(&event + my_plop.bar, bar, strlen(bar)+1);
>>  memcpy(&event + my_plop.bar2, bar2, strlen(bar2)+1);
>>  ring_buffer_unlock();
>>
>> Then on reading, you'd get a variable sized entry, with a fixed size
>> fixed offset struct, that contains relative offset character pointers.
>
> When I replied to Frederic, I thought I could come up with a way to do
> something like you are proposing. Instead, I only ended up with the
> variant that Frederic implemented.
>
> I've done what you are suggesting several times in tracing. Logdev does
> this in its tracing.
>
> The problem that we have, is that we don't have actual code. We have a
> TRACE_EVENT macro that is doing the work for us. This, unfortunately,
> limits what we can do.
>
> One response is to just free code it. Well, then all users of TRACE_EVENT
> would end up doing a lot more work too.
>
> The more power we put into TRACE_EVENT, the uglier and more complex it
> ends up.
>
> But that said, one could still do what you are suggesting. The "__ending"
> wart still needs to be there, since it must happen at the end of the
> struct.
>
>
>        TP_STRUCT__entry(
>                __field(int, str2loc)
>                __field(int, str3loc)
>                __string(str1)
>                __string(str2)
>                __ending_string(data, str3)
>        ),


Or something like:

 TP_STRUCT__ending_string(
)

that would do the __ending_string itself...



>        TP_fast_assign(
>                __entry->str2loc = strlen(str1);
>                __entry->str3loc = strlen(str2) + __entry->str2loc;
>                strcpy(__entry->data, str1);
>                strcpy(__entry->data + __entry->str2loc, str2)
>                strcpy(__entry->data + __entry->str3loc, str3)
>        ),
>        TP_printk("str1:%s str2:%s str3:%s\n",
>                __entry->data,
>                __entry->data + __entry->str2loc,
>                __entry->date + __entry->str3loc)
>
>
> For this to work, we need to just add a define for the __string macro for
> the reservation:
>
> #define __string(x)  __str_size_ += strlen(x) + 1;
>
> -- Steve
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists