All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers via lttng-dev <lttng-dev@lists.lttng.org>
To: Olivier Dion <olivier.dion@polymtl.ca>
Cc: lttng-dev <lttng-dev@lists.lttng.org>
Subject: Re: [PATCH lttng-ust] Add ctor/dtor priorities for tracepoints/events
Date: Sun, 12 Jul 2020 09:49:34 -0400 (EDT)	[thread overview]
Message-ID: <756801034.9565.1594561774215.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20200711152907.676582-1-olivier.dion@polymtl.ca>

----- On Jul 11, 2020, at 11:29 AM, lttng-dev lttng-dev@lists.lttng.org wrote:

> Some library might want to generate events in their ctor/dtor.  If
> LTTng initialize/finalize its tracepoints/events at the wrong time,
> events are lost.
> 
> Order of execution of the ctor/dtor is determined by priority.  When
> some priorities are equal, the order of execution seems to be
> determined by:
> 
>	   a) Order of appearance if in the same compilation unit
> 
>	   b) Order of link if in different compilation units
> 
>	   c) Order of load by ld-linux.so or dlopen(3) for
>	      share objects

I recall different rules about constructor priorities. Can you provide
links to documentation stating the priority order you describe above ?

Also, we should compare two approaches to fulfill your goal:
one alternative would be to have application/library constructors
explicitly call tracepoint constructors if they wish to use them.

Thanks,

Mathieu

> 
> Also, using the constructor/destructor attributes without any priority
> will default to the _lowest_ priority 65535, at least for GCC.
> 
> Thus, Providing the LTTNG_*_PRIO definitions allows users to set their
> ctor/dtor priority like so:
> ----------------------------------------------------------------------
> ...
> __attribute__((constructor(LTTNG_CTOR_PRIO + 1)))
> ...
> __attribute__((destructor(LTTNG_DTOR_PRIO + 1)))
> ...
> ----------------------------------------------------------------------
> or without any priority, that would also work.
> 
> Note that LTTNG_*_PRIO are set to 101 because it is the _highest_
> priority and 0 to 100 are reserved.
> 
> Signed-off-by: Olivier Dion <olivier.dion@polymtl.ca>
> ---
> include/lttng/tracepoint.h           | 8 ++++----
> include/lttng/ust-compiler.h         | 7 +++++++
> include/lttng/ust-tracepoint-event.h | 4 ++--
> 3 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
> index d77a2fb2..70903757 100644
> --- a/include/lttng/tracepoint.h
> +++ b/include/lttng/tracepoint.h
> @@ -318,7 +318,7 @@ __tracepoint__init_urcu_sym(void)
> }
> #endif
> 
> -static void lttng_ust_notrace __attribute__((constructor))
> +static void lttng_ust_notrace __attribute__((constructor(LTTNG_CTOR_PRIO)))
> __tracepoints__init(void);
> static void
> __tracepoints__init(void)
> @@ -340,7 +340,7 @@ __tracepoints__init(void)
> 	__tracepoint__init_urcu_sym();
> }
> 
> -static void lttng_ust_notrace __attribute__((destructor))
> +static void lttng_ust_notrace __attribute__((destructor(LTTNG_DTOR_PRIO)))
> __tracepoints__destroy(void);
> static void
> __tracepoints__destroy(void)
> @@ -444,7 +444,7 @@ extern struct lttng_ust_tracepoint * const
> __stop___tracepoints_ptrs[]
> 		__attribute__((used, section("__tracepoints_ptrs"))) =		\
> 			&__tracepoint_##_provider##___##_name;
> 
> -static void lttng_ust_notrace __attribute__((constructor))
> +static void lttng_ust_notrace __attribute__((constructor(LTTNG_CTOR_PRIO)))
> __tracepoints__ptrs_init(void);
> static void
> __tracepoints__ptrs_init(void)
> @@ -488,7 +488,7 @@ __tracepoints__ptrs_init(void)
> 	}
> }
> 
> -static void lttng_ust_notrace __attribute__((destructor))
> +static void lttng_ust_notrace __attribute__((destructor(LTTNG_DTOR_PRIO)))
> __tracepoints__ptrs_destroy(void);
> static void
> __tracepoints__ptrs_destroy(void)
> diff --git a/include/lttng/ust-compiler.h b/include/lttng/ust-compiler.h
> index 1d04da1a..4f1b1a40 100644
> --- a/include/lttng/ust-compiler.h
> +++ b/include/lttng/ust-compiler.h
> @@ -27,4 +27,11 @@
> #define lttng_ust_notrace __attribute__((no_instrument_function))
> #define LTTNG_PACKED	__attribute__((__packed__))
> 
> +/*
> + * Value in range [0, 100] are reserved.  Thus 101 is the higest
> + * priority that can be used.
> + */
> +#define LTTNG_CTOR_PRIO 101
> +#define LTTNG_DTOR_PRIO 101
> +
> #endif /* _LTTNG_UST_COMPILER_H */
> diff --git a/include/lttng/ust-tracepoint-event.h
> b/include/lttng/ust-tracepoint-event.h
> index 7890c247..cd45ae08 100644
> --- a/include/lttng/ust-tracepoint-event.h
> +++ b/include/lttng/ust-tracepoint-event.h
> @@ -1009,7 +1009,7 @@ static int
> _TP_COMBINE_TOKENS(__probe_register_refcount___, TRACEPOINT_PROVIDER)
> 
> /* Reset all macros within TRACEPOINT_EVENT */
> #include <lttng/ust-tracepoint-event-reset.h>
> -static void lttng_ust_notrace __attribute__((constructor))
> +static void lttng_ust_notrace __attribute__((constructor(LTTNG_CTOR_PRIO)))
> _TP_COMBINE_TOKENS(__lttng_events_init__, TRACEPOINT_PROVIDER)(void);
> static void
> _TP_COMBINE_TOKENS(__lttng_events_init__, TRACEPOINT_PROVIDER)(void)
> @@ -1036,7 +1036,7 @@ _TP_COMBINE_TOKENS(__lttng_events_init__,
> TRACEPOINT_PROVIDER)(void)
> 	}
> }
> 
> -static void lttng_ust_notrace __attribute__((destructor))
> +static void lttng_ust_notrace __attribute__((destructor(LTTNG_DTOR_PRIO)))
> _TP_COMBINE_TOKENS(__lttng_events_exit__, TRACEPOINT_PROVIDER)(void);
> static void
> _TP_COMBINE_TOKENS(__lttng_events_exit__, TRACEPOINT_PROVIDER)(void)
> --
> 2.27.0
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Desnoyers via lttng-dev <lttng-dev@lists.lttng.org>
To: Olivier Dion <olivier.dion@polymtl.ca>
Cc: lttng-dev <lttng-dev@lists.lttng.org>
Subject: Re: [lttng-dev] [PATCH lttng-ust] Add ctor/dtor priorities for tracepoints/events
Date: Sun, 12 Jul 2020 09:49:34 -0400 (EDT)	[thread overview]
Message-ID: <756801034.9565.1594561774215.JavaMail.zimbra@efficios.com> (raw)
Message-ID: <20200712134934.wY-fU0D2Yzb8fz98QO0-T9W0o3FrBPJilh8CdVPqLVQ@z> (raw)
In-Reply-To: <20200711152907.676582-1-olivier.dion@polymtl.ca>

