[ltt-dev] [PATCH 02/13] get name and fmt from permanent eID

[compudj@krystal.dyndns.org (Mathieu Desnoyers)]

* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> 
> struct marker_entry is permanent, we introduce two APIs for
> getting name and fmt from eIDs.
> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
>  b/kernel/marker.c      |   34 ++++++++++++++++++++++++++++++++++
>  include/linux/marker.h |    3 +++
>  2 files changed, 37 insertions(+)
> diff --git a/include/linux/marker.h b/include/linux/marker.h
> index 32387e8..2fe8977 100644
> --- a/include/linux/marker.h
> +++ b/include/linux/marker.h
> @@ -238,6 +238,9 @@ extern int marker_probe_unregister_private_data(marker_probe_func *probe,
>  extern void *marker_get_private_data(const char *channel, const char *name,
>  	marker_probe_func *probe, int num);
>  
> +const char *marker_get_name_form_id(u16 channel_id, u16 event_id);
> +const char *marker_get_fmt_form_id(u16 channel_id, u16 event_id);
> +
>  /*
>   * marker_synchronize_unregister must be called between the last marker probe
>   * unregistration and the end of module exit to make sure there is no caller
> diff --git a/kernel/marker.c b/kernel/marker.c
> index b11a82a..6a918e2 100644
> --- a/kernel/marker.c
> +++ b/kernel/marker.c
> @@ -997,6 +1010,40 @@ void *marker_get_private_data(const char *channel, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(marker_get_private_data);
>  
> +static struct marker_entry *get_entry_from_id(u16 channel_id, u16 event_id)
> +{
> +	struct hlist_head *head;
> +	struct hlist_node *node;
> +	struct marker_entry *e, *found = NULL;
> +	u32 hash = hash_32((channel_id << 16) | event_id, MARKER_HASH_BITS);
> +

Can we add a define rather that hardcoding 16 in the c file ?

> +	mutex_lock(&markers_mutex);
> +	head = id_table + hash;
> +	hlist_for_each_entry(e, node, head, id_list) {
> +		if (e->channel_id == channel_id && e->event_id == event_id) {
> +			found = e;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&markers_mutex);
> +	return found;

Hrm, it would be good to add a comment to this function's header telling
for what validity period we can safely use the marker_entry * returned
because it is not protected by any mutex.

The idea is that the function is only called when tracing is active, and
we insure that the marker_entry is kept there (permanent) until the
trace ends. We are sure that we stopped using the marker entry before
trace end if we make sure we wait for all references to the trace to be
down to zero (that includes any bin-to-ascii trace output module which
uses this get_entry_from_id()) before we consider the trace unused. Not
only does that require a comment, but we should carefull check the code
to make sure refcounting is ok.


> +}
> +
> +/* must call when ids/marker_entry are kept alive */
> +const char *marker_get_name_form_id(u16 channel_id, u16 event_id)
> +{
> +	struct marker_entry *e = get_entry_from_id(channel_id, event_id);
> +	return e ? e->name : NULL;
> +}
> +EXPORT_SYMBOL_GPL(marker_get_name_form_id);
> +
Same here.

> +const char *marker_get_fmt_form_id(u16 channel_id, u16 event_id)
> +{
> +	struct marker_entry *e = get_entry_from_id(channel_id, event_id);
> +	return e ? e->format : NULL;
> +}
> +EXPORT_SYMBOL_GPL(marker_get_fmt_form_id);
> +

And here.

Hrm, actually, it might be good to expose the symbol :

get_entry_from_id

So we need only one hash table lookup to use both the name and fmt ?

Mathieu

>  /**
>   * markers_compact_event_ids: Compact markers event IDs and reassign channels
>   *
> 
> 
> 
> 
> _______________________________________________
> 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