This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [RFC PATCH] Kernel Tracepoints (update)
- From: Masami Hiramatsu <mhiramat at redhat dot com>
- To: Mathieu Desnoyers <mathieu dot desnoyers at polymtl dot ca>
- Cc: KOSAKI Motohiro <kosaki dot motohiro at jp dot fujitsu dot com>, Takashi Nishiie <t-nishiie at np dot css dot fujitsu dot com>, "'Alexey Dobriyan'" <adobriyan at gmail dot com>, "'Peter Zijlstra'" <peterz at infradead dot org>, "'Steven Rostedt'" <rostedt at goodmis dot org>, "'Frank Ch. Eigler'" <fche at redhat dot com>, "'Ingo Molnar'" <mingo at elte dot hu>, "'LKML'" <linux-kernel at vger dot kernel dot org>, "'systemtap-ml'" <systemtap at sources dot redhat dot com>, "'Hideo AOKI'" <haoki at redhat dot com>
- Date: Thu, 03 Jul 2008 14:46:37 -0400
- Subject: Re: [RFC PATCH] Kernel Tracepoints (update)
- References: <007601c8d5ca$18fa0e10$4aee2a30$@css.fujitsu.com> <48611B03.1000003@redhat.com> <20080625011951.D83E.KOSAKI.MOTOHIRO@jp.fujitsu.com> <48612879.5090809@redhat.com> <20080625235214.GA14249@Krystal> <486403F0.4020801@redhat.com> <20080627133622.GA15210@Krystal> <486CEFDB.7070508@redhat.com> <20080703181839.GA10672@Krystal>
Hi Mathieu,
Mathieu Desnoyers wrote:
> Hi Masami,
>
> I am thinking about removing the private_data, because such declaration
> won't work :
>
> DEFINE_TRACE(kernel_irq_exit,
> TPPROTO(void),
> TPARGS());
>
> because the arguments become :
>
> (void *private_data, void)
Oh, you right.
since there is no tracepoint user who uses private_data,
we can remove it.
> Any better idea ?
If you need it, I think you can introduce some macros like below.
#define TPPROTO(args...) (void *private_data, args)
#define __define_trace_register(name, proto) \ /*shared by DEFINE_TRACE/DEFINE_TRACE_NOARG*/
static inline int register_trace_##name(\
void (*probe)proto, \
void *private_data) \
{ \
return tracepoint_probe_register(#name ":" #proto, \
(void *)probe, private_data); \
} \
static inline void unregister_trace_##name( \
void (*probe)proto, \
void *private_data) \
{ \
tracepoint_probe_unregister(#name ":" #proto, \
(void *)probe, private_data); \
}
#define DEFINE_TRACE_NOARG(name) \
static inline void _do_trace_##name(struct tracepoint *tp) \
...
static inline void trace_##name(void) \
...
__define_trace_register(name, (void *private_data))
But I think removing private_data is better...
Thank you,
>
> Mathieu
>
> * Masami Hiramatsu (mhiramat@redhat.com) wrote:
>> Hi Mathieu,
>>
>> I found a bug.
>> Mathieu Desnoyers wrote:
>> [...]
>>
>>> +/*
>>> + * Remove the tracepoint from the tracepoint hash table. Must be called with
>>> + * mutex_lock held.
>>> + */
>>> +static int remove_tracepoint(const char *name)
>>> +{
>>> + struct hlist_head *head;
>>> + struct hlist_node *node;
>>> + struct tracepoint_entry *e;
>>> + int found = 0;
>>> + size_t len = strlen(name) + 1;
>>> + u32 hash = jhash(name, len-1, 0);
>>> +
>>> + head = &tracepoint_table[hash & ((1 << TRACEPOINT_HASH_BITS)-1)];
>>> + hlist_for_each_entry(e, node, head, hlist) {
>>> + if (!strcmp(name, e->name)) {
>>> + found = 1;
>>> + break;
>>> + }
>>> + }
>>> + if (!found)
>>> + return -ENOENT;
>> before removing, you have to ensure refcount == 0. So,
>> if (e->refcount != 0)
>> return -EBUSY;
>>
>>> + hlist_del(&e->hlist);
>>> + /* Make sure the call_rcu has been executed */
>>> + if (e->rcu_pending)
>>> + rcu_barrier();
>>> + kfree(e);
>>> + return 0;
>>> +}
>> Thank you,
>>
>> --
>> Masami Hiramatsu
>>
>> Software Engineer
>> Hitachi Computer Products (America) Inc.
>> Software Solutions Division
>>
>> e-mail: mhiramat@redhat.com
>>
>
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com