----- On Jul 11, 2020, at 11:29 AM, lttng-dev lttng-dev@lists.lttng.org wrote:

> Some library might want to generate events in their ctor/dtor.  If
> LTTng initialize/finalize its tracepoints/events at the wrong time,
> events are lost.
> 
> Order of execution of the ctor/dtor is determined by priority.  When
> some priorities are equal, the order of execution seems to be
> determined by:
> 
>	   a) Order of appearance if in the same compilation unit
> 
>	   b) Order of link if in different compilation units
> 
>	   c) Order of load by ld-linux.so or dlopen(3) for
>	      share objects

I recall different rules about constructor priorities. Can you provide
links to documentation stating the priority order you describe above ?

Also, we should compare two approaches to fulfill your goal:
one alternative would be to have application/library constructors
explicitly call tracepoint constructors if they wish to use them.

Thanks,

Mathieu

> 
> Also, using the constructor/destructor attributes without any priority
> will default to the _lowest_ priority 65535, at least for GCC.
> 
> Thus, Providing the LTTNG_*_PRIO definitions allows users to set their
> ctor/dtor priority like so:
> ----------------------------------------------------------------------
> ...
> __attribute__((constructor(LTTNG_CTOR_PRIO + 1)))
> ...
> __attribute__((destructor(LTTNG_DTOR_PRIO + 1)))
> ...
> ----------------------------------------------------------------------
> or without any priority, that would also work.
> 
> Note that LTTNG_*_PRIO are set to 101 because it is the _highest_
> priority and 0 to 100 are reserved.
> 
> Signed-off-by: Olivier Dion <olivier.dion@polymtl.ca>
> ---
> include/lttng/tracepoint.h           | 8 ++++----
> include/lttng/ust-compiler.h         | 7 +++++++
> include/lttng/ust-tracepoint-event.h | 4 ++--
> 3 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
> index d77a2fb2..70903757 100644
> --- a/include/lttng/tracepoint.h
> +++ b/include/lttng/tracepoint.h
> @@ -318,7 +318,7 @@ __tracepoint__init_urcu_sym(void)
> }
> #endif
> 
> -static void lttng_ust_notrace __attribute__((constructor))
> +static void lttng_ust_notrace __attribute__((constructor(LTTNG_CTOR_PRIO)))
> __tracepoints__init(void);
> static void
> __tracepoints__init(void)
> @@ -340,7 +340,7 @@ __tracepoints__init(void)
> 	__tracepoint__init_urcu_sym();
> }
> 
> -static void lttng_ust_notrace __attribute__((destructor))
> +static void lttng_ust_notrace __attribute__((destructor(LTTNG_DTOR_PRIO)))
> __tracepoints__destroy(void);
> static void
> __tracepoints__destroy(void)
> @@ -444,7 +444,7 @@ extern struct lttng_ust_tracepoint * const
> __stop___tracepoints_ptrs[]
> 		__attribute__((used, section("__tracepoints_ptrs"))) =		\
> 			&__tracepoint_##_provider##___##_name;
> 
> -static void lttng_ust_notrace __attribute__((constructor))
> +static void lttng_ust_notrace __attribute__((constructor(LTTNG_CTOR_PRIO)))
> __tracepoints__ptrs_init(void);
> static void
> __tracepoints__ptrs_init(void)
> @@ -488,7 +488,7 @@ __tracepoints__ptrs_init(void)
> 	}
> }
> 
> -static void lttng_ust_notrace __attribute__((destructor))
> +static void lttng_ust_notrace __attribute__((destructor(LTTNG_DTOR_PRIO)))
> __tracepoints__ptrs_destroy(void);
> static void
> __tracepoints__ptrs_destroy(void)
> diff --git a/include/lttng/ust-compiler.h b/include/lttng/ust-compiler.h
> index 1d04da1a..4f1b1a40 100644
> --- a/include/lttng/ust-compiler.h
> +++ b/include/lttng/ust-compiler.h
> @@ -27,4 +27,11 @@
> #define lttng_ust_notrace __attribute__((no_instrument_function))
> #define LTTNG_PACKED	__attribute__((__packed__))
> 
> +/*
> + * Value in range [0, 100] are reserved.  Thus 101 is the higest
> + * priority that can be used.
> + */
> +#define LTTNG_CTOR_PRIO 101
> +#define LTTNG_DTOR_PRIO 101
> +
> #endif /* _LTTNG_UST_COMPILER_H */
> diff --git a/include/lttng/ust-tracepoint-event.h
> b/include/lttng/ust-tracepoint-event.h
> index 7890c247..cd45ae08 100644
> --- a/include/lttng/ust-tracepoint-event.h
> +++ b/include/lttng/ust-tracepoint-event.h
> @@ -1009,7 +1009,7 @@ static int
> _TP_COMBINE_TOKENS(__probe_register_refcount___, TRACEPOINT_PROVIDER)
> 
> /* Reset all macros within TRACEPOINT_EVENT */
> #include <lttng/ust-tracepoint-event-reset.h>
> -static void lttng_ust_notrace __attribute__((constructor))
> +static void lttng_ust_notrace __attribute__((constructor(LTTNG_CTOR_PRIO)))
> _TP_COMBINE_TOKENS(__lttng_events_init__, TRACEPOINT_PROVIDER)(void);
> static void
> _TP_COMBINE_TOKENS(__lttng_events_init__, TRACEPOINT_PROVIDER)(void)
> @@ -1036,7 +1036,7 @@ _TP_COMBINE_TOKENS(__lttng_events_init__,
> TRACEPOINT_PROVIDER)(void)
> 	}
> }
> 
> -static void lttng_ust_notrace __attribute__((destructor))
> +static void lttng_ust_notrace __attribute__((destructor(LTTNG_DTOR_PRIO)))
> _TP_COMBINE_TOKENS(__lttng_events_exit__, TRACEPOINT_PROVIDER)(void);
> static void
> _TP_COMBINE_TOKENS(__lttng_events_exit__, TRACEPOINT_PROVIDER)(void)
> --
> 2.27.0
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
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

  reply	other threads:[~2020-07-12 13:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-11 15:29 [PATCH lttng-ust] Add ctor/dtor priorities for tracepoints/events Olivier Dion via lttng-dev
2020-07-11 15:29 ` [lttng-dev] " Olivier Dion via lttng-dev
2020-07-12 13:49 ` Mathieu Desnoyers via lttng-dev [this message]
2020-07-12 13:49   ` Mathieu Desnoyers via lttng-dev
2020-07-12 15:49   ` Olivier Dion via lttng-dev
2020-07-12 15:49     ` [lttng-dev] " Olivier Dion via lttng-dev
2020-07-13 13:24     ` Mathieu Desnoyers via lttng-dev
2020-07-13 13:24       ` [lttng-dev] " Mathieu Desnoyers via lttng-dev
2020-07-13 15:19       ` Olivier Dion via lttng-dev
2020-07-13 15:19         ` [lttng-dev] " Olivier Dion via lttng-dev
2020-07-13 15:28         ` Mathieu Desnoyers via lttng-dev
2020-07-13 15:28           ` [lttng-dev] " Mathieu Desnoyers via lttng-dev
2020-07-13 18:46           ` Olivier Dion via lttng-dev
2020-07-13 18:46             ` [lttng-dev] " Olivier Dion via lttng-dev
2020-07-13 18:58             ` Mathieu Desnoyers via lttng-dev
2020-07-13 18:58               ` [lttng-dev] " Mathieu Desnoyers via lttng-dev
2020-07-13 19:44               ` Olivier Dion via lttng-dev
2020-07-13 19:44                 ` [lttng-dev] " Olivier Dion via lttng-dev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=756801034.9565.1594561774215.JavaMail.zimbra@efficios.com \
    --to=lttng-dev@lists.lttng.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=olivier.dion@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.