From: Finn Thain <fthain@telegraphics.com.au>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Greg Ungerer <gerg@linux-m68k.org>, Sam Creasey <sammy@sammy.net>,
Joshua Thompson <funaho@jurai.org>,
linux-m68k <linux-m68k@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 02/22] m68k/mac: macints - Modernize printing of kernel messages
Date: Fri, 9 Dec 2016 10:11:36 +1100 (AEDT) [thread overview]
Message-ID: <alpine.LNX.2.00.1612090956380.2773@nippy.intranet> (raw)
In-Reply-To: <CAMuHMdU+Fp+1QCVU4r+h9uUdZKsfa=E4JT7kuK56QjdPxPrSAg@mail.gmail.com>
On Thu, 8 Dec 2016, Geert Uytterhoeven wrote:
> On Wed, Dec 7, 2016 at 11:45 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
> > On Wed, 7 Dec 2016, Geert Uytterhoeven wrote:
> >
> >> - Introduce helpers for printing debug messages, incl. dummies for
> >> validating format strings when debugging is disabled,
> >> - Convert from printk() to pr_*(),
> >> - Add missing continuations, to fix user-visible breakage.
> >>
> >> Fixes: 4bcc595ccd80decb ("printk: reinstate KERN_CONT for printing continuation lines")
> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >> ---
> >> arch/m68k/mac/macints.c | 42 ++++++++++++++++++++++--------------------
> >> 1 file changed, 22 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/arch/m68k/mac/macints.c b/arch/m68k/mac/macints.c
> >> index 9f98c08719010e27..8572290cb93b6679 100644
> >> --- a/arch/m68k/mac/macints.c
> >> +++ b/arch/m68k/mac/macints.c
> >> @@ -135,6 +135,11 @@
> >> irqreturn_t mac_debug_handler(int, void *);
> >>
> >> /* #define DEBUG_MACINTS */
> >> +#ifdef DEBUG_MACINTS
> >> +#define pr_irq(fmt, ...) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> >> +#else
> >> +#define pr_irq(fmt, ...) no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> >> +#endif
> >>
> >> static unsigned int mac_irq_startup(struct irq_data *);
> >> static void mac_irq_shutdown(struct irq_data *);
> >
> > I would prefer to delete all the DEBUG_MACINTS clutter.
>
> OK, can do that.
Thanks.
>
> >> @@ -281,7 +282,7 @@ static void mac_irq_shutdown(struct irq_data *data)
> >> irqreturn_t mac_debug_handler(int irq, void *dev_id)
> >> {
> >> if (num_debug[irq] < 10) {
> >> - printk("DEBUG: Unexpected IRQ %d\n", irq);
> >> + pr_info("DEBUG: Unexpected IRQ %d\n", irq);
> >> num_debug[irq]++;
> >> }
> >> return IRQ_HANDLED;
> >
> > Is this dead code?
>
> Yes, mac_debug_handler() is unused.
>
> Removing that can be done in a separate patch (this is one of the
> patches I'd like to send upstream ASAP).
OK.
>
> >> @@ -319,20 +320,21 @@ irqreturn_t mac_nmi_handler(int irq, void *dev_id)
> >> #if 0
> >> struct pt_regs *fp = get_irq_regs();
> >> show_state();
> >> - printk("PC: %08lx\nSR: %04x SP: %p\n", fp->pc, fp->sr, fp);
> >> - printk("d0: %08lx d1: %08lx d2: %08lx d3: %08lx\n",
> >> - fp->d0, fp->d1, fp->d2, fp->d3);
> >> - printk("d4: %08lx d5: %08lx a0: %08lx a1: %08lx\n",
> >> - fp->d4, fp->d5, fp->a0, fp->a1);
> >> + pr_info("PC: %08lx\nSR: %04x SP: %p\n", fp->pc, fp->sr, fp);
> >> + pr_info("d0: %08lx d1: %08lx d2: %08lx d3: %08lx\n",
> >> + fp->d0, fp->d1, fp->d2, fp->d3);
> >> + pr_info("d4: %08lx d5: %08lx a0: %08lx a1: %08lx\n",
> >> + fp->d4, fp->d5, fp->a0, fp->a1);
> >>
> >> if (STACK_MAGIC != *(unsigned long *)current->kernel_stack_page)
> >> - printk("Corrupted stack page\n");
> >> - printk("Process %s (pid: %d, stackpage=%08lx)\n",
> >> - current->comm, current->pid, current->kernel_stack_page);
> >> + pr_info("Corrupted stack page\n");
> >> + pr_info("Process %s (pid: %d, stackpage=%08lx)\n",
> >> + current->comm, current->pid,
> >> + current->kernel_stack_page);
> >> if (intr_count == 1)
> >> dump_stack((struct frame *)fp);
> >> #else
> >> - /* printk("NMI "); */
> >> + /* pr_info("NMI "); */
> >> #endif
> >> }
> >> in_nmi--;
> >>
> >
> > I think it would be good to use pr_debug here instead of #if 0. But that
> > will probably break the build... better ignore the #if 0 section for this
> > series, until I put together a different patch?
>
> Or worse, break at runtime.
>
The NMI isn't easily tiggered, unless you know how. It's solely for
developers.
Anyway, this is dead code so it makes little difference whether you patch
it or not. But this hunk will break an NMI debug patch that I've been
using for ten years or so. Since this code needs a rewrite anyway, you
might as well avoid the churn IMHO.
> There's also non-printing code in that block.
But no side-effects, right?
--
> Gr{oetje,eeting}s,
>
> Geert
>
next prev parent reply other threads:[~2016-12-08 23:11 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-07 15:08 [PATCH 00/22] m68k: Modernize printing of kernel messages Geert Uytterhoeven
2016-12-07 15:08 ` [PATCH 01/22] m68k/atari: " Geert Uytterhoeven
2016-12-07 22:36 ` Finn Thain
2016-12-08 12:22 ` Geert Uytterhoeven
2016-12-08 22:55 ` Finn Thain
2016-12-10 0:44 ` Michael Schmitz
2017-02-09 11:59 ` Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 02/22] m68k/mac: macints - " Geert Uytterhoeven
2016-12-07 22:45 ` Finn Thain
2016-12-08 12:25 ` Geert Uytterhoeven
2016-12-08 23:11 ` Finn Thain [this message]
2016-12-07 15:09 ` [PATCH 03/22] m68k/mac: via " Geert Uytterhoeven
2016-12-07 22:48 ` Finn Thain
2016-12-07 15:09 ` [PATCH 04/22] m68k/68000: " Geert Uytterhoeven
2016-12-09 12:00 ` Greg Ungerer
2016-12-07 15:09 ` [PATCH 05/22] m68k/bvme6000: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 06/22] m68k/coldfire: " Geert Uytterhoeven
2016-12-07 22:53 ` Finn Thain
2016-12-09 12:09 ` Greg Ungerer
2016-12-07 15:09 ` [PATCH 07/22] m68k/mac: baboon - " Geert Uytterhoeven
2016-12-07 22:54 ` Finn Thain
2016-12-07 15:09 ` [PATCH 08/22] m68k/mac: iop " Geert Uytterhoeven
2016-12-07 23:06 ` Finn Thain
2016-12-07 15:09 ` [PATCH 09/22] m68k/mac: oss " Geert Uytterhoeven
2016-12-07 23:08 ` Finn Thain
2016-12-07 15:09 ` [PATCH 10/22] m68k/mac: psc " Geert Uytterhoeven
2016-12-07 23:10 ` Finn Thain
2016-12-07 15:09 ` [PATCH 11/22] m68k/mac: " Geert Uytterhoeven
2016-12-07 23:17 ` Finn Thain
2016-12-07 15:09 ` [PATCH 12/22] m68k/mvme147: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 13/22] m68k/mvme16x: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 14/22] m68k/q40: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 15/22] m68k/sun3: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 16/22] m68k/sun3x: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 17/22] m68k/kernel: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 18/22] m68k/mm: kmap - " Geert Uytterhoeven
2016-12-07 23:26 ` Finn Thain
2016-12-09 22:50 ` Finn Thain
2016-12-07 15:09 ` [PATCH 19/22] m68k/mm: motorola " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 20/22] m68k/mm: sun3 " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 21/22] m68k/mm: " Geert Uytterhoeven
2016-12-07 15:09 ` [PATCH 22/22] m68k/include: " Geert Uytterhoeven
2017-02-09 13:25 ` [PATCH 00/22] m68k: " Geert Uytterhoeven
2017-02-12 9:43 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LNX.2.00.1612090956380.2773@nippy.intranet \
--to=fthain@telegraphics.com.au \
--cc=funaho@jurai.org \
--cc=geert@linux-m68k.org \
--cc=gerg@linux-m68k.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@vger.kernel.org \
--cc=sammy@sammy.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).