| Message ID | 1493301446.25766.276.camel@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> The existing verbose debug code doesn't build when enabled.
So why don't we convert all the DBG_VERBOSE() to pr_devel()?
If there's non-verbose debug that we think would be useful to
differentiate from verbose then those could be pr_debug() - which means
they'll be jump labelled off in most production kernels, but still able
to be enabled.
cheers
On Fri, 2017-04-28 at 13:07 +1000, Michael Ellerman wrote: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > > > The existing verbose debug code doesn't build when enabled. > > So why don't we convert all the DBG_VERBOSE() to pr_devel()? pr_devel provides a bunch of debug at init/setup/mask/unmask etc... but the system is still usable DBG_VERBOSE starts spewing stuff on every interrupt and eoi, the system is no longer usable. > If there's non-verbose debug that we think would be useful to > differentiate from verbose then those could be pr_debug() - which means > they'll be jump labelled off in most production kernels, but still able > to be enabled. Maybe... I don't like the giant "debug" switch accross the whole kernel, though. Ben.
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Fri, 2017-04-28 at 13:07 +1000, Michael Ellerman wrote: >> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: >> >> > The existing verbose debug code doesn't build when enabled. >> >> So why don't we convert all the DBG_VERBOSE() to pr_devel()? > > pr_devel provides a bunch of debug at init/setup/mask/unmask etc... but > the system is still usable OK so those could be converted to pr_debug(). > DBG_VERBOSE starts spewing stuff on every interrupt and eoi, the system > is no longer usable. And those could stay at pr_devel(), requiring a #define DEBUG and recompile to enable. >> If there's non-verbose debug that we think would be useful to >> differentiate from verbose then those could be pr_debug() - which means >> they'll be jump labelled off in most production kernels, but still able >> to be enabled. > > Maybe... I don't like the giant "debug" switch accross the whole > kernel, though. Not sure what you mean. You can enable pr_debug()s individually, by function, by module, by file, or for the whole kernel. To enable everything in xive you'd do: # echo 'file *xive* +p' > /sys/kernel/debug/dynamic_debug/control Or boot with: loglevel=8 dyndbg="file *xive* +p" cheers
On Fri, 2017-04-28 at 16:34 +1000, Michael Ellerman wrote: > > > If there's non-verbose debug that we think would be useful to > > > differentiate from verbose then those could be pr_debug() - which means > > > they'll be jump labelled off in most production kernels, but still able > > > to be enabled. > > > > Maybe... I don't like the giant "debug" switch accross the whole > > kernel, though. > > Not sure what you mean. You can enable pr_debug()s individually, by > function, by module, by file, or for the whole kernel. > > To enable everything in xive you'd do: > > # echo 'file *xive* +p' > /sys/kernel/debug/dynamic_debug/control > > Or boot with: loglevel=8 dyndbg="file *xive* +p" Ah that's new goodness I wasn't aware of. Anyway, I can spin that later, not planning on doing any work today ;-) Cheers, Ben.
From: Michael Ellerman > Sent: 28 April 2017 07:34 ... > Not sure what you mean. You can enable pr_debug()s individually, by > function, by module, by file, or for the whole kernel. It is sort of a shame that you can't turn pr_info() off in the same way. David
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c index 6a98efb..2305aa9 100644 --- a/arch/powerpc/sysdev/xive/common.c +++ b/arch/powerpc/sysdev/xive/common.c @@ -143,7 +143,6 @@ static u32 xive_scan_interrupts(struct xive_cpu *xc, bool just_peek) struct xive_q *q; prio = ffs(xc->pending_prio) - 1; - DBG_VERBOSE("scan_irq: trying prio %d\n", prio); /* Try to fetch */ irq = xive_read_eq(&xc->queue[prio], just_peek); @@ -171,12 +170,18 @@ static u32 xive_scan_interrupts(struct xive_cpu *xc, bool just_peek) } /* If nothing was found, set CPPR to 0xff */ - if (irq == 0) + if (irq == 0) { prio = 0xff; + DBG_VERBOSE("scan_irq(%d): nothing found\n", just_peek); + } else { + DBG_VERBOSE("scan_irq(%d): found irq %d prio %d\n", + just_peek, irq, prio); + } /* Update HW CPPR to match if necessary */ if (prio != xc->cppr) { - DBG_VERBOSE("scan_irq: adjusting CPPR to %d\n", prio); + DBG_VERBOSE("scan_irq(%d): adjusting CPPR %d->%d\n", + just_peek, xc->cppr, prio); xc->cppr = prio; out_8(xive_tima + xive_tima_offset + TM_CPPR, prio); } @@ -260,7 +265,7 @@ static unsigned int xive_get_irq(void) /* Scan our queue(s) for interrupts */ irq = xive_scan_interrupts(xc, false); - DBG_VERBOSE("get_irq: got irq 0x%x, new pending=0x%02x\n", + DBG_VERBOSE("get_irq: got irq %d new pending=0x%02x\n", irq, xc->pending_prio); /* Return pending interrupt if any */ @@ -282,7 +287,7 @@ static unsigned int xive_get_irq(void) static void xive_do_queue_eoi(struct xive_cpu *xc) { if (xive_scan_interrupts(xc, true) != 0) { - DBG_VERBOSE("eoi: pending=0x%02x\n", xc->pending_prio); + DBG_VERBOSE("eoi_irq: more pending !\n"); force_external_irq_replay(); } } @@ -327,11 +332,13 @@ void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd) in_be64(xd->eoi_mmio); else { eoi_val = xive_poke_esb(xd, XIVE_ESB_SET_PQ_00); - DBG_VERBOSE("eoi_val=%x\n", offset, eoi_val); + DBG_VERBOSE("hwirq 0x%x eoi_val=%x\n", hw_irq, eoi_val); /* Re-trigger if needed */ - if ((eoi_val & XIVE_ESB_VAL_Q) && xd->trig_mmio) + if ((eoi_val & XIVE_ESB_VAL_Q) && xd->trig_mmio) { + DBG_VERBOSE(" -> eoi retrigger !\n"); out_be64(xd->trig_mmio, 0); + } } } } @@ -380,10 +387,15 @@ static void xive_do_source_set_mask(struct xive_irq_data *xd, if (mask) { val = xive_poke_esb(xd, XIVE_ESB_SET_PQ_01); xd->saved_p = !!(val & XIVE_ESB_VAL_P); - } else if (xd->saved_p) - xive_poke_esb(xd, XIVE_ESB_SET_PQ_10); - else - xive_poke_esb(xd, XIVE_ESB_SET_PQ_00); + DBG_VERBOSE("masking val=%llx, sp=%d\n", + val, xd->saved_p); + } else { + DBG_VERBOSE("unmasking sp=%d\n", xd->saved_p); + if (xd->saved_p) + xive_poke_esb(xd, XIVE_ESB_SET_PQ_10); + else + xive_poke_esb(xd, XIVE_ESB_SET_PQ_00); + } } /* @@ -526,6 +538,7 @@ static unsigned int xive_irq_startup(struct irq_data *d) pr_devel("xive_irq_startup: irq %d [0x%x] data @%p\n", d->irq, hw_irq, d); + pr_devel(" eoi_mmio=%p trig_mmio=%p\n", xd->eoi_mmio, xd->trig_mmio); #ifdef CONFIG_PCI_MSI /* @@ -754,6 +767,8 @@ static int xive_irq_retrigger(struct irq_data *d) if (WARN_ON(xd->flags & XIVE_IRQ_FLAG_LSI)) return 0; + DBG_VERBOSE("retrigger irq %d\n", d->irq); + /* * To perform a retrigger, we first set the PQ bits to * 11, then perform an EOI.
The existing verbose debug code doesn't build when enabled. This fixes it and generally improves the output to make it more useful. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- arch/powerpc/sysdev/xive/common.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)