diff mbox series

compaction: make tracepoints condtional on CONFIG_COMPACTION

Message ID 20220803192327.3726094-1-bruce.ashfield@gmail.com
State New
Headers show
Series compaction: make tracepoints condtional on CONFIG_COMPACTION | expand

Commit Message

Bruce Ashfield Aug. 3, 2022, 7:23 p.m. UTC 
From: Bruce Ashfield <bruce.ashfield at gmail.com>

When building against a v5.19 mips kernel, the following errors
were triggered:

  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:105:42: note: expected 'struct compact_control *' but argument is of type 'struct compact_control *'
  |   105 |         TP_PROTO(struct compact_control *cc,
  |       |                  ~~~~~~~~~~~~~~~~~~~~~~~~^~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:779:68: note: in definition of macro 'LTTNG_TRACEPOINT_EVENT_CLASS_CODE'
  |   779 | static inline size_t __event_get_align__##_name(void *__tp_locvar, _proto)    \
  |       |                                                                    ^~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:106:50: note: in expansion of macro 'PARAMS'
  |   106 |         LTTNG_TRACEPOINT_EVENT_CLASS_CODE(_name, PARAMS(_proto), PARAMS(_args), , , \
  |       |                                                  ^~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:41:9: note: in expansion of macro 'LTTNG_TRACEPOINT_EVENT_CLASS'
  |    41 |         LTTNG_TRACEPOINT_EVENT_CLASS(map,                               \
  |       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:42:30: note: in expansion of macro 'PARAMS'
  |    42 |                              PARAMS(proto),                             \
  |       |                              ^~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:101:1: note: in expansion of macro 'LTTNG_TRACEPOINT_EVENT_MAP'
  |   101 | LTTNG_TRACEPOINT_EVENT_MAP(mm_compaction_migratepages,
  |       | ^~~~~~~~~~~~~~~~~~~~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:105:9: note: in expansion of macro 'TP_PROTO'
  |   105 |         TP_PROTO(struct compact_control *cc,
  |       |         ^~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:112:57: error: invalid use of undefined type 'struct compact_control'

...

  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:108:25: warning: 'struct compact_control' declared inside parameter list will not be visible outside of this definition or declaration
  |   108 |         TP_PROTO(struct compact_control *cc,
  |       |                         ^~~~~~~~~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:159:43: note: in definition of macro 'LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP'
  |   159 | void __event_template_proto___##_template(_proto);
  |       |                                           ^~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:45:61: note: in expansion of macro 'PARAMS'
  |    45 |         LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP(map, name, map, PARAMS(proto), PARAMS(args))
  |       |                                                             ^~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:104:1: note: in expansion of macro 'LTTNG_TRACEPOINT_EVENT_MAP'
  |   104 | LTTNG_TRACEPOINT_EVENT_MAP(mm_compaction_migratepages,
  |       | ^~~~~~~~~~~~~~~~~~~~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/instrumentation/events/compaction.h:108:9: note: in expansion of macro 'TP_PROTO'
  |   108 |         TP_PROTO(struct compact_control *cc,
  |       |         ^~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:159:6: error: conflicting types for '__event_template_proto___compaction_migratepages'; have 'void(struct compact_control *, unsigned int)'
  |   159 | void __event_template_proto___##_template(_proto);
  |       |      ^~~~~~~~~~~~~~~~~~~~~~~~~
  | lttng-modules-2.13.4/src/probes/../../include/lttng/tracepoint-event-impl.h:45:9: note: in expansion of macro 'LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP'
  |    45 |         LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP(map, name, map, PARAMS(proto), PARAMS(args))
  |       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The root cause appears to be that this particular mips kernel does not
have CONFIG_COMPACTION avaiable, and hence we end up with the struct
declaration within this tracepoint, and then conflicting types.

While putting the conditional around only compaction_migratepages seemed
sufficient to get the build working, it doesn't look like any of the
tracepoints should be valid, so we extend it to all definitions.

Signed-off-by: Bruce Ashfield <bruce.ashfield at gmail.com>
---

Hi all,

This was found during the update of the yocto project reference
kernel to 5.19. Out of all the supported architectures, only
mips failed during the build of lttng-modules.

Specifically, it seems like the lack of CONFIG_COMPACTION being
enabled in the kernel was the significant issue. Only one of the
tracepoints blew up, but I can't see how any of them would be
valid in this scenario, or why it would be limited to mips.

While I don't think this is the right fix, this is what got me
up and passing sanity testing, so I wanted to send the change
and see what the actual, correct fix would be :)

Cheers,

Bruce


 include/instrumentation/events/compaction.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Michael Jeanson Aug. 10, 2022, 3:20 p.m. UTC | #1 
On 2022-08-03 15 h 23, Bruce Ashfield via lttng-dev wrote:
> Hi all,
> 
> This was found during the update of the yocto project reference
> kernel to 5.19. Out of all the supported architectures, only
> mips failed during the build of lttng-modules.
> 
> Specifically, it seems like the lack of CONFIG_COMPACTION being
> enabled in the kernel was the significant issue. Only one of the
> tracepoints blew up, but I can't see how any of them would be
> valid in this scenario, or why it would be limited to mips.
> 
> While I don't think this is the right fix, this is what got me
> up and passing sanity testing, so I wanted to send the change
> and see what the actual, correct fix would be :)
> 
> Cheers,
> 
> Bruce

Hi Bruce,

Thanks for reporting this issue, I propose a simpler fix [1] of just 
disabling the build of the compaction probe module when 
CONFIG_COMPACTION is disabled in the kernel configuration.

Can you confirm this patch fixes the issue on your side?

Thanks,

Michael

[1] https://review.lttng.org/c/lttng-modules/+/8660
Bruce Ashfield Aug. 10, 2022, 5:48 p.m. UTC | #2 
On Wed, Aug 10, 2022 at 11:20 AM Michael Jeanson <mjeanson at efficios.com> wrote:
>
> On 2022-08-03 15 h 23, Bruce Ashfield via lttng-dev wrote:
> > Hi all,
> >
> > This was found during the update of the yocto project reference
> > kernel to 5.19. Out of all the supported architectures, only
> > mips failed during the build of lttng-modules.
> >
> > Specifically, it seems like the lack of CONFIG_COMPACTION being
> > enabled in the kernel was the significant issue. Only one of the
> > tracepoints blew up, but I can't see how any of them would be
> > valid in this scenario, or why it would be limited to mips.
> >
> > While I don't think this is the right fix, this is what got me
> > up and passing sanity testing, so I wanted to send the change
> > and see what the actual, correct fix would be :)
> >
> > Cheers,
> >
> > Bruce
>
> Hi Bruce,
>
> Thanks for reporting this issue, I propose a simpler fix [1] of just
> disabling the build of the compaction probe module when
> CONFIG_COMPACTION is disabled in the kernel configuration.
>
> Can you confirm this patch fixes the issue on your side?

confirmed, it fixes the issue on my side.

Bruce

>
> Thanks,
>
> Michael
>
> [1] https://review.lttng.org/c/lttng-modules/+/8660
Mathieu Desnoyers Aug. 17, 2022, 3:27 p.m. UTC | #3 
----- On Aug 10, 2022, at 1:48 PM, Bruce Ashfield via lttng-dev lttng-dev at lists.lttng.org wrote:

> On Wed, Aug 10, 2022 at 11:20 AM Michael Jeanson <mjeanson at efficios.com> wrote:
>>
>> On 2022-08-03 15 h 23, Bruce Ashfield via lttng-dev wrote:
>> > Hi all,
>> >
>> > This was found during the update of the yocto project reference
>> > kernel to 5.19. Out of all the supported architectures, only
>> > mips failed during the build of lttng-modules.
>> >
>> > Specifically, it seems like the lack of CONFIG_COMPACTION being
>> > enabled in the kernel was the significant issue. Only one of the
>> > tracepoints blew up, but I can't see how any of them would be
>> > valid in this scenario, or why it would be limited to mips.
>> >
>> > While I don't think this is the right fix, this is what got me
>> > up and passing sanity testing, so I wanted to send the change
>> > and see what the actual, correct fix would be :)
>> >
>> > Cheers,
>> >
>> > Bruce
>>
>> Hi Bruce,
>>
>> Thanks for reporting this issue, I propose a simpler fix [1] of just
>> disabling the build of the compaction probe module when
>> CONFIG_COMPACTION is disabled in the kernel configuration.
>>
>> Can you confirm this patch fixes the issue on your side?
> 
> confirmed, it fixes the issue on my side.

Merged into lttng-modules master, stable-2.13, stable-2.12, thanks!

Mathieu

> 
> Bruce
> 
>>
>> Thanks,
>>
>> Michael
>>
>> [1] https://review.lttng.org/c/lttng-modules/+/8660
> 
> 
> 
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
diff mbox series

Patch

diff --git a/include/instrumentation/events/compaction.h b/include/instrumentation/events/compaction.h
index ecae39a..155804a 100644
--- a/include/instrumentation/events/compaction.h
+++ b/include/instrumentation/events/compaction.h
@@ -9,6 +9,8 @@ 
 #include <linux/types.h>
 #include <lttng/kernel-version.h>
 
+#ifdef CONFIG_COMPACTION
+
 #if LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(4,0,0)
 
 LTTNG_TRACEPOINT_EVENT_CLASS(compaction_isolate_template,
@@ -97,6 +99,7 @@  LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP(compaction_isolate_template,
 
 #endif /* #else #if LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(4,0,0) */
 
+
 #if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,18,0))
 LTTNG_TRACEPOINT_EVENT_MAP(mm_compaction_migratepages,
 
@@ -182,6 +185,8 @@  LTTNG_TRACEPOINT_EVENT_MAP(mm_compaction_migratepages,
 )
 #endif /* #else #if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(3,16,0)) */
 
+#endif /* CONFIG_COMPACTION */
+
 #endif /* LTTNG_TRACE_COMPACTION_H */
 
 /* This part must be outside protection */