fche changed the topic of #systemtap to: http://sourceware.org/systemtap; email systemtap@sourceware.org if answers here not timely, conversations may be logged
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
khaled has quit [Quit: Konversation terminated!]
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
hpt has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek088_ has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek088_ has quit [Remote host closed the connection]
<kerneltoast>
fche, so i'm trying to pass the context to every print statement
<kerneltoast>
it's all fine and dandy until we get to _stp_vlog inside runtime/linux/io.c
<kerneltoast>
_stp_vlog is used by _stp_error, _stp_warn, _stp_softerror, _stp_dbug
<kerneltoast>
and _stp_vlog needs the context pointer
<kerneltoast>
which makes everything suddenly very messy
<kerneltoast>
because we'd have to somehow get a context pointer to every [_stp_error, _stp_warn, _stp_softerror, _stp_dbug] call
<kerneltoast>
ideas on what to do about that?
<fche>
hmmmmmmmmmmmm
<kerneltoast>
agreed.
<fche>
so even if we were to extend all those apis to pass the context around, we'd have to be willing to GRAB one at every entrypoint into the runtime, for all probe point APIs (perf, kprobes, tracepoints, everything)
<fche>
and our problem recently was that we couldn't safely (in light of reentrancy or whatever?) do the entryfn_get_context() at any context whatsoever?
<kerneltoast>
entryfn_get_context() is safe to do in any context, but it's not reentrant. our problem is that there is a lot of reentrant print usage, and instead of introducing a new lock to handle the reentrancy protection, you suggested leaning on the context
<fche>
yeah, that just means having to -get- a suitable context
<fche>
"but it's not reentrant"
<fche>
............................. ..... but it should be it should be it should be
<fche>
in the sense of safely blocking reentrancy
<kerneltoast>
blocking reentrancy with the context is hard though
<kerneltoast>
the next possible solution would be to get rid of the reentrancy dependencies within the print driver
<kerneltoast>
and adding back our own reentrancy lock
<fche>
blocking reentrancy is hard .... I'm sure we've treaded on this before, but ... why?
<kerneltoast>
because of _stp_vlog
<fche>
are we just using the wrong level of locking gadgetry to detect/prevent reentrancy safely?
<kerneltoast>
it's hard because of that evil function
<kerneltoast>
_stp_vlog
<fche>
I mean within the entryfn_get_context function
<kerneltoast>
ah well
<kerneltoast>
prints are allowed to be used outside of probes where the context is held
<kerneltoast>
such as in module_exit
<kerneltoast>
we would have to track down all this errant usage
<kerneltoast>
so there are prints that occur while the context isn't held
<kerneltoast>
the compiler could track all of those down for us if we pass the context pointer around
<kerneltoast>
you said you were semi certain that module_exit was the only place using prints without a context being held
<fche>
yeah other than dbug & pals
<kerneltoast>
my most recent non-invasive solution was to check if the context is held when printing and, if it isn't, acquire a reentrancy lock
<kerneltoast>
but that's redundant because our friend mr context can cover the reentrancy protection for us
<kerneltoast>
for the low price of our sanity
<kerneltoast>
i'm tempted to roll with that
<kerneltoast>
i.e., falling back to a reentrancy lock inside the print driver if the context isn't held
<kerneltoast>
thoughts?
<fche>
terminology nit:
<fche>
if context is not held, we'd take the context the normal way
<fche>
if the context is held, we'd take it "reentrantly" - i.e., just use it, assuming that the probe handler that grabbed it will keep the ref alive just fine (sort of kind of rcu like)
<kerneltoast>
yes but if the context isn't held i don't wanna grab it
<fche>
for the duration of an stp_vlog? why not?
<kerneltoast>
instead i'd use a spin lock local to the print driver
<kerneltoast>
because it makes semantics more annoying
<kerneltoast>
we'd have to track if the print driver grabbed a context or not
<fche>
hm yeah
<fche>
ok
<fche>
hmmmmmmmmmm
<fche>
ok I'll wander off and see if another Cunning Plan appears
<fche>
now as a separate matter
<kerneltoast>
hah
<fche>
how much would that switch to STP_BULKMODE on the runtime side help with all this?
<kerneltoast>
it has nothing to do with this
<kerneltoast>
:)
<kerneltoast>
the goal right now is to fix panics and data corruption caused by unchecked print reentrancy
<kerneltoast>
oh actually this does help with the bulkmode stuff
<kerneltoast>
it ensures that per-cpu log buffers are only ever modified while irqs are disabled
<kerneltoast>
which is a requirement for the userspace side of things
<kerneltoast>
so that userspace can't read out the log buffer while a print is writing to it
<fche>
userspace won't be running while an irq handler is running on that cpu
<kerneltoast>
hmm yeah and disabling irqs isn't going to provide our reentrancy protection
<kerneltoast>
guess i can change that to a preempt_disable
<kerneltoast>
oh nvm i should keep irqs disabled while printing
<kerneltoast>
so that only NMIs have the possibility of their prints getting dropped
<kerneltoast>
and then it helps with the bulkmode stuff because disabling irqs implies disabling preempt
<kerneltoast>
voila
<fche>
well anyway I'll wander off now so I don't get confused and pass on that virus
<fche>
see ya in oh 9 or so hours :-)
<kerneltoast>
sweet dreams
<kerneltoast>
maybe you'll find the elegant solution to all this in your subconscious
<fche>
will be dreaming of locks and semaphores and that annoying neighbour who keeps on interrupting
<fche>
NO PROMISES :-)
modem has quit [Read error: Connection reset by peer]
<kerneltoast>
Nathan Maxwell Irwin
<kerneltoast>
N.M.I.
modem has joined #systemtap
<kerneltoast>
the worst neighbor
<fche>
with an infinity of clones
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Ping timeout: 260 seconds]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Ping timeout: 272 seconds]
orivej has quit [Ping timeout: 264 seconds]
orivej has joined #systemtap
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Ping timeout: 265 seconds]
orivej has quit [Ping timeout: 272 seconds]
khaled has joined #systemtap
orivej has joined #systemtap
hpt has quit [Ping timeout: 260 seconds]
mjw has joined #systemtap
tromey has joined #systemtap
modem has quit [Ping timeout: 240 seconds]
modem has joined #systemtap
modem has quit [Ping timeout: 260 seconds]
modem has joined #systemtap
khaled has quit [Remote host closed the connection]
khaled has joined #systemtap
derek0883 has joined #systemtap
_whitelogger has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
<irker576>
systemtap: sultan systemtap.git:master * release-4.4-13-gbb25d64f7 / runtime/linux/runtime_context.h: runtime_context: synchronize _stp_context_stop more strictly
irker576 has joined #systemtap
<kerneltoast>
fche, i pushed a small change to fix the only nitpicks i saw with my un-RCU patch
<kerneltoast>
if we're lucky, it'll make the buildbots happy
<fche>
will report
<fche>
thanks
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
wcohen has quit [Read error: Connection reset by peer]
wcohen has joined #systemtap
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Read error: Connection reset by peer]
derek0883 has joined #systemtap
derek0883 has quit [Ping timeout: 240 seconds]
derek0883 has joined #systemtap
derek0883 has quit [Ping timeout: 240 seconds]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
derek0883 has quit [Remote host closed the connection]
derek0883 has joined #systemtap
mjw has quit [Quit: Leaving]
<fche>
kerneltoast, no final results yet
<fche>
but one failure (pr14546.exp on a rhel8 buildbot) that may or may not be related
derek0883 has quit [Ping timeout: 260 seconds]
<kerneltoast>
related to the change i pushed today or the un-RCU in general?
<fche>
not sure
<fche>
hm the rhel8 buildbot survived the un-rcu patch
<fche>
from last week
<fche>
rerunning the testsuite against the current git/master now
<kerneltoast>
it's running through the testsuite atm
<kerneltoast>
hoping this is all good so i can proceed to the bulkmode stuff
<fche>
can you elaborate on why _stp_print_lock can't be STP_DEF*'d ? I believe the intent there is to use raw spinlocks even on -rt kernels. Why is that a problem?
<kerneltoast>
because prints need to be reentrant
<fche>
sorry why?
<fche>
"to be reentrant" ?
<kerneltoast>
because the print api is heavily bootstrapped and some print functions are implemented using other print functions
<kerneltoast>
a prominent example is _stp_vsnprintf
<kerneltoast>
when the buffer given to _stp_vsnprintf is NULL, _stp_vsnprintf will use the print log buffer
<fche>
ok go on, still not seeing the reentrance
<kerneltoast>
_stp_printf uses _stp_vsnprintf, and _stp_vsnprintf needs to acquire _stp_print_lock when the buffer given to it is NULL, which _stp_printf does
<kerneltoast>
then _stp_printf is nested in a bunch of places where the log buffer is manually accessed with _stp_reserve_bytes
<kerneltoast>
and then there are nested _stp_error usages as well, such as in _stp_vsnprintf itself
<fche>
eww, doesn't seem as though stp_vsnprintf should make any error calls that could conceivably result in infinite recursion
<kerneltoast>
there's no recursion though
<fche>
what is the lowest level function that talks straight to the data structures that require concurrency control
<kerneltoast>
btw, the STP_DEF* for rw locks doesn't affect the trylock api
<kerneltoast>
and we're only ever trylocking _stp_print_lock
derek0883 has joined #systemtap
<kerneltoast>
fche, i hit a deadlock while the testsuite was running on that patch. but the deadlock is unrelated to the patch