From mboxrd@z Thu Jan 1 00:00:00 1970 From: compudj@krystal.dyndns.org (Mathieu Desnoyers) Date: Mon, 23 Feb 2009 11:34:25 -0500 Subject: [ltt-dev] [PATCH take2 01/13] make marker_entry permanent In-Reply-To: <498A5D79.8010701@cn.fujitsu.com> References: <498A5D79.8010701@cn.fujitsu.com> Message-ID: <20090223163425.GA31815@Krystal> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote: > text output need a permanent struct for permanent eID > we make marker_entry permanent > > Signed-off-by: Lai Jiangshan Hi Lai, This patch looks good, except for a small detail below, > --- > diff --git a/include/linux/ltt-channels.h b/include/linux/ltt-channels.h > index 4a3924b..db513cb 100644 > --- a/include/linux/ltt-channels.h > +++ b/include/linux/ltt-channels.h > @@ -67,6 +77,7 @@ int ltt_channels_set_default(const char *name, > unsigned int subbuf_cnt); > const char *ltt_channels_get_name_from_index(unsigned int index); > int ltt_channels_get_index_from_name(const char *name); > +int ltt_channel_alive(const char *channel); > struct ltt_channel_struct *ltt_channels_trace_alloc(unsigned int *nr_channels, > int overwrite, > int active); > diff --git a/kernel/marker.c b/kernel/marker.c > index b11a82a..6a918e2 100644 > --- a/kernel/marker.c > +++ b/kernel/marker.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -59,6 +60,7 @@ void unlock_markers(void) > #define MARKER_HASH_BITS 6 > #define MARKER_TABLE_SIZE (1 << MARKER_HASH_BITS) > static struct hlist_head marker_table[MARKER_TABLE_SIZE]; > +static struct hlist_head id_table[MARKER_TABLE_SIZE]; > > /* > * Note about RCU : > @@ -70,6 +72,7 @@ static struct hlist_head marker_table[MARKER_TABLE_SIZE]; > */ > struct marker_entry { > struct hlist_node hlist; > + struct hlist_node id_list; > char *format; > char *name; > /* Probe wrapper */ > @@ -465,7 +468,7 @@ static struct marker_entry *add_marker(const char *channel, const char *name, > * Remove the marker from the marker hash table. Must be called with mutex_lock > * held. > */ > -static int remove_marker(const char *channel, const char *name) > +static int remove_marker(const char *channel, const char *name, int registered) > { > struct hlist_head *head; > struct hlist_node *node; > @@ -488,11 +491,18 @@ static int remove_marker(const char *channel, const char *name) > return -ENOENT; > if (e->single.func != __mark_empty_function) > return -EBUSY; > + > + if (registered && ltt_channel_alive(channel)) > + return 0; > + > hlist_del(&e->hlist); > + hlist_del(&e->id_list); > + if (registered) { > + ret = ltt_channels_unregister(e->channel); > + WARN_ON(ret); > + } > if (e->format_allocated) > kfree(e->format); > - ret = ltt_channels_unregister(e->channel); > - WARN_ON(ret); > /* Make sure the call_rcu has been executed */ > if (e->rcu_pending) > rcu_barrier_sched(); > @@ -749,6 +759,9 @@ int marker_probe_register(const char *channel, const char *name, > if (ret < 0) > goto error_unregister_channel; > entry->event_id = ret; > + hlist_add_head(&entry->id_list, id_table + hash_32( > + (entry->channel_id << 16) | entry->event_id, > + MARKER_HASH_BITS)); > ret = 0; > trace_mark(metadata, core_marker_id, > "channel %s name %s event_id %hu " > @@ -801,7 +814,7 @@ error_unregister_channel: > ret_err = ltt_channels_unregister(channel); > WARN_ON(ret_err); > error_remove_marker: > - ret_err = remove_marker(channel, name); > + ret_err = remove_marker(channel, name, 0); > WARN_ON(ret_err); > end: > mutex_unlock(&markers_mutex); > @@ -851,7 +864,7 @@ int marker_probe_unregister(const char *channel, const char *name, > /* write rcu_pending before calling the RCU callback */ > smp_wmb(); > call_rcu_sched(&entry->rcu, free_old_closure); > - remove_marker(channel, name); /* Ignore busy error message */ > + remove_marker(channel, name, 1); /* Ignore busy error message */ > ret = 0; > end: > mutex_unlock(&markers_mutex); > @@ -938,7 +951,7 @@ int marker_probe_unregister_private_data(marker_probe_func *probe, > smp_wmb(); > call_rcu_sched(&entry->rcu, free_old_closure); > /* Ignore busy error message */ > - remove_marker(channel, name); > + remove_marker(channel, name, 1); > end: > mutex_unlock(&markers_mutex); > kfree(channel); > @@ -1008,12 +1055,16 @@ void markers_compact_event_ids(void) > struct marker_entry *entry; > unsigned int i; > struct hlist_head *head; > - struct hlist_node *node; > + struct hlist_node *node, *next; > int ret; > > for (i = 0; i < MARKER_TABLE_SIZE; i++) { > head = &marker_table[i]; > - hlist_for_each_entry(entry, node, head, hlist) { > + hlist_for_each_entry_safe(entry, node, next, head, hlist) { > + if (!entry->refcount) { > + remove_marker(entry->channel, entry->name, 1); > + continue; > + } > ret = ltt_channels_get_index_from_name(entry->channel); > WARN_ON(ret < 0); > entry->channel_id = ret; > @@ -1023,6 +1074,16 @@ void markers_compact_event_ids(void) > entry->event_id = ret; > } > } > + > + memset(id_table, 0, sizeof(id_table)); > + for (i = 0; i < MARKER_TABLE_SIZE; i++) { > + head = &marker_table[i]; > + hlist_for_each_entry(entry, node, head, hlist) { > + hlist_add_head(&entry->id_list, id_table + hash_32( > + (entry->channel_id << 16) > + | entry->event_id, MARKER_HASH_BITS)); > + } > + } > } > > #ifdef CONFIG_MODULES > diff --git a/ltt/ltt-channels.c b/ltt/ltt-channels.c > index e01b8dd..0badd9d 100644 > --- a/ltt/ltt-channels.c > +++ b/ltt/ltt-channels.c > @@ -40,6 +40,15 @@ static struct ltt_channel_setting *lookup_channel(const char *name) > return NULL; > } > > +int ltt_channel_alive(const char *channel) > +{ > + int ret; > + mutex_lock(<t_channel_mutex); > + ret = !!atomic_read(&index_kref.refcount); > + mutex_unlock(<t_channel_mutex); Why are you taking the ltt_channel_mutex to read a variable atomically ? There is probably something you intend to protect (race between stopping to use a channel and removing a marker), but I am pretty much sure this locking does not do what you intend. Mathieu > + return ret; > +} > + > /* > * Must be called when channel refcount falls to 0 _and_ also when the last > * trace is freed. This function is responsible for compacting the channel and > > > > _______________________________________________ > 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