Re: [lttng-dev] [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
- On Feb 8, 2018, at 3:42 PM, Francis Deslauriers
francis.deslauri...@efficios.com wrote:
> 2018-02-07 15:50 GMT-05:00 Mathieu Desnoyers :
>>
>>
>> - On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
>> francis.deslauri...@efficios.com wrote:
>>
>> Is this really a fix ? Or is it a preparation step in order to be able to
>> remove events before the end of the session lifetime ?
>
> It's not a fix in the sense that it was not causing issues because it
> was only called at session destruction.
> But it's a fix in the sense that the _lttng_event_destroy function was
> not doing all the work necessary to destroy an event passed as
> argument. Taken by itself, a call to this function would leave one of
> the hlists of the hash table in a incorrect state by freeing an
> element without cds_hlist_del() it from the hlist before.
>
> That was my thought process. What do you think?
A fix is something I need to backport to make things work. I don't see
this as a fix based on that definition.
Thanks,
Mathieu
>
> Thank you,
> Francis
>
>> Thanks,
>>
>> Mathieu
>>
>>
>>> Signed-off-by: Francis Deslauriers
>>> ---
>>> liblttng-ust/lttng-events.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>>> index f4a7ccc..7419f78 100644
>>> --- a/liblttng-ust/lttng-events.c
>>> +++ b/liblttng-ust/lttng-events.c
>>> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
>>> {
>>> struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
>>>
>>> + /* Remove from event list. */
>>> cds_list_del(&event->node);
>>> + /* Remove from event hash table. */
>>> + cds_hlist_del(&event->hlist);
>>> +
>>> lttng_destroy_context(event->ctx);
>>> lttng_free_event_filter_runtime(event);
>>> /* Free event enabler refs */
>>> --
>>> 2.7.4
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> http://www.efficios.com
>
>
>
> --
> Francis Deslauriers
> Software developer
> EfficiOS inc.
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Re: [lttng-dev] [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
2018-02-07 15:50 GMT-05:00 Mathieu Desnoyers :
>
>
> - On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
> francis.deslauri...@efficios.com wrote:
>
> Is this really a fix ? Or is it a preparation step in order to be able to
> remove events before the end of the session lifetime ?
It's not a fix in the sense that it was not causing issues because it
was only called at session destruction.
But it's a fix in the sense that the _lttng_event_destroy function was
not doing all the work necessary to destroy an event passed as
argument. Taken by itself, a call to this function would leave one of
the hlists of the hash table in a incorrect state by freeing an
element without cds_hlist_del() it from the hlist before.
That was my thought process. What do you think?
Thank you,
Francis
> Thanks,
>
> Mathieu
>
>
>> Signed-off-by: Francis Deslauriers
>> ---
>> liblttng-ust/lttng-events.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>> index f4a7ccc..7419f78 100644
>> --- a/liblttng-ust/lttng-events.c
>> +++ b/liblttng-ust/lttng-events.c
>> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
>> {
>> struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
>>
>> + /* Remove from event list. */
>> cds_list_del(&event->node);
>> + /* Remove from event hash table. */
>> + cds_hlist_del(&event->hlist);
>> +
>> lttng_destroy_context(event->ctx);
>> lttng_free_event_filter_runtime(event);
>> /* Free event enabler refs */
>> --
>> 2.7.4
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
--
Francis Deslauriers
Software developer
EfficiOS inc.
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Re: [lttng-dev] [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
francis.deslauri...@efficios.com wrote:
Is this really a fix ? Or is it a preparation step in order to be able to
remove events before the end of the session lifetime ?
Thanks,
Mathieu
> Signed-off-by: Francis Deslauriers
> ---
> liblttng-ust/lttng-events.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index f4a7ccc..7419f78 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
> {
> struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
>
> + /* Remove from event list. */
> cds_list_del(&event->node);
> + /* Remove from event hash table. */
> + cds_hlist_del(&event->hlist);
> +
> lttng_destroy_context(event->ctx);
> lttng_free_event_filter_runtime(event);
> /* Free event enabler refs */
> --
> 2.7.4
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
[lttng-dev] [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
Signed-off-by: Francis Deslauriers
---
liblttng-ust/lttng-events.c | 4
1 file changed, 4 insertions(+)
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index f4a7ccc..7419f78 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
{
struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
+ /* Remove from event list. */
cds_list_del(&event->node);
+ /* Remove from event hash table. */
+ cds_hlist_del(&event->hlist);
+
lttng_destroy_context(event->ctx);
lttng_free_event_filter_runtime(event);
/* Free event enabler refs */
--
2.7.4
___
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
