[ltt-dev] [PATCH take2 01/13] make marker_entry permanent
Mathieu Desnoyers
compudj at krystal.dyndns.org
Mon Feb 23 11:34:25 EST 2009
* 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 <laijs at cn.fujitsu.com>
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 <linux/mutex.h>
> #include <linux/types.h>
> #include <linux/jhash.h>
> +#include <linux/hash.h>
> #include <linux/list.h>
> #include <linux/rcupdate.h>
> #include <linux/marker.h>
> @@ -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
More information about the lttng-dev
mailing list