Markers Implementation for Preempt RCU Boost Tracing
PATCH 2/2 Markers Implementation for Preempt RCU Boost Tracing
This patch converts the tracing mechanism of Preempt RCU boosting into markers. The handler functions for these markers are included inside rcupreempt_trace.c and will be included only when PREEMPT_RCU_BOOST is chosen.
--- include/linux/rcupreempt_trace.h | 46 ++++++++ kernel/rcupreempt-boost.c | 211 ++++----------------------------------- kernel/rcupreempt_trace.c | 183 +++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 189 deletions(-)
include/linux/rcupreempt_trace.h +46 -0
@@ -102,5 +102,51 @@ extern int rcupreempt_flip_flag(int cpu)extern int rcupreempt_mb_flag (int cpu);extern char *rcupreempt_try_flip_state_name (void ); +#ifdef CONFIG_PREEMPT_RCU_BOOST +struct preempt_rcu_boost_trace { +unsigned long rbs_stat_task_boost_called; +unsigned long rbs_stat_task_boosted; +unsigned long rbs_stat_boost_called; +unsigned long rbs_stat_try_boost; +unsigned long rbs_stat_boosted; +unsigned long rbs_stat_unboost_called; +unsigned long rbs_stat_unboosted; +unsigned long rbs_stat_try_boost_readers; +unsigned long rbs_stat_boost_readers; +unsigned long rbs_stat_try_unboost_readers; +unsigned long rbs_stat_unboost_readers; +unsigned long rbs_stat_over_taken; +}; + +#define DEFINE_PREEMPT_RCU_BOOST_MARKER_HANDLER (preempt_rcu_boost_var) \ +void preempt_rcu_boost_var##_callback(const struct marker *mdata, \ + void *private_data, const char *format, ...) \ +{ \ + va_list ap; \ + int cpu; \ + struct preempt_rcu_boost_trace *boost_trace; \ + va_start(ap, format); \ + cpu = va_arg(ap, typeof(unsigned int)); \ + boost_trace = (&per_cpu(boost_trace_data, cpu)); \ + boost_trace->rbs_stat_##preempt_rcu_boost_var++; \ + va_end(ap); \ +} + +struct preempt_rcu_boost_probe { +const char *name; +const char *format; +marker_probe_func *probe_func; +}; + +#define INIT_PREEMPT_RCU_BOOST_PROBE (preempt_rcu_boost_probe_worker) \ +{ \ + .name = __stringify(preempt_rcu_boost_probe_worker), \ + .format = "%u", \ + .probe_func = preempt_rcu_boost_probe_worker##_callback \ +} + +extern int read_rcu_boost_prio (void ); +#endif /* CONFIG_PREEMPT_RCU_BOOST */ +#endif /* __KERNEL__ */ #endif /* __LINUX_RCUPREEMPT_TRACE_H */
kernel/rcupreempt_trace.c +183 -0
207 lines not on this page · raw message
kernel/rcupreempt-boost.c +22 -189
292 lines not on this page · raw message
kernel/rcupreempt-boost.c void rcu_unboost_readers(void) · line 312
Quoted 6 lines · K. Prasad
kernel/rcupreempt-boost.c +1
-1@@ -486,12 +309,12 @@ void rcu_unboost_readers(void)spin_lock_irqsave (&rcu_boost_wake_lock, flags); -rcu_trace_boost_try_unboost_readers (RCU_BOOST_ME ); +trace_mark (try_unboost_readers,"%u" ,smp_processor_id ());
this isnt directed at you or your patch, it's more directed at Mathieu, but looking at this actual markers patch submitted to me, i'm still fundamentally worried about the whole marker approach.
Firstly, why on earth does a full format string have to be passed in for something as simple as a CPU id? This way we basically codify it forever that tracing _has_ to be expensive when enabled. The latency-tracer (which i'd love to convert to markers, if only markers were capable enough) has shown it that tracing _can_ be used to capture performance data without disturbing the measured system, even at hundreds of thousands context switches a second per CPU.
Secondly, the inlined overhead of trace_mark() is still WAY too large:
if (unlikely(__mark_##name.state)) { \
preempt_disable(); \
(*__mark_##name.call) \
(&__mark_##name, call_data, \
format, ## args); \
preempt_enable(); \
} \Whatever became of the obvious suggestion that i outlined years ago, to have a _single_ trace call instruction and to _patch out_ the damn marker calls by default? No .state flag checking inlined. No preempt_disable()/enable() pair. Patching static tracepoints OUT of the kernel, only leaving a ~5-byte NOP sequence behind them (and some minimal disturbance to the variables the tracepoint accesses). We've got all the alternatives.h code patching infrastructure available for such purposes. Why are we 2 years down the line and _STILL_ arguing about this?
Thirdly, the patch selects CONFIG_MARKERS:
Review void rcu_unboost_readers(void) · line 316
Quoted 6 lines
(quoted diff) +2
-2config RCU_TRACE - bool "Enable tracing for RCU - currently stats in debugfs" + tristate "Enable tracing for RCU - currently stats in debugfs" select DEBUG_FS - default y + select MARKERS
Which adds overhead (inlined checks for markers) all around the kernel, even if all markers are deactivated! Imagine a thousand of them and the kernel blows up measurably.
Sadly, this whole trace_mark() API seems to have gotten much worse since i last saw it. It's sub-par when it's turned on and it's sub-par when it's turned off. It gets us the worst of both worlds.
If it continues like this then i'd much rather see people add printks as tracing, because there you _know_ that it's high-overhead and people wont start arguing about trace compatibility either, etc.
Ingo
Quoted 4 lines · Ingo Molnar
[...] Firstly, why on earth does a full format string have to be passed in for something as simple as a CPU id? This way we basically codify it forever that tracing _has_ to be expensive when enabled. [...]
FWIW, I'm not keen about the format strings either, but they don't constitute a performance hit beyond an additional parameter. It does not need to actually get parsed at run time.
Quoted 6 lines
[...]
Secondly, the inlined overhead oftrace_mark()is still WAY too large:if (unlikely(__mark_##name.state)) { \ [...] } \
Note that this is for the unoptimized case. The immediate-value code is better. I have still yet to see some good measurements of how much the overheads of the various variants are, however. It's only fair to gather these numbers and continue the debate with them in hand.
Quoted 5 lines
Whatever became of the obvious suggestion that i outlined years ago, to have a _single_ trace call instruction and to
_patchout_the damn marker calls by default? [...] only leaving a ~5-byte NOP sequence behind them (and some minimal disturbance to the variables the tracepoint accesses). [...]
This has been answered several times before. It's because the marker parameters have to be (conditionally) evaluated and pushed onto a call frame. It's not just a call that would need being nop'd, but a whole function call setup/teardown sequence, which itself can be interleaved with adjacent code.
- FChE
Quoted 10 lines · Frank Ch. Eigler
Quoted 4 lines · Ingo Molnar
[...] Firstly, why on earth does a full format string have to be passed in for something as simple as a CPU id? This way we basically codify it forever that tracing _has_ to be expensive when enabled. [...]
FWIW, I'm not keen about the format strings either, but they don't constitute a performance hit beyond an additional parameter. It does not need to actually get parsed at run time.
"only" an additional parameter. The whole _point_ behind these markers is for them to have minimal effect!
Quoted 11 lines
Quoted 6 lines
[...]
Secondly, the inlined overhead oftrace_mark()is still WAY too large:if (unlikely(__mark_##name.state)) { \ [...] } \Note that this is for the unoptimized case. The immediate-value code is better. I have still yet to see some good measurements of how much the overheads of the various variants are, however. It's only fair to gather these numbers and continue the debate with them in hand.
this is a general policy matter. It is _so much easier_ to add markers if they _can_ have near-zero overhead (as in 1-2 instructions). Otherwise we'll keep arguing about it, especially if any is added to performance-critical codepath. (where we are counting instructions)
Quoted 11 lines
Quoted 5 lines
Whatever became of the obvious suggestion that i outlined years ago, to have a _single_ trace call instruction and to
_patchout_the damn marker calls by default? [...] only leaving a ~5-byte NOP sequence behind them (and some minimal disturbance to the variables the tracepoint accesses). [...]This has been answered several times before. It's because the marker parameters have to be (conditionally) evaluated and pushed onto a call frame. It's not just a call that would need being nop'd, but a whole function call setup/teardown sequence, which itself can be interleaved with adjacent code.
you are still missing my point. Firstly, the kernel is regparm built so there's no call frame to be pushed to in most cases - we pass most parameters in registers. (hence my stressing of minimizing the number of parameters to an absolute minimum)
Secondly, the side-effects of a function call is what i referred to via:
Quoted 2 lines
[...] (and some minimal disturbance to the variables the tracepoint accesses). [...]
really, a tracepoint obviously accesses data that is readily accessible in that spot. Worst-case we'll have some additional register constraints that make the code a bit less optimal.
Yes, if a trace point references data that is _not_ readily available, then of course the preparation for the function call might not be cheap. But that can be optimized by placing the tracepoints intelligently.
what cannot be optimized away at all are the conditional instructions introduced by the probe points, extra parameters and the space overhead of the function call itself.
Ingo
Hi -
Quoted 7 lines · Ingo Molnar
[...]
Quoted 3 lines
FWIW, I'm not keen about the format strings either, but they don't constitute a performance hit beyond an additional parameter. It does not need to actually get parsed at run time.
"only" an additional parameter. The whole _point_ behind these markers is for them to have minimal effect!
Agreed. The only alternative I recall seeing proposed was my own cartesian-product macro suite that encodes parameter types into the marker function/macro name itself. (Maybe some of that could be hidden with gcc typeof() magic.) There appeared to be a consensus that this was more undesirable. Do you agree?
Quoted 5 lines
[...]
this is a general policy matter. It is_somucheasier_to add markers if they _can_ have near-zero overhead (as in 1-2 instructions). Otherwise we'll keep arguing about it, especially if any is added to performance-critical codepath. (where we are counting instructions)
The effect of the immediate-values patch, combined with gcc CFLAGS+=-freorder-blocks, *is* to keep the overhead at 1-2 dcache-impact-free instructions. The register saves, parameter evaluation, the function call, can all be moved out of line.
Quoted 12 lines
Quoted 11 lines
Quoted 5 lines
Whatever became of the obvious suggestion that i outlined years ago, to have a _single_ trace call instruction and to
_patchout_the damn marker calls by default? [...] only leaving a ~5-byte NOP sequence behind them (and some minimal disturbance to the variables the tracepoint accesses). [...]This has been answered several times before. It's because the marker parameters have to be (conditionally) evaluated and pushed onto a call frame. It's not just a call that would need being nop'd, but a whole function call setup/teardown sequence, which itself can be interleaved with adjacent code.
Quoted 3 lines
you are still missing my point. Firstly, the kernel is regparm built so there's no call frame to be pushed to in most cases - we pass most parameters in registers. [...]
Yes, but those registers will already be used, so their former values need to be saved or later recomputed as a part of the call sequence. Even wrapping it all with an asm barrier may not be enough to completely bound the code with an address range that can be naively NOP'd out.
Quoted 8 lines
Secondly, the side-effects of a function call is what i referred to via:
Quoted 2 lines
[...] (and some minimal disturbance to the variables the tracepoint accesses). [...]
really, a tracepoint obviously accesses data that is readily accessible in that spot. Worst-case we'll have some additional register constraints that make the code a bit less optimal. [...]
It's more than that - people will want to pass some dereferenced fields from a struct*; some old function parameters that were by the time of the marker call shoved out of registers again. Register constraints that prohibit general expressions would impede the utility of the tool.
Quoted 3 lines
what cannot be optimized away at all are the conditional instructions introduced by the probe points, extra parameters and the space overhead of the function call itself.
Almost all of that can & should be moved out of line.
- FChE
Quoted 10 lines · Frank Ch. Eigler
Quoted 5 lines
[...] this is a general policy matter. It is
_somucheasier_to add markers if they _can_ have near-zero overhead (as in 1-2 instructions). Otherwise we'll keep arguing about it, especially if any is added to performance-critical codepath. (where we are counting instructions)The effect of the immediate-values patch, combined with gcc CFLAGS+=-freorder-blocks, *is* to keep the overhead at 1-2 dcache-impact-free instructions. The register saves, parameter evaluation, the function call, can all be moved out of line.
well, -freorder-blocks seems to be default-enabled at -O2 on gcc 4.2, so we should already be getting that, right?
There's one thing that would make out-of-line tracepoints have a lot less objectionable to me: right now the 'out of line' area is put to the end of functions. That splinters the kernel image with inactive, rarely taken areas of code - blowing up its icache footprint considerably. For example sched.o has ~100 functions, with the average function size being 200 bytes. At 64 bytes L1 cacheline size that's a 10-20% icache waste already.
It's true that keeping the off-site code within the function keeps total codesize slightly smaller, because the offsets (and hence the conditional jumps) are thus 8 bit - but that's below 1% and the cache-blow-up aspect is more severe in practice at 10-20%.
So it would be nice if we could collect all this offline code and stuff it away into another portion of the kernel image. (or, into another portion of the object file - which would still be good enough in practice)
Ingo
Hi -
Quoted 3 lines · Ingo Molnar
[...]
well, -freorder-blocks seems to be default-enabled at -O2 on gcc 4.2, so we should already be getting that, right?
Right.
Quoted 4 lines
[...] So it would be nice if we could collect all this offline code and stuff it away into another portion of the kernel image. (or, into another portion of the object file - which would still be good enough in practice)
That would be the -freorder-blocks-and-partition flag, as proposed by Arjan two Februarys ago. I don't see any traces of Andi's overriding "-fno-reorder-blocks" in the current linus tree, so maybe it's time to resurrect this one:
http://readlist.com/lists/vger.kernel.org/linux-kernel/39/196123.html
- FChE
Quoted 14 lines · Frank Ch. Eigler
Hi -
Quoted 3 lines · Ingo Molnar
[...]
well, -freorder-blocks seems to be default-enabled at -O2 on gcc 4.2, so we should already be getting that, right?Right.
Quoted 4 lines
[...] So it would be nice if we could collect all this offline code and stuff it away into another portion of the kernel image. (or, into another portion of the object file - which would still be good enough in practice)
Quoted 6 lines
That would be the -freorder-blocks-and-partition flag, as proposed by Arjan two Februarys ago. I don't see any traces of Andi's overriding "-fno-reorder-blocks" in the current linus tree, so maybe it's time to resurrect this one:
http://readlist.com/lists/vger.kernel.org/linux-kernel/39/196123.html
hm, that gives:
Forbidden
You don't have permission to access /lists/vger.kernel.org/linux-kernel/39/196123.html on this server.
but yeah, i had the impression that gcc couldnt yet do this. Not a showstopper, but it would be nice to initiate the gcc side of things ...
Ingo
Quoted 17 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 10 lines · Frank Ch. Eigler
Quoted 5 lines
[...] this is a general policy matter. It is
_somucheasier_to add markers if they _can_ have near-zero overhead (as in 1-2 instructions). Otherwise we'll keep arguing about it, especially if any is added to performance-critical codepath. (where we are counting instructions)The effect of the immediate-values patch, combined with gcc CFLAGS+=-freorder-blocks, *is* to keep the overhead at 1-2 dcache-impact-free instructions. The register saves, parameter evaluation, the function call, can all be moved out of line.
well, -freorder-blocks seems to be default-enabled at -O2 on gcc 4.2, so we should already be getting that, right?
Quoted 7 lines
There's one thing that would make out-of-line tracepoints have a lot less objectionable to me: right now the 'out of line' area is put to the end of functions. That splinters the kernel image with inactive, rarely taken areas of code - blowing up its icache footprint considerably. For example sched.o has ~100 functions, with the average function size being 200 bytes. At 64 bytes L1 cacheline size that's a 10-20% icache waste already.
Hrm, I agree this can be a problem on architectures with more standard associative icaches, but aren't most x86_64 machines (and modern x86_32) using an instruction trace cache instead ? This makes the problem irrelevant.
But I agree that, as Frank proposed, -freorder-blocks-and-partition could help us in that matter for the architectures using an associative L1 icache.
Mathieu
Quoted 12 lines
It's true that keeping the off-site code within the function keeps total codesize slightly smaller, because the offsets (and hence the conditional jumps) are thus 8 bit - but that's below 1% and the cache-blow-up aspect is more severe in practice at 10-20%.
So it would be nice if we could collect all this offline code and stuff it away into another portion of the kernel image. (or, into another portion of the object file - which would still be good enough in practice)
Ingo
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 26 lines · Mathieu Desnoyers
Quoted 17 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 10 lines · Frank Ch. Eigler
Quoted 5 lines
[...] this is a general policy matter. It is
_somucheasier_to add markers if they _can_ have near-zero overhead (as in 1-2 instructions). Otherwise we'll keep arguing about it, especially if any is added to performance-critical codepath. (where we are counting instructions)The effect of the immediate-values patch, combined with gcc CFLAGS+=-freorder-blocks, *is* to keep the overhead at 1-2 dcache-impact-free instructions. The register saves, parameter evaluation, the function call, can all be moved out of line.
well, -freorder-blocks seems to be default-enabled at -O2 on gcc 4.2, so we should already be getting that, right?
Quoted 7 lines
There's one thing that would make out-of-line tracepoints have a lot less objectionable to me: right now the 'out of line' area is put to the end of functions. That splinters the kernel image with inactive, rarely taken areas of code - blowing up its icache footprint considerably. For example sched.o has ~100 functions, with the average function size being 200 bytes. At 64 bytes L1 cacheline size that's a 10-20% icache waste already.
Quoted 8 lines
Hrm, I agree this can be a problem on architectures with more standard associative icaches, but aren't most
x86_64machines (and modernx86_32) using an instruction trace cache instead ? This makes the problem irrelevant.But I agree that, as Frank proposed, -freorder-blocks-and-partition could help us in that matter for the architectures using an associative L1 icache.
I thought trace cache died with P4?
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 32 lines · * Pavel Machek ([email protected]) wrote:
Quoted 31 lines · Mathieu Desnoyers
Quoted 17 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 10 lines · Frank Ch. Eigler
Quoted 5 lines
[...] this is a general policy matter. It is
_somucheasier_to add markers if they _can_ have near-zero overhead (as in 1-2 instructions). Otherwise we'll keep arguing about it, especially if any is added to performance-critical codepath. (where we are counting instructions)The effect of the immediate-values patch, combined with gcc CFLAGS+=-freorder-blocks, *is* to keep the overhead at 1-2 dcache-impact-free instructions. The register saves, parameter evaluation, the function call, can all be moved out of line.
well, -freorder-blocks seems to be default-enabled at -O2 on gcc 4.2, so we should already be getting that, right?
Quoted 7 lines
There's one thing that would make out-of-line tracepoints have a lot less objectionable to me: right now the 'out of line' area is put to the end of functions. That splinters the kernel image with inactive, rarely taken areas of code - blowing up its icache footprint considerably. For example sched.o has ~100 functions, with the average function size being 200 bytes. At 64 bytes L1 cacheline size that's a 10-20% icache waste already.
Hrm, I agree this can be a problem on architectures with more standard associative icaches, but aren't most
x86_64machines (and modernx86_32) using an instruction trace cache instead ? This makes the problem irrelevant.
Quoted 6 lines
Quoted 3 lines
But I agree that, as Frank proposed, -freorder-blocks-and-partition could help us in that matter for the architectures using an associative L1 icache.
I thought trace cache died with P4?
And you are absolutely right.
We would have to figure out if enabling -freorder-blocks-and-partition makes sense kernel-wide.
Mathieu
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 3 lines · Mathieu Desnoyers
We would have to figure out if enabling -freorder-blocks-and-partition makes sense kernel-wide.
Last I saw, it generates crappy code, with lots more jumps back and forth, and the image just blows up.
There's a reason we use -Os, and that's that small footprint I$ is generally more important than fake compiler optimizations that don't actually help except on microbenchmarks where everything fits in the cache.
Taking a branch instruction from two bytes to five is almost always a mistake, unless you *know* that the code it jumps to will effectively never be done at all (which is not necessarily the case at all). It also makes debugging much nastier, because if now things like backtraces probably look like crap too!
Don't go there. The *best* we can do is to just use the optimizations that generate good-looking code that humans can read. The rest is just compiler masturbation.
Linus
Quoted 21 lines · * Linus Torvalds ([email protected]) wrote:
Quoted 3 lines · Mathieu Desnoyers
We would have to figure out if enabling -freorder-blocks-and-partition makes sense kernel-wide.
Last I saw, it generates crappy code, with lots more jumps back and forth, and the image just blows up.
There's a reason we use -Os, and that's that small footprint I$ is generally more important than fake compiler optimizations that don't actually help except on microbenchmarks where everything fits in the cache.
Taking a branch instruction from two bytes to five is almost always a mistake, unless you *know* that the code it jumps to will effectively never be done at all (which is not necessarily the case at all). It also makes debugging much nastier, because if now things like backtraces probably look like crap too!
Quoted 4 lines
Don't go there. The *best* we can do is to just use the optimizations that generate good-looking code that humans can read. The rest is just compiler masturbation.
I agree that turning this flag on does not seem like an interesting solution.
Well, I wonder how important this issue of not sharing L1 instruction cachelines with scheduler code is. If we care as much about it as Ingo states, I wonder why we leave about 22 BUG() macros in sched.c (calculated from the number of ud2 instructions generated on x86), which adds up to 7 bytes at the end of many scheduler functions (7 bytes coming from ud2, jmp and .p2align on x86).
And 22 markers in sched.c is already much more than needed. I actually propose only 5 in my patchset.
Mathieu
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 8 lines · Frank Ch. Eigler
Hi -
Quoted 5 lines · Ingo Molnar
[...]
Quoted 3 lines
FWIW, I'm not keen about the format strings either, but they don't constitute a performance hit beyond an additional parameter. It does not need to actually get parsed at run time.
Quoted 10 lines
Quoted 2 lines
"only" an additional parameter. The whole _point_ behind these markers is for them to have minimal effect!
Agreed. The only alternative I recall seeing proposed was my own cartesian-product macro suite that encodes parameter types into the marker function/macro name itself. (Maybe some of that could be hidden with gcc
typeof()magic.) There appeared to be a consensus that this was more undesirable. Do you agree?
C++ name mangling would be extremely useful here.
Actually, why isn't the DWARF information for the functions sufficient?
Signature · 1 line
Nicholas Miell <[email protected]>
Quoted 5 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 2 lines · K. Prasad
(quoted diff) +0
-0@@ -486,12 +309,12 @@ void rcu_unboost_readers(void)
Quoted 9 lines
kernel/rcupreempt-boost.c void rcu_unboost_readers(void) · line 312
Quoted 4 lines
kernel/rcupreempt-boost.c +1
-1spin_lock_irqsave (&rcu_boost_wake_lock, flags); -rcu_trace_boost_try_unboost_readers (RCU_BOOST_ME ); +trace_mark (try_unboost_readers,"%u" ,smp_processor_id ());this isnt directed at you or your patch, it's more directed at Mathieu, but looking at this actual markers patch submitted to me, i'm still fundamentally worried about the whole marker approach.
Hi Ingo,
Just to note that some of your concerns have been answered by Frank and that I fully agree with what he said. Sorry for the late reply (vacation..)
Quoted 8 lines
Firstly, why on earth does a full format string have to be passed in for something as simple as a CPU id? This way we basically codify it forever that tracing _has_ to be expensive when enabled. The latency-tracer (which i'd love to convert to markers, if only markers were capable enough) has shown it that tracing _can_ be used to capture performance data without disturbing the measured system, even at hundreds of thousands context switches a second per CPU.
As I replyed in my other email, the cpu id does not have to be passed as an argument. Also, when no argument is passed, the format string does not have to be parsed at all in the callback.
Quoted 10 lines
Secondly, the inlined overhead of
trace_mark()is still WAY too large:if (unlikely(__mark_##name.state)) { \ preempt_disable(); \ (*__mark_##name.call) \ (&__mark_##name, call_data, \ format, ## args); \ preempt_enable(); \ } \
To get the full version of my trace_mark, you would have to take a few bits from the -mm tree, which includes the "multiple probes support" (this one moves the preempt disable/enable completely out of line) and you should also get the "markers use immediate values" patch which I submitted a few weeks ago : it uses code patching to modify a byte in a mov instruction that is used to jump over the entire function call (memory references, stack setup and the call itself)
Quoted 10 lines
Whatever became of the obvious suggestion that i outlined years ago, to have a _single_ trace call instruction and to
_patchout_the damn marker calls by default? No .state flag checking inlined. No preempt_disable()/enable() pair. Patching static tracepoints OUT of the kernel, only leaving a ~5-byte NOP sequence behind them (and some minimal disturbance to the variables the tracepoint accesses). We've got all thealternatives.hcode patching infrastructure available for such purposes. Why are we 2 years down the line and _STILL_ arguing about this?
Frank replied appropriately to this. It's mostly because of stack setup and/or move to registers to prepare the call.
Quoted 13 lines
Thirdly, the patch selects
CONFIG_MARKERS:Review void rcu_unboost_readers(void) · line 316
Quoted 6 lines
(quoted diff) +2
-2config RCU_TRACE - bool "Enable tracing for RCU - currently stats in debugfs" + tristate "Enable tracing for RCU - currently stats in debugfs" select DEBUG_FS - default y + select MARKERSWhich adds overhead (inlined checks for markers) all around the kernel, even if all markers are deactivated! Imagine a thousand of them and the kernel blows up measurably.
Quoted 8 lines
Sadly, this whole
trace_mark()API seems to have gotten much worse since i last saw it. It's sub-par when it's turned on and it's sub-par when it's turned off. It gets us the worst of both worlds.If it continues like this then i'd much rather see people add printks as tracing, because there you _know_ that it's high-overhead and people wont start arguing about trace compatibility either, etc.
I think Frank's response explains things in enough depth.
Quoted 1 line
Ingo
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 35 lines · * K. Prasad ([email protected]) wrote:
This patch converts the tracing mechanism of Preempt RCU boosting into markers. The handler functions for these markers are included inside
rcupreempt_trace.cand will be included only whenPREEMPT_RCU_BOOSTis chosen.Signed-off-by K.Prasad <[email protected]>--- include/linux/rcupreempt_trace.h | 46 ++++++++ kernel/rcupreempt-boost.c | 211 ++++----------------------------------- kernel/rcupreempt_trace.c | 183 +++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 189 deletions(-)Index:
linux-2.6.24-rc2-rt1.MARKER_PATCHES_NEW/include/linux/rcupreempt_trace.h
===================================================================include/linux/rcupreempt_trace.h +15
-0@@ -102,5 +102,51 @@ extern int rcupreempt_flip_flag(int cpu)extern int rcupreempt_mb_flag (int cpu);extern char *rcupreempt_try_flip_state_name (void ); +#ifdef CONFIG_PREEMPT_RCU_BOOST +struct preempt_rcu_boost_trace { +unsigned long rbs_stat_task_boost_called; +unsigned long rbs_stat_task_boosted; +unsigned long rbs_stat_boost_called; +unsigned long rbs_stat_try_boost; +unsigned long rbs_stat_boosted; +unsigned long rbs_stat_unboost_called; +unsigned long rbs_stat_unboosted; +unsigned long rbs_stat_try_boost_readers; +unsigned long rbs_stat_boost_readers; +unsigned long rbs_stat_try_unboost_readers; +unsigned long rbs_stat_unboost_readers; +unsigned long rbs_stat_over_taken; +};
include/linux/rcupreempt_trace.h extern int rcupreempt_flip_flag(int cpu) · line 129
Quoted 10 lines
include/linux/rcupreempt_trace.h +10
-0+ +#define DEFINE_PREEMPT_RCU_BOOST_MARKER_HANDLER (preempt_rcu_boost_var) \ +void preempt_rcu_boost_var##_callback(const struct marker *mdata, \ + void *private_data, const char *format, ...) \ +{ \ + va_list ap; \ + int cpu; \ + struct preempt_rcu_boost_trace *boost_trace; \ + va_start(ap, format); \ + cpu = va_arg(ap, typeof(unsigned int)); \
(sorry for late response, I just came back from vacation)
Instead of passing the cpu id as a marker parameter, why don't you simply use smp_processor_id() right here ?
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 49 lines · Mathieu Desnoyers
Quoted 35 lines · * K. Prasad ([email protected]) wrote:
This patch converts the tracing mechanism of Preempt RCU boosting into markers. The handler functions for these markers are included inside
rcupreempt_trace.cand will be included only whenPREEMPT_RCU_BOOSTis chosen.Signed-off-by K.Prasad <[email protected]>--- include/linux/rcupreempt_trace.h | 46 ++++++++ kernel/rcupreempt-boost.c | 211 ++++----------------------------------- kernel/rcupreempt_trace.c | 183 +++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 189 deletions(-)Index:
linux-2.6.24-rc2-rt1.MARKER_PATCHES_NEW/include/linux/rcupreempt_trace.h
===================================================================include/linux/rcupreempt_trace.h +15
-0@@ -102,5 +102,51 @@ extern int rcupreempt_flip_flag(int cpu)extern int rcupreempt_mb_flag (int cpu);extern char *rcupreempt_try_flip_state_name (void ); +#ifdef CONFIG_PREEMPT_RCU_BOOST +struct preempt_rcu_boost_trace { +unsigned long rbs_stat_task_boost_called; +unsigned long rbs_stat_task_boosted; +unsigned long rbs_stat_boost_called; +unsigned long rbs_stat_try_boost; +unsigned long rbs_stat_boosted; +unsigned long rbs_stat_unboost_called; +unsigned long rbs_stat_unboosted; +unsigned long rbs_stat_try_boost_readers; +unsigned long rbs_stat_boost_readers; +unsigned long rbs_stat_try_unboost_readers; +unsigned long rbs_stat_unboost_readers; +unsigned long rbs_stat_over_taken; +};include/linux/rcupreempt_trace.h extern int rcupreempt_flip_flag(int cpu) · line 129
Quoted 10 lines
include/linux/rcupreempt_trace.h +10
-0+ +#define DEFINE_PREEMPT_RCU_BOOST_MARKER_HANDLER (preempt_rcu_boost_var) \ +void preempt_rcu_boost_var##_callback(const struct marker *mdata, \ + void *private_data, const char *format, ...) \ +{ \ + va_list ap; \ + int cpu; \ + struct preempt_rcu_boost_trace *boost_trace; \ + va_start(ap, format); \ + cpu = va_arg(ap, typeof(unsigned int)); \(sorry for late response, I just came back from vacation)
Quoted 3 lines
Instead of passing the cpu id as a marker parameter, why don't you simply use
smp_processor_id()right here ?
Agreed.
Also in patch 1, in cases where we're using markers in place of RCU_TRACE_ME(), we need not pass the parameter and simply use smp_processor_id().
It's only the RCU_TRACE_RDP() that requires cpuid to be passed.
Thanks and Regards
gautham.
Quoted 4 lines
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Signature · 5 lines
Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!"
Quoted 54 lines · * Gautham R Shenoy ([email protected]) wrote:
Quoted 49 lines · Mathieu Desnoyers
Quoted 35 lines · * K. Prasad ([email protected]) wrote:
This patch converts the tracing mechanism of Preempt RCU boosting into markers. The handler functions for these markers are included inside
rcupreempt_trace.cand will be included only whenPREEMPT_RCU_BOOSTis chosen.Signed-off-by K.Prasad <[email protected]>--- include/linux/rcupreempt_trace.h | 46 ++++++++ kernel/rcupreempt-boost.c | 211 ++++----------------------------------- kernel/rcupreempt_trace.c | 183 +++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 189 deletions(-)Index:
linux-2.6.24-rc2-rt1.MARKER_PATCHES_NEW/include/linux/rcupreempt_trace.h
===================================================================include/linux/rcupreempt_trace.h +15
-0@@ -102,5 +102,51 @@ extern int rcupreempt_flip_flag(int cpu)extern int rcupreempt_mb_flag (int cpu);extern char *rcupreempt_try_flip_state_name (void ); +#ifdef CONFIG_PREEMPT_RCU_BOOST +struct preempt_rcu_boost_trace { +unsigned long rbs_stat_task_boost_called; +unsigned long rbs_stat_task_boosted; +unsigned long rbs_stat_boost_called; +unsigned long rbs_stat_try_boost; +unsigned long rbs_stat_boosted; +unsigned long rbs_stat_unboost_called; +unsigned long rbs_stat_unboosted; +unsigned long rbs_stat_try_boost_readers; +unsigned long rbs_stat_boost_readers; +unsigned long rbs_stat_try_unboost_readers; +unsigned long rbs_stat_unboost_readers; +unsigned long rbs_stat_over_taken; +};include/linux/rcupreempt_trace.h extern int rcupreempt_flip_flag(int cpu) · line 129
Quoted 10 lines
include/linux/rcupreempt_trace.h +10
-0+ +#define DEFINE_PREEMPT_RCU_BOOST_MARKER_HANDLER (preempt_rcu_boost_var) \ +void preempt_rcu_boost_var##_callback(const struct marker *mdata, \ + void *private_data, const char *format, ...) \ +{ \ + va_list ap; \ + int cpu; \ + struct preempt_rcu_boost_trace *boost_trace; \ + va_start(ap, format); \ + cpu = va_arg(ap, typeof(unsigned int)); \(sorry for late response, I just came back from vacation)
Quoted 3 lines
Instead of passing the cpu id as a marker parameter, why don't you simply use
smp_processor_id()right here ?
Quoted 7 lines
Agreed.
Also in patch 1, in cases where we're using markers in place of
RCU_TRACE_ME(), we need not pass the parameter and simply usesmp_processor_id().It's only the
RCU_TRACE_RDP()that requires cpuid to be passed.
And it would help if you declare the format string with something like :
"cpuid %u" instead of "%u"
This way, if a generic tracer like LTTng dumps the traces and later a plugin in an analysis tool like LTTV have to hook on a specific field of your event, it can access it by specifying the marker name and field name. And even if fields are added to this event, it won't break compatibility as long as this field is there and have the same name.
Mathieu
Quoted 16 lines
Thanks and Regards
gautham.Quoted 4 lines
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68Signature · 5 lines
Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!"
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 68 lines · Mathieu Desnoyers
Quoted 54 lines · * Gautham R Shenoy ([email protected]) wrote:
Quoted 49 lines · Mathieu Desnoyers
Quoted 35 lines · * K. Prasad ([email protected]) wrote:
This patch converts the tracing mechanism of Preempt RCU boosting into markers. The handler functions for these markers are included inside
rcupreempt_trace.cand will be included only whenPREEMPT_RCU_BOOSTis chosen.Signed-off-by K.Prasad <[email protected]>--- include/linux/rcupreempt_trace.h | 46 ++++++++ kernel/rcupreempt-boost.c | 211 ++++----------------------------------- kernel/rcupreempt_trace.c | 183 +++++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+), 189 deletions(-)Index:
linux-2.6.24-rc2-rt1.MARKER_PATCHES_NEW/include/linux/rcupreempt_trace.h
===================================================================include/linux/rcupreempt_trace.h +15
-0@@ -102,5 +102,51 @@ extern int rcupreempt_flip_flag(int cpu)extern int rcupreempt_mb_flag (int cpu);extern char *rcupreempt_try_flip_state_name (void ); +#ifdef CONFIG_PREEMPT_RCU_BOOST +struct preempt_rcu_boost_trace { +unsigned long rbs_stat_task_boost_called; +unsigned long rbs_stat_task_boosted; +unsigned long rbs_stat_boost_called; +unsigned long rbs_stat_try_boost; +unsigned long rbs_stat_boosted; +unsigned long rbs_stat_unboost_called; +unsigned long rbs_stat_unboosted; +unsigned long rbs_stat_try_boost_readers; +unsigned long rbs_stat_boost_readers; +unsigned long rbs_stat_try_unboost_readers; +unsigned long rbs_stat_unboost_readers; +unsigned long rbs_stat_over_taken; +};include/linux/rcupreempt_trace.h extern int rcupreempt_flip_flag(int cpu) · line 129
Quoted 10 lines
include/linux/rcupreempt_trace.h +10
-0+ +#define DEFINE_PREEMPT_RCU_BOOST_MARKER_HANDLER (preempt_rcu_boost_var) \ +void preempt_rcu_boost_var##_callback(const struct marker *mdata, \ + void *private_data, const char *format, ...) \ +{ \ + va_list ap; \ + int cpu; \ + struct preempt_rcu_boost_trace *boost_trace; \ + va_start(ap, format); \ + cpu = va_arg(ap, typeof(unsigned int)); \(sorry for late response, I just came back from vacation)
Quoted 3 lines
Instead of passing the cpu id as a marker parameter, why don't you simply use
smp_processor_id()right here ?Quoted 7 lines
Agreed.
Also in patch 1, in cases where we're using markers in place of
RCU_TRACE_ME(), we need not pass the parameter and simply usesmp_processor_id().It's only the
RCU_TRACE_RDP()that requires cpuid to be passed.And it would help if you declare the format string with something like :
"cpuid %u" instead of "%u"
Quoted 7 lines
This way, if a generic tracer like LTTng dumps the traces and later a plugin in an analysis tool like LTTV have to hook on a specific field of your event, it can access it by specifying the marker name and field name. And even if fields are added to this event, it won't break compatibility as long as this field is there and have the same name.
Mathieu
Hi Mathieu,
Thanks for your suggestions.
Even in functions such as rcu_check_callbacks_rt, rcu_advance_callbacks_rt and __rcu_advance_callbacks, where a call to RCU_TRACE_RDP() is made, the 'cpu' variable is populated at an early stage with the
return value of smp_processor_id().
__rcu_advance_callbacks() <-- rcu_check_callbacks_rt() <-- rcu_check_callbacks() <-- update_process_times().
update_process_times() [if understood correctly, is called at timer interrupt level] populates the 'cpu' with smp_processor_id().
There doesn't seem to be a need to pass down the 'cpu' parameter for the above RCU functions including respective trace_mark() calls.
I am re-sending the patches with your suggested changes in subsequent mails.
Thanks,
K.Prasad
Quoted 22 lines
Quoted 17 lines
Thanks and Regards
gautham.Quoted 4 lines
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68Signature · 5 lines
Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!"Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
cover Markers Implementation for RCU Tracing - Ver II
In reply to Mathieu DesnoyersHi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.
These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Patch [1/2] - Patch that converts the Preempt RCU tracing in rcupreempt.c into markers.
Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in rcupreempt-boost.c into markers.
Thanks,
K.Prasad
([email protected])
Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II
Quoted 14 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.Thanks,
K.Prasad
([email protected])
The correct marker annotation for "no arguments" is MARK_NOARGS.
--- kernel/rcupreempt-boost.c | 24 ++++++++++++------------ kernel/rcupreempt.c | 42 +++++++++++++++++++++--------------------- 2 files changed, 33 insertions(+), 33 deletions(-)
kernel/rcupreempt-boost.c +12 -12
@@ -57,10 +57,10 @@ static void rcu_boost_task(struct task_sWARN_ON (!irqs_disabled ());WARN_ON_SMP (!spin_is_locked (&task->pi_lock)); -trace_mark (task_boost_called,"NULL" ); +trace_mark (task_boost_called,MARK_NOARGS );if (task->rcu_prio < task->prio) { -trace_mark (task_boosted,"NULL" ); +trace_mark (task_boosted,MARK_NOARGS );task_setprio (task, task->rcu_prio); } }
@@ -84,7 +84,7 @@ void __rcu_preempt_boost(void)WARN_ON (!current->rcu_read_lock_nesting); -trace_mark (boost_called,"NULL" ); +trace_mark (boost_called,MARK_NOARGS );/* check to see if we are already boosted */ if (unlikely (rcu_is_boosted (curr)))
@@ -102,7 +102,7 @@ void __rcu_preempt_boost(void) curr->rcub_rbdp = rbd; -trace_mark (try_boost,"NULL" ); +trace_mark (try_boost,MARK_NOARGS ); prio =rt_mutex_getprio (curr);
@@ -111,7 +111,7 @@ void __rcu_preempt_boost(void)if (prio <= rbd->rbs_prio) goto out; -trace_mark (boosted,"NULL" ); +trace_mark (boosted,MARK_NOARGS ); curr->rcu_prio = rbd->rbs_prio;rcu_boost_task (curr);
@@ -136,7 +136,7 @@ void __rcu_preempt_unboost(void)int prio;unsigned long flags; -trace_mark (unboost_called,"NULL" ); +trace_mark (unboost_called,MARK_NOARGS );/* if not boosted, then ignore */ if (likely (!rcu_is_boosted (curr)))
@@ -174,7 +174,7 @@ void __rcu_preempt_unboost(void)list_del_init (&curr->rcub_entry); -trace_mark (unboosted,"NULL" ); +trace_mark (unboosted,MARK_NOARGS ); curr->rcu_prio =MAX_PRIO ;
@@ -235,7 +235,7 @@ static int __rcu_boost_readers(struct rc* Another task may have taken over. */ if (curr->rcu_preempt_counter != rcu_boost_counter) { -trace_mark (over_taken,"NULL" ); +trace_mark (over_taken,MARK_NOARGS );return 1 ; }
@@ -266,7 +266,7 @@ void rcu_boost_readers(void) prio =rt_mutex_getprio (curr); -trace_mark (try_boost_readers,"NULL" ); +trace_mark (try_boost_readers,MARK_NOARGS );if (prio >= rcu_boost_prio) {/* already boosted */
@@ -276,7 +276,7 @@ void rcu_boost_readers(void) rcu_boost_prio = prio; -trace_mark (boost_readers,"NULL" ); +trace_mark (boost_readers,MARK_NOARGS );/* Flag that we are the one to unboost */ curr->rcu_preempt_counter = ++rcu_boost_counter;
@@ -309,12 +309,12 @@ void rcu_unboost_readers(void)spin_lock_irqsave (&rcu_boost_wake_lock, flags); -trace_mark (try_unboost_readers,"NULL" ); +trace_mark (try_unboost_readers,MARK_NOARGS );if (current->rcu_preempt_counter != rcu_boost_counter) goto out; -trace_mark (unboost_readers,"NULL" ); +trace_mark (unboost_readers,MARK_NOARGS );/* * We could also put in something that
kernel/rcupreempt.c +21 -21
146 lines not on this page · raw message
Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II
In reply to K. PrasadQuoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.
I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void *call_private,
const char *fmt, ...)
{
va_list args;
char ptype;
/*
* rcu_read_lock does two things : disabling preemption to make sure the
* teardown of the callbacks can be done correctly when they are in
* modules and they insure RCU read coherency.
*/
rcu_read_lock();
preempt_disable();
...Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Jan
PS: You will run into this issue if you try to marry latest -rt with latest LTTng. Straightforward workaround is to comment-out any RCU trace_mark occurrences.
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II
Quoted 39 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...
Quoted 3 lines
Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
If you substitute synchronize_sched() for synchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.
If tracing is using call_rcu(), we will need to add a call_sched() or some such.
Thanx, Paul
Quoted 9 lines
Jan
PS: You will run into this issue if you try to marry latest -rt with latest LTTng. Straightforward workaround is to comment-out any RCU
trace_markoccurrences.Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II
Quoted 44 lines · * Paul E. McKenney ([email protected]) wrote:
Quoted 39 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Quoted 3 lines
Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 7 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.
Yes, I use call_rcu, so I guess a call_sched would be useful here.
Mathieu
Quoted 11 lines
Thanx, Paul
Quoted 9 lines
Jan
PS: You will run into this issue if you try to marry latest -rt with latest LTTng. Straightforward workaround is to comment-out any RCU
trace_markoccurrences.Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
In reply to Paul E. McKenneyQuoted 43 lines · Paul E. McKenney
Quoted 38 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Quoted 3 lines
Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.
You mean something like "#define call_sched call_rcu_classic"?
I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with PREEMPT_RCU_BOOST (hit probe inside rq-lock protected region => rcu_read_unlock triggers unboost => stuck on rq_lock :( ).
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Re: Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
Quoted 51 lines · * Jan Kiszka ([email protected]) wrote:
Quoted 43 lines · Paul E. McKenney
Quoted 38 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Quoted 3 lines
Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.
Quoted 7 lines
You mean something like "#define
call_schedcall_rcu_classic"?I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with
PREEMPT_RCU_BOOST(hit probe inside rq-lock protected region =>rcu_read_unlocktriggers unboost => stuck onrq_lock:( ).
Yep, ok, let's do this :
in include/linux/rcupdate.h
#ifndef PREEMPT_RT
#define call_sched call_rcu
#else
#define call_sched call_rcu_classic
#endif
And I'll adapt the markers accordingly.
Mathieu
Quoted 5 lines
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Re: Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
Quoted 64 lines · Mathieu Desnoyers
Quoted 51 lines · * Jan Kiszka ([email protected]) wrote:
Quoted 43 lines · Paul E. McKenney
Quoted 38 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Quoted 3 lines
Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.Quoted 7 lines
You mean something like "#define
call_schedcall_rcu_classic"?I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with
PREEMPT_RCU_BOOST(hit probe inside rq-lock protected region =>rcu_read_unlocktriggers unboost => stuck onrq_lock:( ).Yep, ok, let's do this :
in
include/linux/rcupdate.h
Quoted 7 lines
#ifndef
PREEMPT_RT
#definecall_schedcall_rcu
#else
#definecall_schedcall_rcu_classic
#endifAnd I'll adapt the markers accordingly.
Good point, this will indeed work for 2.6.24-rt1!
Will need to do a bit more for 2.6.25-rc1. My current thought is to provide a kernel thread that loops over the CPUs, advancing/invoking per-CPU lists of callbacks as it does so. Then call_sched() would simply enqueue its callback on the current CPU's next list.
Thanx, Paul
Re: Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
Quoted 71 lines · * Paul E. McKenney ([email protected]) wrote:
Quoted 70 lines · Mathieu Desnoyers
Quoted 51 lines · * Jan Kiszka ([email protected]) wrote:
Quoted 43 lines · Paul E. McKenney
Quoted 38 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Quoted 3 lines
Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.Quoted 7 lines
You mean something like "#define
call_schedcall_rcu_classic"?I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with
PREEMPT_RCU_BOOST(hit probe inside rq-lock protected region =>rcu_read_unlocktriggers unboost => stuck onrq_lock:( ).Yep, ok, let's do this :
in
include/linux/rcupdate.h#ifndef
PREEMPT_RT
#definecall_schedcall_rcu
#else
#definecall_schedcall_rcu_classic
#endif
Quoted 9 lines
Quoted 1 line
And I'll adapt the markers accordingly.
Good point, this will indeed work for 2.6.24-rt1!
Will need to do a bit more for 2.6.25-rc1. My current thought is to provide a kernel thread that loops over the CPUs, advancing/invoking per-CPU lists of callbacks as it does so. Then
call_sched()would simply enqueue its callback on the current CPU's next list.
I just noticed that my multiple probes support patch, that just got into mainline, does not include the fixes I did you asked for (which includes protecting with rcu_read_lock). I guess Andrew did not notice the patch. Therefore, I think we'll need to address this pretty soon to make the markers play nicely with CONFIG_PREEMPT_RCU.
Mathieu
Quoted 1 line
Thanx, Paul
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Re: Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
In reply to Jan KiszkaQuoted 44 lines · Jan Kiszka
Quoted 43 lines · Paul E. McKenney
Quoted 41 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 8 lines
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.You mean something like "#define
call_schedcall_rcu_classic"?
This would work for Classic RCU. For preemptible RCU, a bit more work is needed.
Quoted 4 lines
I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with
PREEMPT_RCU_BOOST(hit probe inside rq-lock protected region =>rcu_read_unlocktriggers unboost => stuck onrq_lock:( ).
OK, good to know. Guess we need a call_sched() for -rt and for preemptible RCU sooner rather than later...
Thanx, Paul
Re: Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
Quoted 57 lines · * Paul E. McKenney ([email protected]) wrote:
Quoted 44 lines · Jan Kiszka
Quoted 43 lines · Paul E. McKenney
Quoted 41 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 8 lines
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.You mean something like "#define
call_schedcall_rcu_classic"?This would work for Classic RCU. For preemptible RCU, a bit more work is needed.
Quoted 8 lines
Quoted 4 lines
I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with
PREEMPT_RCU_BOOST(hit probe inside rq-lock protected region =>rcu_read_unlocktriggers unboost => stuck onrq_lock:( ).OK, good to know. Guess we need a
call_sched()for -rt and for preemptible RCU sooner rather than later...
Yup, I would need call_sched() and sched_barrier().
Thanks,
Mathieu
Quoted 1 line
Thanx, Paul
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Re: Markers: multi-probe locking fun (was: Re: [PATCH 0/2] Markers Implementation for RCU Tracing - Ver II)
Quoted 63 lines · Mathieu Desnoyers
Quoted 62 lines · * Paul E. McKenney ([email protected]) wrote:
Quoted 44 lines · Jan Kiszka
Quoted 43 lines · Paul E. McKenney
Quoted 41 lines · Jan Kiszka
Quoted 11 lines · K. Prasad
Hi Ingo,
Please accept these patches into the rt tree which convert the
existing RCU tracing mechanism for Preempt RCU and RCU Boost into markers.These patches are based upon the 2.6.24-rc5-rt1 kernel tree.
Along with marker transition, the RCU Tracing infrastructure has also been modularised to be built as a kernel module, thereby enabling runtime changes to the RCU Tracing infrastructure.
Quoted 6 lines
Patch [1/2] - Patch that converts the Preempt RCU tracing in
rcupreempt.cinto markers.Patch [1/2] - Patch that converts the Preempt RCU Boost tracing in
rcupreempt-boost.cinto markers.I have a technical problem with marker-based RCU tracing: It causes nasty recursions with latest multi-probe marker patches (sorry, no link at hand, can be found in latest LTTng, maybe also already in -mm). Those patches introduce a marker probe trampoline like this:
void marker_probe_cb(const struct marker *mdata, void
*call_private,const char *fmt, ...) { va_list args; char ptype; /* * rcu_read_lock does two things : disabling preemption to make sure the * teardown of the callbacks can be done correctly when they are in * modules and they insure RCU read coherency. */ rcu_read_lock(); preempt_disable(); ...Can we do multi-probe with pure preempt_disable/enable protection? I guess it's fine with classic RCU, but what about preemptible RCU? Any suggestion appreciated!
Quoted 8 lines
Quoted 6 lines
If you substitute
synchronize_sched()forsynchronize_rcu(), this should work fine. Of course, this approach would cause RCU tracing to degrade latencies somewhat in -rt.If tracing is using
call_rcu(), we will need to add acall_sched()or some such.You mean something like "#define
call_schedcall_rcu_classic"?This would work for Classic RCU. For preemptible RCU, a bit more work is needed.
Quoted 4 lines
I just learned that there is another reason for killing rcu_read_lock&friends from the marker probes: It can deadlock on -rt with
PREEMPT_RCU_BOOST(hit probe inside rq-lock protected region =>rcu_read_unlocktriggers unboost => stuck onrq_lock:( ).
Quoted 5 lines
Quoted 3 lines
OK, good to know. Guess we need a
call_sched()for -rt and for preemptible RCU sooner rather than later...Yup, I would need
call_sched()andsched_barrier().
Ah, good point, kprobes does indeed use modules!
Thanx, Paul
PATCH 1/2 Markers Implementation for RCU Preempt Tracing - Ver II
In reply to Mathieu DesnoyersThis patch converts Preempt RCU Tracing code infrastructure to implement markers.
- The rcupreempt_trace structure has been moved to the tracing infrastructure and de-linked from the rcupreempt.c code. A per-cpu instance of rcupreempt_trace structure will be maintained in rcupreempt_trace.c
- The above change also renders a few macro definitions unused (such as RCU_TRACE_CPU, RCU_TRACE_ME and RCU_TRACE_RDP) which have been removed.
- Some of the helper functions in rcupreempt.c which were exported only when CONFIG_RCU_TRACE was set are now exported unconditionally. These functions operate on per-cpu variables that are used both by the RCU and RCU Tracing code. The changes help in making RCU Tracing code operate as a kernel module also.
- The references to rcupreempt-boost tracing in the module initialisation and cleanup have been removed here to enable kernel build, but will be brought in after enclosing them inside a #ifdef CONFIG_PREEMPT_RCU_BOOST.
--- include/linux/rcupreempt.h | 10 ---- include/linux/rcupreempt_trace.h | 50 ++++++++++++------------ kernel/Kconfig.preempt | 7 +-- kernel/rcupreempt.c | 77 ++++++++++---------------------------- kernel/rcupreempt_trace.c | 79 +++++++++++++++++++++++++++++++++++++-- 5 files changed, 125 insertions(+), 98 deletions(-)
include/linux/rcupreempt.h +0 -10
@@ -96,16 +96,6 @@ extern int rcu_pending_rt(int cpu);struct softirq_action ;extern void rcu_process_callbacks_rt (struct softirq_action *unused); -#ifdef CONFIG_RCU_TRACE -struct rcupreempt_trace ; -extern int *rcupreempt_flipctr (int cpu); -extern long rcupreempt_data_completed (void ); -extern int rcupreempt_flip_flag (int cpu); -extern int rcupreempt_mb_flag (int cpu); -extern char *rcupreempt_try_flip_state_name (void ); -extern struct rcupreempt_trace *rcupreempt_trace_cpu (int cpu); -#endif -struct softirq_action ;#ifdef CONFIG_NO_HZ
include/linux/rcupreempt_trace.h +25 -25
57 lines not on this page · raw message
kernel/rcupreempt.c +21 -56
216 lines not on this page · raw message
kernel/rcupreempt_trace.c +75 -4
120 lines not on this page · raw message
kernel/Kconfig.preempt +4 -3
@@ -172,14 +172,15 @@ config PREEMPT_RCU_BOOST possible OOM problems. config RCU_TRACE - bool "Enable tracing for RCU - currently stats in debugfs" + tristate "Enable tracing for RCU - currently stats in debugfs" select DEBUG_FS - default y + select MARKERS + default m help This option provides tracing in RCU which presents stats in debugfs for debugging RCU implementation. - Say Y here if you want to enable RCU tracing + Say Y/M here if you want to enable RCU tracing in-kernel/module. Say N if you are unsure. config SPINLOCK_BKL
PATCH 2/2 Markers Implementation for Preempt RCU Boost Tracing - Ver II
In reply to Mathieu DesnoyersThis patch converts the tracing mechanism of Preempt RCU boosting into markers. The handler functions for these markers are included inside rcupreempt_trace.c and will be included only when PREEMPT_RCU_BOOST is chosen.
--- include/linux/rcupreempt_trace.h | 40 +++++++ kernel/rcupreempt-boost.c | 211 ++++----------------------------------- kernel/rcupreempt_trace.c | 183 +++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+), 189 deletions(-)
include/linux/rcupreempt_trace.h +40 -0
@@ -95,5 +95,45 @@ extern int rcupreempt_flip_flag(int cpu)extern int rcupreempt_mb_flag (int cpu);extern char *rcupreempt_try_flip_state_name (void ); +#ifdef CONFIG_PREEMPT_RCU_BOOST +struct preempt_rcu_boost_trace { +unsigned long rbs_stat_task_boost_called; +unsigned long rbs_stat_task_boosted; +unsigned long rbs_stat_boost_called; +unsigned long rbs_stat_try_boost; +unsigned long rbs_stat_boosted; +unsigned long rbs_stat_unboost_called; +unsigned long rbs_stat_unboosted; +unsigned long rbs_stat_try_boost_readers; +unsigned long rbs_stat_boost_readers; +unsigned long rbs_stat_try_unboost_readers; +unsigned long rbs_stat_unboost_readers; +unsigned long rbs_stat_over_taken; +}; + +#define DEFINE_PREEMPT_RCU_BOOST_MARKER_HANDLER (preempt_rcu_boost_var) \ +void preempt_rcu_boost_var##_callback(const struct marker *mdata, \ + void *private_data, const char *format, ...) \ +{ \ + struct preempt_rcu_boost_trace *boost_trace; \ + boost_trace = (&per_cpu(boost_trace_data, smp_processor_id())); \ + boost_trace->rbs_stat_##preempt_rcu_boost_var++; \ +} + +struct preempt_rcu_boost_probe { +const char *name; +const char *format; +marker_probe_func *probe_func; +}; + +#define INIT_PREEMPT_RCU_BOOST_PROBE (preempt_rcu_boost_probe_worker) \ +{ \ + .name = __stringify(preempt_rcu_boost_probe_worker), \ + .probe_func = preempt_rcu_boost_probe_worker##_callback \ +} + +extern int read_rcu_boost_prio (void ); +#endif /* CONFIG_PREEMPT_RCU_BOOST */ +#endif /* __KERNEL__ */ #endif /* __LINUX_RCUPREEMPT_TRACE_H */
kernel/rcupreempt_trace.c +183 -0
207 lines not on this page · raw message
kernel/rcupreempt-boost.c +22 -189
292 lines not on this page · raw message
__get_cpu_var() called from a preempt-unsafe context in __rcu_preempt_unboost() ?
In reply to Mathieu DesnoyersHi Paul, Steve,
This is with reference to the preemptible rcu implementation in 2.6.24-rc5-rt1.
In __rcu_read_unlock(), we call __rcu_preempt_unboost() to unboost a task's priority which had been bumped up when it was preempted out while the rcu_read section.
The code path is as follows
__rcu_read_unlock()
| |--> local_irq_restore(oldirq); | |--> __rcu_preempt_unboost(); | |--> rcu_trace_boost_unboost_called(RCU_BOOST_ME)
where RCU_BOOST_ME is #defined as &__get_cpu_var(rcu_boost_data).
Is calling __get_cpu_var() safe in this context, since we've already enabled the local interrupts and we're not in a preempt_disabled() ?
Thanks and Regards
gautham.
Signature · 5 lines
Gautham R Shenoy Linux Technology Center IBM India. "Freedom comes with a price tag of responsibility, which is still a bargain, because Freedom is priceless!"
Steven Rostedt
reviewerRe: __get_cpu_var() called from a preempt-unsafe context in __rcu_preempt_unboost() ?
Quoted 8 lines · Gautham R Shenoy
| |--> rcu_trace_boost_unboost_called(RCU_BOOST_ME)where
RCU_BOOST_MEis #defined as&__get_cpu_var(rcu_boost_data).Is calling
__get_cpu_var()safe in this context, since we've already enabled the local interrupts and we're not in apreempt_disabled()?
Probably not, but the worst that can happen is that we corrupt the trace counter, and miss an increment. I'll see if I can fix that, but it should be quite harmless wrt the stability of the system.
Thanks for pointing that out.
-- Steve