[RFC] printk's sync mode for ftrace_dump()

[RFC] printk's sync mode for ftrace_dump()

[Sebastian Andrzej Siewior]

ftrace_dump() is used for instance by alt-sysrq-z / sysrq_ftrace_dump().
The function itself dumps the whole trace buffer in an irq-off section so
no need to talk about max latencies unless this is going to change.

The output on the serial is more or less brief and starts with
    "** 397774 printk messages dropped **"
and so I do see only the end of it. Might be okay.
Any reason not to use the sync mode + atomic console while ftrace_dump()
is in progress?

Sebastian

Re: [RFC] printk's sync mode for ftrace_dump()

[John Ogness]

On 2021-10-29, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> ftrace_dump() is used for instance by alt-sysrq-z / sysrq_ftrace_dump().
> The function itself dumps the whole trace buffer in an irq-off section so
> no need to talk about max latencies unless this is going to change.
>
> The output on the serial is more or less brief and starts with
>     "** 397774 printk messages dropped **"
> and so I do see only the end of it. Might be okay.
> Any reason not to use the sync mode + atomic console while ftrace_dump()
> is in progress?

Since latencies are not a concern, I see no reason not to use
sync+atomic.

However, it does put us into the situation of beginning to define
non-emergency scenarios where sync+atomic is used. That rabbit hole
might go quite deep. I wonder if it might be better to introduce a new
sysrq to toggle sync mode.

John

Re: [RFC] printk's sync mode for ftrace_dump()

[Sebastian Andrzej Siewior]

On 2021-10-29 13:18:33 [+0206], John Ogness wrote:
> On 2021-10-29, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> > ftrace_dump() is used for instance by alt-sysrq-z / sysrq_ftrace_dump().
> > The function itself dumps the whole trace buffer in an irq-off section so
> > no need to talk about max latencies unless this is going to change.
> >
> > The output on the serial is more or less brief and starts with
> >     "** 397774 printk messages dropped **"
> > and so I do see only the end of it. Might be okay.
> > Any reason not to use the sync mode + atomic console while ftrace_dump()
> > is in progress?
> 
> Since latencies are not a concern, I see no reason not to use
> sync+atomic.
> 
> However, it does put us into the situation of beginning to define
> non-emergency scenarios where sync+atomic is used. That rabbit hole
> might go quite deep. I wonder if it might be better to introduce a new
> sysrq to toggle sync mode.

This can be triggered not only by sysrq. panic-on-oops seems to do the
right thing. It seems that it makes sense to have this sync-mode always
while ftrace_dump() is running.
But then any sysrq-request has this limitation so maybe any sysrq should
force sync-mode. But the sysrq request itself could be redirected into
printing thread unless the system has some kind of scheduling problem.
The printing request may not disable interrupts for the whole time as it
is the case with the trace buffer so it would be possible to wait until
printing is done if done from the printing thread.
An additional sysrq to trigger the sync mode seems to be problematic
because people might not be aware of it. Also, what is it point of
sysrq-t if you see the last few entries?

As per rabbit hole, I'm bringing my case to the high council.
I wouldn't mind if using the sync-mode would taint the kernel..

> John

Sebastian

Re: [RFC] printk's sync mode for ftrace_dump()

[Steven Rostedt]

On Tue, 2 Nov 2021 13:20:48 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> This can be triggered not only by sysrq. panic-on-oops seems to do the
> right thing. It seems that it makes sense to have this sync-mode always
> while ftrace_dump() is running.
> But then any sysrq-request has this limitation so maybe any sysrq should
> force sync-mode. But the sysrq request itself could be redirected into
> printing thread unless the system has some kind of scheduling problem.
> The printing request may not disable interrupts for the whole time as it
> is the case with the trace buffer so it would be possible to wait until
> printing is done if done from the printing thread.
> An additional sysrq to trigger the sync mode seems to be problematic
> because people might not be aware of it. Also, what is it point of
> sysrq-t if you see the last few entries?

Looking at ftrace_dump(), I'm not sure we need interrupts disabled. I think
that was there because it was created for panic only, but today I believe
the ring buffer is more robust, and it does disable everything before
reading the ring buffer. We may want to add some logic to make sure the
user doesn't re-enable tracing while the dump is happening. Or maybe we
don't care. If the user wants the dump to never end, let them re-enable it
;-)

I can write a patch to remove the irq disabling, or at least (to be on the
safe side) to enable it before entering the loop.

-- Steve