Re: [lttng-dev] [PATCH lttng-ust 4/8] Add probe provider unregister function
- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
francis.deslauri...@efficios.com wrote:
> Signed-off-by: Francis Deslauriers
> ---
> include/lttng/ust-events.h | 1 +
> liblttng-ust/lttng-events.c | 89 +
> 2 files changed, 90 insertions(+)
>
> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
> index caf7e63..019b0eb 100644
> --- a/include/lttng/ust-events.h
> +++ b/include/lttng/ust-events.h
> @@ -656,6 +656,7 @@ void synchronize_trace(void);
>
> int lttng_probe_register(struct lttng_probe_desc *desc);
> void lttng_probe_unregister(struct lttng_probe_desc *desc);
> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
> int lttng_fix_pending_events(void);
> int lttng_probes_init(void);
> void lttng_probes_exit(void);
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index 7419f78..e8d4857 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -789,6 +789,95 @@ void lttng_create_event_if_missing(struct lttng_enabler
> *enabler)
> }
>
> /*
> + * Iterate over all the UST sessions to unregister and destroy all probes
> from
> + * the probe provider descriptor passed in arguments. Must me called with the
passed in arguments -> received as argument
> + * ust_lock held.
> + */
> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc
> *provider_desc)
> +{
> + int i;
move int i; to the last variable (shortest line).
> + struct cds_list_head *sessionsp;
> + struct lttng_session *session;
> + struct cds_hlist_head *head;
> + struct cds_hlist_node *node;
> + struct lttng_event *event;
> +
> + /* Get handle on list of sessions. */
> + sessionsp = _lttng_get_sessions();
> +
> + /*
> + * Iterate over all events in the probe provider descriptions and
> sessions
> + * to queue the unregistration of the events.
> + */
> + for (i = 0; i < provider_desc->nr_events; i++) {
> + const char *event_name;
> + uint32_t hash;
> + size_t name_len;
> + const struct lttng_event_desc *event_desc;
same here about var definition layout.
> +
> + event_desc = provider_desc->event_desc[i];
> + event_name = event_desc->name;
> + name_len = strlen(event_name);
> + hash = jhash(event_name, name_len, 0);
> +
> + /* Iterate over all session to find the current event
> description. */
> + cds_list_for_each_entry(session, sessionsp, node) {
remove whiteline.
> +
> + /*
> + * Get the list of events in the hashtable bucket and
> iterate to
> + * find the event matching this descriptor.
> + */
> + head = &session->events_ht.table[hash &
> (LTTNG_UST_EVENT_HT_SIZE - 1)];
> + cds_hlist_for_each_entry(event, node, head, hlist) {
> + if (event_desc == event->desc) {
> +
> + /* Queue the unregistration of this
> event. */
> + _lttng_event_unregister(event);
> + break;
> + }
> + }
> + }
> + }
> +
> + /* Wait for grace period. */
> + synchronize_trace();
> + /* Prune the unregistration queue. */
> + __tracepoint_probe_prune_release_queue();
> +
> + /*
> + * It is now safe to destroy the events and remove them from the event
> list
> + * and hashtables.
> + */
> + for (i = 0; i < provider_desc->nr_events; i++) {
> + const char *event_name;
> + uint32_t hash;
> + size_t name_len;
> + const struct lttng_event_desc *event_desc;
same.
> +
> + event_desc = provider_desc->event_desc[i];
> + event_name = event_desc->name;
> + name_len = strlen(event_name);
> + hash = jhash(event_name, name_len, 0);
> +
> + /* Iterate over all sessions to find the current event
> description. */
> + cds_list_for_each_entry(session, sessionsp, node) {
whiteline.
> +
> + /*
> + * Get the list of events in the hashtable bucket and
> iterate to
> + * find the event matching this descriptor.
> + */
beware for the iteration below: you may need to use a
cds_list_for_each_entry_safe()
if you can remove the list entry while you iterate.
> + head = &session->events_ht.table[hash &
> (LTTNG_UST_EVENT_HT_SIZE - 1)];
> + cds_hlist_for_each_entry(event, node, head, hlist) {
> + if (event_desc == event->desc) {
> + _lttng_event_destroy(event);
you have a "break" here. Is there
Re: [lttng-dev] [PATCH lttng-ust 4/8] Add probe provider unregister function
2018-02-07 15:54 GMT-05:00 Mathieu Desnoyers :
>
> - On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
> francis.deslauri...@efficios.com wrote:
>
>> Signed-off-by: Francis Deslauriers
>> ---
>> include/lttng/ust-events.h | 1 +
>> liblttng-ust/lttng-events.c | 89
>> +
>> 2 files changed, 90 insertions(+)
>>
>> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
>> index caf7e63..019b0eb 100644
>> --- a/include/lttng/ust-events.h
>> +++ b/include/lttng/ust-events.h
>> @@ -656,6 +656,7 @@ void synchronize_trace(void);
>>
>> int lttng_probe_register(struct lttng_probe_desc *desc);
>> void lttng_probe_unregister(struct lttng_probe_desc *desc);
>> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
>> int lttng_fix_pending_events(void);
>> int lttng_probes_init(void);
>> void lttng_probes_exit(void);
>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>> index 7419f78..e8d4857 100644
>> --- a/liblttng-ust/lttng-events.c
>> +++ b/liblttng-ust/lttng-events.c
>> @@ -789,6 +789,95 @@ void lttng_create_event_if_missing(struct lttng_enabler
>> *enabler)
>> }
>>
>> /*
>> + * Iterate over all the UST sessions to unregister and destroy all probes
>> from
>> + * the probe provider descriptor passed in arguments. Must me called with
>> the
>
> passed in arguments -> received as argument
>
>> + * ust_lock held.
>> + */
>> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc
>> *provider_desc)
>> +{
>> + int i;
>
> move int i; to the last variable (shortest line).
>
>> + struct cds_list_head *sessionsp;
>> + struct lttng_session *session;
>> + struct cds_hlist_head *head;
>> + struct cds_hlist_node *node;
>> + struct lttng_event *event;
>> +
>> + /* Get handle on list of sessions. */
>> + sessionsp = _lttng_get_sessions();
>> +
>> + /*
>> + * Iterate over all events in the probe provider descriptions and
>> sessions
>> + * to queue the unregistration of the events.
>> + */
>> + for (i = 0; i < provider_desc->nr_events; i++) {
>> + const char *event_name;
>> + uint32_t hash;
>> + size_t name_len;
>> + const struct lttng_event_desc *event_desc;
>
> same here about var definition layout.
>
>> +
>> + event_desc = provider_desc->event_desc[i];
>> + event_name = event_desc->name;
>> + name_len = strlen(event_name);
>> + hash = jhash(event_name, name_len, 0);
>> +
>> + /* Iterate over all session to find the current event
>> description. */
>> + cds_list_for_each_entry(session, sessionsp, node) {
>
> remove whiteline.
>
>> +
>> + /*
>> + * Get the list of events in the hashtable bucket and
>> iterate to
>> + * find the event matching this descriptor.
>> + */
>> + head = &session->events_ht.table[hash &
>> (LTTNG_UST_EVENT_HT_SIZE - 1)];
>> + cds_hlist_for_each_entry(event, node, head, hlist) {
>> + if (event_desc == event->desc) {
>> +
>> + /* Queue the unregistration of this
>> event. */
>> + _lttng_event_unregister(event);
>> + break;
>> + }
>> + }
>> + }
>> + }
>> +
>> + /* Wait for grace period. */
>> + synchronize_trace();
>> + /* Prune the unregistration queue. */
>> + __tracepoint_probe_prune_release_queue();
>> +
>> + /*
>> + * It is now safe to destroy the events and remove them from the event
>> list
>> + * and hashtables.
>> + */
>> + for (i = 0; i < provider_desc->nr_events; i++) {
>> + const char *event_name;
>> + uint32_t hash;
>> + size_t name_len;
>> + const struct lttng_event_desc *event_desc;
>
> same.
>
>> +
>> + event_desc = provider_desc->event_desc[i];
>> + event_name = event_desc->name;
>> + name_len = strlen(event_name);
>> + hash = jhash(event_name, name_len, 0);
>> +
>
>
>
>> + /* Iterate over all sessions to find the current event
>> description. */
>> + cds_list_for_each_entry(session, sessionsp, node) {
>
> whiteline.
>
>> +
>> + /*
>> + * Get the list of events in the hashtable bucket and
>> iterate to
>> + * find the event matching this descriptor.
>> + */
>
> beware for the iteration below: you may need to use a
> cds_list_for_each_entry_safe()
> if you can remove the list entry while you iterate.
Good point. Fixed.
>
>> + head = &session->events_ht.table[hash &
>> (LTTNG_UST_EVENT_HT_SIZE - 1)];
>> +
