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)]; >> +