SysRP-P is not all that useful on SMP systems, since the sysrq
irq rarely ends up on the CPU that we actually want to investigate.
This patch extends sysrq-p to print a backtrace for every CPU,
not just the lucky one that gets the sysrq irq. With this patch,
"echo p > /proc/sysrq-trigger" does something useful.
Signed-off-by: Rik van Riel <[email protected]>
diff -up linux-2.6.25-rc3-mm1/drivers/char/sysrq.c.multicpu linux-2.6.25-rc3-mm1/drivers/char/sysrq.c
--- linux-2.6.25-rc3-mm1/drivers/char/sysrq.c.multicpu 2008-03-09 20:22:17.000000000 -0400
+++ linux-2.6.25-rc3-mm1/drivers/char/sysrq.c 2008-03-09 21:54:02.000000000 -0400
@@ -196,11 +196,29 @@ static struct sysrq_key_op sysrq_showloc
#define sysrq_showlocks_op (*(struct sysrq_key_op *)0)
#endif
+static DEFINE_SPINLOCK(show_lock);
+static void showacpu(void *dummy)
+{
+ struct pt_regs *regs = get_irq_regs();
+
+ spin_lock(&show_lock);
+ printk("CPU%d:\n", smp_processor_id());
+ show_stack(NULL, NULL);
+ spin_unlock(&show_lock);
+}
+static void sysrq_showregs_othercpus(struct work_struct *dummy)
+{
+ smp_call_function(showacpu, NULL, 0, 0);
+}
+static DECLARE_WORK(sysrq_showregs, sysrq_showregs_othercpus);
static void sysrq_handle_showregs(int key, struct tty_struct *tty)
{
struct pt_regs *regs = get_irq_regs();
- if (regs)
+ if (regs) {
+ printk("CPU%d:\n", smp_processor_id());
show_regs(regs);
+ }
+ schedule_work(&sysrq_showregs);
}
static struct sysrq_key_op sysrq_showregs_op = {
.handler = sysrq_handle_showregs,
On Sun, 9 Mar 2008 22:14:58 -0400 Rik van Riel <[email protected]> wrote:
> SysRP-P is not all that useful on SMP systems, since the sysrq
> irq rarely ends up on the CPU that we actually want to investigate.
>
> This patch extends sysrq-p to print a backtrace for every CPU,
> not just the lucky one that gets the sysrq irq. With this patch,
> "echo p > /proc/sysrq-trigger" does something useful.
>
> Signed-off-by: Rik van Riel <[email protected]>
>
> diff -up linux-2.6.25-rc3-mm1/drivers/char/sysrq.c.multicpu linux-2.6.25-rc3-mm1/drivers/char/sysrq.c
> --- linux-2.6.25-rc3-mm1/drivers/char/sysrq.c.multicpu 2008-03-09 20:22:17.000000000 -0400
> +++ linux-2.6.25-rc3-mm1/drivers/char/sysrq.c 2008-03-09 21:54:02.000000000 -0400
> @@ -196,11 +196,29 @@ static struct sysrq_key_op sysrq_showloc
> #define sysrq_showlocks_op (*(struct sysrq_key_op *)0)
> #endif
>
> +static DEFINE_SPINLOCK(show_lock);
> +static void showacpu(void *dummy)
> +{
> + struct pt_regs *regs = get_irq_regs();
> +
> + spin_lock(&show_lock);
> + printk("CPU%d:\n", smp_processor_id());
> + show_stack(NULL, NULL);
> + spin_unlock(&show_lock);
> +}
> +static void sysrq_showregs_othercpus(struct work_struct *dummy)
> +{
> + smp_call_function(showacpu, NULL, 0, 0);
> +}
> +static DECLARE_WORK(sysrq_showregs, sysrq_showregs_othercpus);
> static void sysrq_handle_showregs(int key, struct tty_struct *tty)
> {
> struct pt_regs *regs = get_irq_regs();
> - if (regs)
> + if (regs) {
> + printk("CPU%d:\n", smp_processor_id());
> show_regs(regs);
> + }
> + schedule_work(&sysrq_showregs);
> }
> static struct sysrq_key_op sysrq_showregs_op = {
> .handler = sysrq_handle_showregs,
Doesn't everyone have a copy of this somewhere? ;)
However it does have the downside that info can scroll away on large cpu
counts. Maybe it should be a new sysrq command?
On Sun, 9 Mar 2008 22:47:59 -0700
Andrew Morton <[email protected]> wrote:
> On Sun, 9 Mar 2008 22:14:58 -0400 Rik van Riel <[email protected]> wrote:
>
> > SysRP-P is not all that useful on SMP systems, since the sysrq
> > irq rarely ends up on the CPU that we actually want to investigate.
> Doesn't everyone have a copy of this somewhere? ;)
Yes, but the old version of the patch calls on_each_cpu from
sysrq context, which is illegal since it could cause a deadlock.
> However it does have the downside that info can scroll away on large cpu
> counts. Maybe it should be a new sysrq command?
It used to be sysrq-w in the patches that everybody has, but that
letter got taken for sysrq_showstate_blocked_op.
Only sysrq h, j, l, y and z are still free. H is needed for help,
leaving just j, l, y and z.
I can see your point about overflowing the screen, however just
sysrq-p seems like a waste to have because it will probably not
print anything useful on a large CPU system...
If you still want it to be a separate letter, just let me know
which one of the last four I should take.
--
All rights reversed.
Andrew Morton wrote:
>On Sun, 9 Mar 2008 22:14:58 -0400 Rik van Riel <[email protected]> wrote:
>
>
>
>>SysRP-P is not all that useful on SMP systems, since the sysrq
>>irq rarely ends up on the CPU that we actually want to investigate.
>>
>>This patch extends sysrq-p to print a backtrace for every CPU,
>>not just the lucky one that gets the sysrq irq. With this patch,
>>"echo p > /proc/sysrq-trigger" does something useful.
>>
>>Signed-off-by: Rik van Riel <[email protected]>
>>
>>diff -up linux-2.6.25-rc3-mm1/drivers/char/sysrq.c.multicpu linux-2.6.25-rc3-mm1/drivers/char/sysrq.c
>>--- linux-2.6.25-rc3-mm1/drivers/char/sysrq.c.multicpu 2008-03-09 20:22:17.000000000 -0400
>>+++ linux-2.6.25-rc3-mm1/drivers/char/sysrq.c 2008-03-09 21:54:02.000000000 -0400
>>@@ -196,11 +196,29 @@ static struct sysrq_key_op sysrq_showloc
>> #define sysrq_showlocks_op (*(struct sysrq_key_op *)0)
>> #endif
>>
>>+static DEFINE_SPINLOCK(show_lock);
>>+static void showacpu(void *dummy)
>>+{
>>+ struct pt_regs *regs = get_irq_regs();
>>+
>>+ spin_lock(&show_lock);
>>+ printk("CPU%d:\n", smp_processor_id());
>>+ show_stack(NULL, NULL);
>>+ spin_unlock(&show_lock);
>>+}
>>+static void sysrq_showregs_othercpus(struct work_struct *dummy)
>>+{
>>+ smp_call_function(showacpu, NULL, 0, 0);
>>+}
>>+static DECLARE_WORK(sysrq_showregs, sysrq_showregs_othercpus);
>> static void sysrq_handle_showregs(int key, struct tty_struct *tty)
>> {
>> struct pt_regs *regs = get_irq_regs();
>>- if (regs)
>>+ if (regs) {
>>+ printk("CPU%d:\n", smp_processor_id());
>> show_regs(regs);
>>+ }
>>+ schedule_work(&sysrq_showregs);
>> }
>> static struct sysrq_key_op sysrq_showregs_op = {
>> .handler = sysrq_handle_showregs,
>>
>>
>
>Doesn't everyone have a copy of this somewhere? ;)
>
Yes, but we use W instead of P. Dont you want to keep the old AlsSysrq
P and add
a new SPM version using a defferent letter ?
Larry
>
>However it does have the downside that info can scroll away on large cpu
>counts. Maybe it should be a new sysrq command?
>
>
>
On Mon, 10 Mar 2008 09:30:24 -0400 Rik van Riel <[email protected]> wrote:
> On Sun, 9 Mar 2008 22:47:59 -0700
> Andrew Morton <[email protected]> wrote:
>
> > On Sun, 9 Mar 2008 22:14:58 -0400 Rik van Riel <[email protected]> wrote:
> >
> > > SysRP-P is not all that useful on SMP systems, since the sysrq
> > > irq rarely ends up on the CPU that we actually want to investigate.
>
> > Doesn't everyone have a copy of this somewhere? ;)
>
> Yes, but the old version of the patch calls on_each_cpu from
> sysrq context, which is illegal since it could cause a deadlock.
My version did queue_work(), iirc, but I seem to have lost it.
I have an old patch here from Brice Goglin which does it via an NMI, but
that's arch-specific, I guess.
afaict, we _could_ implement an IPI call which isn't deadlockable (on x86,
at least) if we were to make it an asynchronous one.
Even if the hardware doesn't support queueing, this could still be done in
software via suitable locking and software queueing.
Whatever.
> > However it does have the downside that info can scroll away on large cpu
> > counts. Maybe it should be a new sysrq command?
>
> It used to be sysrq-w in the patches that everybody has, but that
> letter got taken for sysrq_showstate_blocked_op.
>
> Only sysrq h, j, l, y and z are still free. H is needed for help,
> leaving just j, l, y and z.
>
> I can see your point about overflowing the screen,
iirc, this was the reason for not doing this years ago. I don't know how
real that is.
> however just
> sysrq-p seems like a waste to have because it will probably not
> print anything useful on a large CPU system...
Yeah, sometimes it helps to keep hitting it until you get the right CPU,
but that rather depends on interrupt distribution.
It would be neat to suppress the trace for idle CPUs. I don't _think_
there's a need to display the trace for idle CPUs in sysrq-P?
> If you still want it to be a separate letter, just let me know
> which one of the last four I should take.
l :)
On Mon, 10 Mar 2008 09:24:13 -0700
Andrew Morton <[email protected]> wrote:
> > however just
> > sysrq-p seems like a waste to have because it will probably not
> > print anything useful on a large CPU system...
>
> Yeah, sometimes it helps to keep hitting it until you get the right CPU,
> but that rather depends on interrupt distribution.
>
> It would be neat to suppress the trace for idle CPUs. I don't _think_
> there's a need to display the trace for idle CPUs in sysrq-P?
>
> > If you still want it to be a separate letter, just let me know
> > which one of the last four I should take.
>
> l :)
OK, I'll do that. I will also change the show_lock to spinlock_irq_save
just in case sysrq-p gets invoked from multiple cpus simultaneously.
I'll send you the updated patch in a separate email, so the changelog
and commit logs do not have this discussion.