* Re: [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined
[not found] <1394617237-30807-1-git-send-email-zifeitong@gmail.com>
@ 2014-03-13 13:49 ` Mathieu Desnoyers
[not found] ` <1204619065.2725.1394718552327.JavaMail.zimbra@efficios.com>
1 sibling, 0 replies; 3+ messages in thread
From: Mathieu Desnoyers @ 2014-03-13 13:49 UTC (permalink / raw)
To: Zifei Tong; +Cc: lttng-dev
----- Original Message -----
> From: "Zifei Tong" <zifeitong@gmail.com>
> To: "mathieu desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org, "Zifei Tong" <zifeitong@gmail.com>
> Sent: Wednesday, March 12, 2014 5:40:37 AM
> Subject: [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined
>
> Compiling tracepoint providers in clang will result in 'Wunused-function'
> warning, since tracepoint callbacks are defined but never called.
>
> lttng-gen-tp is also updated to not put TRACEPOINT_DEFINE in generated
> headers.
>
> Fixes #760
>
> Signed-off-by: Zifei Tong <zifeitong@gmail.com>
> ---
> doc/examples/gen-tp/sample.c | 1 +
> include/lttng/tracepoint.h | 13 ++++++++++---
> tools/lttng-gen-tp | 1 -
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/doc/examples/gen-tp/sample.c b/doc/examples/gen-tp/sample.c
> index dab6e7d..ae22f28 100644
> --- a/doc/examples/gen-tp/sample.c
> +++ b/doc/examples/gen-tp/sample.c
> @@ -23,6 +23,7 @@
>
> #include <unistd.h>
>
> +#define TRACEPOINT_DEFINE
Hrm, so this requires end users to change the way to use lttng-gen-tp.
I'm not sure I like that at all. This means changes in the way people do
instrumentation, and I'm trying really hard not to break that.
> #include "sample_tracepoint.h"
> int main(int argc, char **argv)
> {
> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
> index 66e2abd..462a0fc 100644
> --- a/include/lttng/tracepoint.h
> +++ b/include/lttng/tracepoint.h
> @@ -150,8 +150,8 @@ extern "C" {
> * between caller's ip addresses within the probe using the return
> * address.
> */
> -#define _DECLARE_TRACEPOINT(_provider, _name, ...) \
> -extern struct tracepoint __tracepoint_##_provider##___##_name; \
> +#if defined(TRACEPOINT_DEFINE)
So you are adding a ifdef conditional to skip creation of a macro that defines
static inline functions. What is it achieving really other than silencing a compiler
warning ? Anyway the compiler will optimize those static inline away if they are
not used.
I think this is a case where we might want to use __attribute__((unused)) to just
shut up the clang warning.
Thoughts ?
Thanks,
Mathieu
> +#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...) \
> static inline __attribute__((always_inline)) lttng_ust_notrace \
> void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__));
> \
> static \
> @@ -174,7 +174,14 @@ void
> __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)) \
> } while ((++__tp_probe)->func); \
> end: \
> tp_rcu_read_unlock_bp(); \
> -} \
> +}
> +#else
> +#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...)
> +#endif
> +
> +#define _DECLARE_TRACEPOINT(_provider, _name, ...) \
> +extern struct tracepoint __tracepoint_##_provider##___##_name; \
> +_DECLARE_TRACEPOINT_CB(_provider, _name, __VA_ARGS__) \
> static inline lttng_ust_notrace \
> void __tracepoint_register_##_provider##___##_name(char *name, \
> void (*func)(void), void *data); \
> diff --git a/tools/lttng-gen-tp b/tools/lttng-gen-tp
> index c49e8a5..ff8c22d 100755
> --- a/tools/lttng-gen-tp
> +++ b/tools/lttng-gen-tp
> @@ -69,7 +69,6 @@ class CFile:
> /*
> * The header containing our TRACEPOINT_EVENTs.
> */
> -#define TRACEPOINT_DEFINE
> #include "{headerFilename}"
> """
> def __init__(self, filename, template):
> --
> 1.9.0
>
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined
[not found] ` <1204619065.2725.1394718552327.JavaMail.zimbra@efficios.com>
@ 2014-03-14 13:11 ` Zifei Tong
0 siblings, 0 replies; 3+ messages in thread
From: Zifei Tong @ 2014-03-14 13:11 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: lttng-dev
On Thu, Mar 13, 2014 at 9:49 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> ----- Original Message -----
>> From: "Zifei Tong" <zifeitong@gmail.com>
>> To: "mathieu desnoyers" <mathieu.desnoyers@efficios.com>
>> Cc: lttng-dev@lists.lttng.org, "Zifei Tong" <zifeitong@gmail.com>
>> Sent: Wednesday, March 12, 2014 5:40:37 AM
>> Subject: [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined
>>
>> Compiling tracepoint providers in clang will result in 'Wunused-function'
>> warning, since tracepoint callbacks are defined but never called.
>>
>> lttng-gen-tp is also updated to not put TRACEPOINT_DEFINE in generated
>> headers.
>>
>> Fixes #760
>>
>> Signed-off-by: Zifei Tong <zifeitong@gmail.com>
>> ---
>> doc/examples/gen-tp/sample.c | 1 +
>> include/lttng/tracepoint.h | 13 ++++++++++---
>> tools/lttng-gen-tp | 1 -
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
>> index 66e2abd..462a0fc 100644
>> --- a/include/lttng/tracepoint.h
>> +++ b/include/lttng/tracepoint.h
>> @@ -150,8 +150,8 @@ extern "C" {
>> * between caller's ip addresses within the probe using the return
>> * address.
>> */
>> -#define _DECLARE_TRACEPOINT(_provider, _name, ...) \
>> -extern struct tracepoint __tracepoint_##_provider##___##_name; \
>> +#if defined(TRACEPOINT_DEFINE)
>
> So you are adding a ifdef conditional to skip creation of a macro that defines
> static inline functions. What is it achieving really other than silencing a compiler
> warning ? Anyway the compiler will optimize those static inline away if they are
> not used.
I just want to get rid of these warnings (I am little bit paranoid for
compiler warnings especially for clang's colorful ones :D). I think
these warnings result from compiling a simple 'hello-world' tracepoint
provider are annoying and confusing.
> I think this is a case where we might want to use __attribute__((unused)) to just
> shut up the clang warning.
Yes, with __attribute__((unused)), a one-line patch would be enough to
silence these warnings.
And I've tested with clang, __attribute__((unused)) works well.
>> diff --git a/doc/examples/gen-tp/sample.c b/doc/examples/gen-tp/sample.c
>> index dab6e7d..ae22f28 100644
>> --- a/doc/examples/gen-tp/sample.c
>> +++ b/doc/examples/gen-tp/sample.c
>> @@ -23,6 +23,7 @@
>>
>> #include <unistd.h>
>>
>> +#define TRACEPOINT_DEFINE
>
> Hrm, so this requires end users to change the way to use lttng-gen-tp.
> I'm not sure I like that at all. This means changes in the way people do
> instrumentation, and I'm trying really hard not to break that.
>
>> #include "sample_tracepoint.h"
>> int main(int argc, char **argv)
>> {
There is no need to change lttng-gen-tp anymore.
I'll send a new simple patch for your review.
Thank you!
Zifei Tong
> Thoughts ?
>
> Thanks,
>
> Mathieu
>
>> +#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...) \
>> static inline __attribute__((always_inline)) lttng_ust_notrace \
>> void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__));
>> \
>> static \
>> @@ -174,7 +174,14 @@ void
>> __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)) \
>> } while ((++__tp_probe)->func); \
>> end: \
>> tp_rcu_read_unlock_bp(); \
>> -} \
>> +}
>> +#else
>> +#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...)
>> +#endif
>> +
>> +#define _DECLARE_TRACEPOINT(_provider, _name, ...) \
>> +extern struct tracepoint __tracepoint_##_provider##___##_name; \
>> +_DECLARE_TRACEPOINT_CB(_provider, _name, __VA_ARGS__) \
>> static inline lttng_ust_notrace \
>> void __tracepoint_register_##_provider##___##_name(char *name, \
>> void (*func)(void), void *data); \
>> diff --git a/tools/lttng-gen-tp b/tools/lttng-gen-tp
>> index c49e8a5..ff8c22d 100755
>> --- a/tools/lttng-gen-tp
>> +++ b/tools/lttng-gen-tp
>> @@ -69,7 +69,6 @@ class CFile:
>> /*
>> * The header containing our TRACEPOINT_EVENTs.
>> */
>> -#define TRACEPOINT_DEFINE
>> #include "{headerFilename}"
>> """
>> def __init__(self, filename, template):
>> --
>> 1.9.0
>>
>>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined
@ 2014-03-12 9:40 Zifei Tong
0 siblings, 0 replies; 3+ messages in thread
From: Zifei Tong @ 2014-03-12 9:40 UTC (permalink / raw)
To: mathieu.desnoyers; +Cc: lttng-dev
Compiling tracepoint providers in clang will result in 'Wunused-function'
warning, since tracepoint callbacks are defined but never called.
lttng-gen-tp is also updated to not put TRACEPOINT_DEFINE in generated
headers.
Fixes #760
Signed-off-by: Zifei Tong <zifeitong@gmail.com>
---
doc/examples/gen-tp/sample.c | 1 +
include/lttng/tracepoint.h | 13 ++++++++++---
tools/lttng-gen-tp | 1 -
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/doc/examples/gen-tp/sample.c b/doc/examples/gen-tp/sample.c
index dab6e7d..ae22f28 100644
--- a/doc/examples/gen-tp/sample.c
+++ b/doc/examples/gen-tp/sample.c
@@ -23,6 +23,7 @@
#include <unistd.h>
+#define TRACEPOINT_DEFINE
#include "sample_tracepoint.h"
int main(int argc, char **argv)
{
diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 66e2abd..462a0fc 100644
--- a/include/lttng/tracepoint.h
+++ b/include/lttng/tracepoint.h
@@ -150,8 +150,8 @@ extern "C" {
* between caller's ip addresses within the probe using the return
* address.
*/
-#define _DECLARE_TRACEPOINT(_provider, _name, ...) \
-extern struct tracepoint __tracepoint_##_provider##___##_name; \
+#if defined(TRACEPOINT_DEFINE)
+#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...) \
static inline __attribute__((always_inline)) lttng_ust_notrace \
void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)); \
static \
@@ -174,7 +174,14 @@ void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)) \
} while ((++__tp_probe)->func); \
end: \
tp_rcu_read_unlock_bp(); \
-} \
+}
+#else
+#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...)
+#endif
+
+#define _DECLARE_TRACEPOINT(_provider, _name, ...) \
+extern struct tracepoint __tracepoint_##_provider##___##_name; \
+_DECLARE_TRACEPOINT_CB(_provider, _name, __VA_ARGS__) \
static inline lttng_ust_notrace \
void __tracepoint_register_##_provider##___##_name(char *name, \
void (*func)(void), void *data); \
diff --git a/tools/lttng-gen-tp b/tools/lttng-gen-tp
index c49e8a5..ff8c22d 100755
--- a/tools/lttng-gen-tp
+++ b/tools/lttng-gen-tp
@@ -69,7 +69,6 @@ class CFile:
/*
* The header containing our TRACEPOINT_EVENTs.
*/
-#define TRACEPOINT_DEFINE
#include "{headerFilename}"
"""
def __init__(self, filename, template):
--
1.9.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-03-14 13:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1394617237-30807-1-git-send-email-zifeitong@gmail.com>
2014-03-13 13:49 ` [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined Mathieu Desnoyers
[not found] ` <1204619065.2725.1394718552327.JavaMail.zimbra@efficios.com>
2014-03-14 13:11 ` Zifei Tong
2014-03-12 9:40 Zifei Tong
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.