trace_skb: fix build when CONFIG_NET is not enabled
Subsystems (2)
PATCH 1/1 trace_skb: fix build when CONFIG_NET is not enabled
From: Randy Dunlap <[email protected]>
Fix trace_skb_sources build when CONFIG_NET is not enabled:
kernel/built-in.o: In function `probe_skb_dequeue':
trace_skb_sources.c:(.text+0xd9152): undefined reference to init_net' trace_skb_sources.c:(.text+0xd9188): undefined reference to dev_get_by_index'
--- kernel/trace/Kconfig | 1 + 1 file changed, 1 insertion(+)
kernel/trace/Kconfig +1 -0
@@ -236,6 +236,7 @@ config BOOT_TRACER config SKB_SOURCES_TRACER bool "Trace skb source information" + depends on NET select GENERIC_TRACER help This tracer helps developers/sysadmins correlate skb allocation and
Signature · 3 lines
~Randy LPC 2009, Sept. 23-25, Portland, Oregon http://linuxplumbersconf.org/2009/
Quoted 17 lines · Randy Dunlap
From: Randy Dunlap <[email protected]>
Fix
trace_skb_sourcesbuild whenCONFIG_NETis not enabled:
kernel/built-in.o: In function `probe_skb_dequeue':
trace_skb_sources.c:(.text+0xd9152): undefined reference toinit_net' trace_skb_sources.c:(.text+0xd9188): undefined reference todev_get_by_index'Signed-off-by Randy Dunlap <[email protected]>Cc Neil Horman <[email protected]>--- kernel/trace/Kconfig | 1 + 1 file changed, 1 insertion(+)
kernel/trace/Kconfig config BOOT_TRACER · line 242
Quoted 7 lines
kernel/trace/Kconfig +1
-0@@ -236,6 +236,7 @@ config BOOT_TRACER config SKB_SOURCES_TRACER bool "Trace skb source information" + depends on NET select GENERIC_TRACER help This tracer helps developers/sysadmins correlate skb allocation and
Hm, there's nothing like this in the tracing tree.
Could we please move kernel/trace/* commits to the tracing tree, so that it gets adequate testing and review, etc?
Ingo
From: Ingo Molnar <[email protected]>
Date: Mon, 17 Aug 2009 23:58:08 +0200
Quoted 13 lines
kernel/trace/Kconfig config BOOT_TRACER · line 242
Quoted 10 lines
kernel/trace/Kconfig +1
-0@@ -236,6 +236,7 @@ config BOOT_TRACER config SKB_SOURCES_TRACER bool "Trace skb source information" + depends on NET select GENERIC_TRACER help This tracer helps developers/sysadmins correlate skb allocation andHm, there's nothing like this in the tracing tree.
Quoted 2 lines
Could we please move
kernel/trace/*commits to the tracing tree, so that it gets adequate testing and review, etc?
This one (like previous networking tracing changes Neil has made) touched a decent amount of networking code, and thus we integrated it into net-next-2.6
Quoted 16 lines · David Miller
From: Ingo Molnar <[email protected]>
Date: Mon, 17 Aug 2009 23:58:08 +0200Quoted 13 lines
kernel/trace/Kconfig config BOOT_TRACER · line 242
Quoted 10 lines
kernel/trace/Kconfig +1
-0@@ -236,6 +236,7 @@ config BOOT_TRACER config SKB_SOURCES_TRACER bool "Trace skb source information" + depends on NET select GENERIC_TRACER help This tracer helps developers/sysadmins correlate skb allocation andHm, there's nothing like this in the tracing tree.
Quoted 6 lines
Quoted 2 lines
Could we please move
kernel/trace/*commits to the tracing tree, so that it gets adequate testing and review, etc?This one (like previous networking tracing changes Neil has made) touched a decent amount of networking code, and thus we integrated it into net-next-2.6
the three skb-sources-tracer patches i saw submitted were:
include/trace/events/skb.h | 20 ++++++++++++++++++++ net/core/datagram.c | 3 +++ 2 files changed, 23 insertions(+)
kernel/traceKconfig | 10 ++++++++++
kernel/trace/Makefile | 1 kernel/trace/trace.h | 19 ++++++ kernel/trace/trace_skb_sources.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++
only touched the networking code for 3 lines innet/core/datagram.c.
Think about it: how would you react if i added a new file to net/core/ and modified net/Kconfig, and then broke the build? You'd quite likely insist on it being done via net-next-2.6, right? You'd also likely be upset about that kind of change, wouldnt you?
Also, has the review feedback from the tracing folks been addressed? Please separate these patches out and lets do this properly, this approach is not acceptable.
Ingo
From: Ingo Molnar <[email protected]>
Date: Tue, 18 Aug 2009 00:47:27 +0200
Quoted 3 lines
Also, has the review feedback from the tracing folks been addressed? Please separate these patches out and lets do this properly, this approach is not acceptable.
Fine, will keep you guys in the loop next time :-)
Quoted 8 lines · David Miller
From: Ingo Molnar <[email protected]>
Date: Tue, 18 Aug 2009 00:47:27 +0200Quoted 3 lines
Also, has the review feedback from the tracing folks been addressed? Please separate these patches out and lets do this properly, this approach is not acceptable.
Fine, will keep you guys in the loop next time :-)
thanks
If the TRACE_EVENT() form is used then it can be done just fine from the networking tree too by enhancing include/trace/events/skb.h - TRACE_EVENT() is a decentralized facility.
We try not to add new ftrace plugins but extend the set of generic tracepoints.
Ingo
Quoted 47 lines · Ingo Molnar
Quoted 16 lines · David Miller
From: Ingo Molnar <[email protected]>
Date: Mon, 17 Aug 2009 23:58:08 +0200Quoted 13 lines
kernel/trace/Kconfig config BOOT_TRACER · line 242
Quoted 10 lines
kernel/trace/Kconfig +1
-0@@ -236,6 +236,7 @@ config BOOT_TRACER config SKB_SOURCES_TRACER bool "Trace skb source information" + depends on NET select GENERIC_TRACER help This tracer helps developers/sysadmins correlate skb allocation andHm, there's nothing like this in the tracing tree.
Quoted 6 lines
Quoted 2 lines
Could we please move
kernel/trace/*commits to the tracing tree, so that it gets adequate testing and review, etc?This one (like previous networking tracing changes Neil has made) touched a decent amount of networking code, and thus we integrated it into net-next-2.6
the three skb-sources-tracer patches i saw submitted were:
include/trace/events/skb.h | 20 ++++++++++++++++++++ net/core/datagram.c | 3 +++ 2 files changed, 23 insertions(+)kernel/traceKconfig | 10 ++++++++++kernel/trace/Makefile | 1 kernel/trace/trace.h | 19 ++++++ kernel/trace/trace_skb_sources.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++only touched the networking code for 3 lines in
net/core/datagram.c.Think about it: how would you react if i added a new file to
net/core/and modified net/Kconfig, and then broke the build? You'd quite likely insist on it being done via net-next-2.6, right? You'd also likely be upset about that kind of change, wouldnt you?
Quoted 3 lines
Also, has the review feedback from the tracing folks been addressed? Please separate these patches out and lets do this properly, this approach is not acceptable.
btw., this isnt just a question of patch logistics - these patches look unreviewed to me. Why is a new ftrace plugin needed? Why arent they defined in a TRACE_EVENT() form? That way they'd be generally usable together with other tracepoints, and would likely be enabled in most distros. That way they could also be added via other trees, as TRACE_EVENT() is a generic facility.
We try to migrate ftrace plugins to TRACE_EVENT() tracepoints.
Ingo
David Miller
AppliedFrom: Randy Dunlap <[email protected]>
Date: Mon, 17 Aug 2009 13:37:36 -0700
Quoted 9 lines
From: Randy Dunlap <[email protected]>
Fix
trace_skb_sourcesbuild whenCONFIG_NETis not enabled:
kernel/built-in.o: In function `probe_skb_dequeue':
trace_skb_sources.c:(.text+0xd9152): undefined reference toinit_net' trace_skb_sources.c:(.text+0xd9188): undefined reference todev_get_by_index'Signed-off-by Randy Dunlap <[email protected]>
Applied, thanks Randy.
Quoted 7 lines · David Miller
From: Randy Dunlap <[email protected]>
Date: Mon, 17 Aug 2009 13:37:36 -0700Quoted 4 lines
From: Randy Dunlap <[email protected]>
Fix
trace_skb_sourcesbuild whenCONFIG_NETis not enabled:
Quoted 7 lines
Quoted 5 lines
kernel/built-in.o: In function `probe_skb_dequeue':
trace_skb_sources.c:(.text+0xd9152): undefined reference toinit_net' trace_skb_sources.c:(.text+0xd9188): undefined reference todev_get_by_index'Signed-off-by Randy Dunlap <[email protected]>Applied, thanks Randy.
... but these are the wrong patches, they should be removed or reverted and redone properly.
It's not just about keeping kernel/trace/* changes in the tracing tree (which we can relax on-demand given agreement), it's that these patches are also _wrong_ and we cannot relax anything about that.
Ingo
From: Ingo Molnar <[email protected]>
Date: Tue, 18 Aug 2009 01:06:54 +0200
Quoted 2 lines
... but these are the wrong patches, they should be removed or reverted and redone properly.
Given the number of users of net-next-2.6, removal via rebasing is simply not an option. The other two posibilities, sure...
Quoted 4 lines
It's not just about keeping
kernel/trace/*changes in the tracing tree (which we can relax on-demand given agreement), it's that these patches are also _wrong_ and we cannot relax anything about that.
Why don't we give Neil a chance to review the situation and fix things up?
If revert is the final decision, that's fine, I'll revert everything.
But in the mean time at least give Neil a chance to read all of your feedback and coordinate a way to fix things with everyone.
Quoted 14 lines · David Miller
From: Ingo Molnar <[email protected]>
Date: Tue, 18 Aug 2009 01:06:54 +0200Quoted 2 lines
... but these are the wrong patches, they should be removed or reverted and redone properly.
Given the number of users of net-next-2.6, removal via rebasing is simply not an option. The other two posibilities, sure...
Quoted 4 lines
It's not just about keeping
kernel/trace/*changes in the tracing tree (which we can relax on-demand given agreement), it's that these patches are also _wrong_ and we cannot relax anything about that.
Quoted 8 lines
Why don't we give Neil a chance to review the situation and fix things up?
If revert is the final decision, that's fine, I'll revert everything.
But in the mean time at least give Neil a chance to read all of your feedback and coordinate a way to fix things with everyone.
Oh, sure. Conversion to TRACE_EVENT() is equivalent to the revert of the current patches plus introduction of the TRACE_EVENT() defines.
Ingo