[18/22] m68k/mm: kmap - Modernize printing of kernel messages
diff mbox series

Message ID 1481123360-10978-19-git-send-email-geert@linux-m68k.org
State New, archived
Headers show
Series
  • m68k: Modernize printing of kernel messages
Related show

Commit Message

Geert Uytterhoeven Dec. 7, 2016, 3:09 p.m. UTC
- Convert from printk() to pr_*(),
  - Add missing continuations,
  - Remove #undef DEBUG.

Note that "#ifdef DEBUG" is sometimes retained because pr_cont() is not
optimized away when debugging is disabled.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/mm/kmap.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

Comments

Finn Thain Dec. 7, 2016, 11:26 p.m. UTC | #1
On Wed, 7 Dec 2016, Geert Uytterhoeven wrote:

>   - Convert from printk() to pr_*(),
>   - Add missing continuations,
>   - Remove #undef DEBUG.
> 
> Note that "#ifdef DEBUG" is sometimes retained because pr_cont() is not
> optimized away when debugging is disabled.
> 

I think that argues for using printk(KERN_DEBUG ...) and print(KERN_CONT 
...) inside #ifdef DEBUG, which would need no explanation.

If instead you use a combination of pr_debug and pr_cont and #ifdef DEBUG, 
perhaps the explanation should be moved from the commit log to a comment 
in the code?
Finn Thain Dec. 9, 2016, 10:50 p.m. UTC | #2
On Thu, 8 Dec 2016, I wrote:

> 
> On Wed, 7 Dec 2016, Geert Uytterhoeven wrote:
> 
> >   - Convert from printk() to pr_*(),
> >   - Add missing continuations,
> >   - Remove #undef DEBUG.
> > 
> > Note that "#ifdef DEBUG" is sometimes retained because pr_cont() is 
> > not optimized away when debugging is disabled.
> > 
> 
> I think that argues for using printk(KERN_DEBUG ...) and print(KERN_CONT 
> ...) inside #ifdef DEBUG, which would need no explanation.
> 
> If instead you use a combination of pr_debug and pr_cont and #ifdef 
> DEBUG, perhaps the explanation should be moved from the commit log to a 
> comment in the code?
> 

Perhaps a better solution than these alternatives would be,

#if defined(DEBUG)
#define pr_debug_cont pr_cont
#else
#define pr_debug_cont no_printk
#endif

But this API is still surprising and ugly. It doesn't work with 
CONFIG_DYNAMIC_DEBUG but that's not so important.

IMO, a far better linux/printk.h would have provided us with these 
definitions:

#define pr_emerg(fmt, ...) \
        printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
/* ... */
#define pr_debug(fmt, ...) \
        printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#define pr_cont(fmt, ...) \
        printk(KERN_CONT fmt, ##__VA_ARGS__)

#if defined(CONFIG_DYNAMIC_DEBUG)
#define pr_debug_cond(fmt, ...) \
        dynamic_pr_debug(fmt, ##__VA_ARGS__)
#define pr_cont_cond(fmt, ...) \
        dynamic_pr_cont(fmt, ##__VA_ARGS__)
#elif defined(DEBUG)
#define pr_debug_cond(fmt, ...) \
        printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#define pr_cont_cond(fmt, ...) \
        no_printk(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
#else
#define pr_debug_cond(fmt, ...) \
        no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#define pr_cont_cond(fmt, ...) \
        no_printk(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
#endif

Which have the virtues of symmetry and least surprise.

--

Patch
diff mbox series

diff --git a/arch/m68k/mm/kmap.c b/arch/m68k/mm/kmap.c
index 6e4955bc542bfc59..ce4773461651656a 100644
--- a/arch/m68k/mm/kmap.c
+++ b/arch/m68k/mm/kmap.c
@@ -21,8 +21,6 @@ 
 #include <asm/pgalloc.h>
 #include <asm/io.h>
 
-#undef DEBUG
-
 #define PTRTREESIZE	(256*1024)
 
 /*
@@ -125,9 +123,7 @@  void __iomem *__ioremap(unsigned long physaddr, unsigned long size, int cachefla
 	}
 #endif
 
-#ifdef DEBUG
-	printk("ioremap: 0x%lx,0x%lx(%d) - ", physaddr, size, cacheflag);
-#endif
+	pr_debug("ioremap: 0x%lx,0x%lx(%d) - ", physaddr, size, cacheflag);
 	/*
 	 * Mappings have to be aligned
 	 */
@@ -145,7 +141,7 @@  void __iomem *__ioremap(unsigned long physaddr, unsigned long size, int cachefla
 	virtaddr = (unsigned long)area->addr;
 	retaddr = virtaddr + offset;
 #ifdef DEBUG
-	printk("0x%lx,0x%lx,0x%lx", physaddr, virtaddr, retaddr);
+	pr_cont("0x%lx,0x%lx,0x%lx", physaddr, virtaddr, retaddr);
 #endif
 
 	/*
@@ -187,12 +183,12 @@  void __iomem *__ioremap(unsigned long physaddr, unsigned long size, int cachefla
 	while ((long)size > 0) {
 #ifdef DEBUG
 		if (!(virtaddr & (PTRTREESIZE-1)))
-			printk ("\npa=%#lx va=%#lx ", physaddr, virtaddr);
+			pr_info("\npa=%#lx va=%#lx ", physaddr, virtaddr);
 #endif
 		pgd_dir = pgd_offset_k(virtaddr);
 		pmd_dir = pmd_alloc(&init_mm, pgd_dir, virtaddr);
 		if (!pmd_dir) {
-			printk("ioremap: no mem for pmd_dir\n");
+			pr_err("ioremap: no mem for pmd_dir\n");
 			return NULL;
 		}
 
@@ -204,7 +200,7 @@  void __iomem *__ioremap(unsigned long physaddr, unsigned long size, int cachefla
 		} else {
 			pte_dir = pte_alloc_kernel(pmd_dir, virtaddr);
 			if (!pte_dir) {
-				printk("ioremap: no mem for pte_dir\n");
+				pr_err("ioremap: no mem for pte_dir\n");
 				return NULL;
 			}
 
@@ -215,7 +211,7 @@  void __iomem *__ioremap(unsigned long physaddr, unsigned long size, int cachefla
 		}
 	}
 #ifdef DEBUG
-	printk("\n");
+	pr_cont("\n");
 #endif
 	flush_tlb_all();
 
@@ -254,7 +250,7 @@  void __iounmap(void *addr, unsigned long size)
 	while ((long)size > 0) {
 		pgd_dir = pgd_offset_k(virtaddr);
 		if (pgd_bad(*pgd_dir)) {
-			printk("iounmap: bad pgd(%08lx)\n", pgd_val(*pgd_dir));
+			pr_err("iounmap: bad pgd(%08lx)\n", pgd_val(*pgd_dir));
 			pgd_clear(pgd_dir);
 			return;
 		}
@@ -274,7 +270,7 @@  void __iounmap(void *addr, unsigned long size)
 		}
 
 		if (pmd_bad(*pmd_dir)) {
-			printk("iounmap: bad pmd (%08lx)\n", pmd_val(*pmd_dir));
+			pr_err("iounmap: bad pmd (%08lx)\n", pmd_val(*pmd_dir));
 			pmd_clear(pmd_dir);
 			return;
 		}
@@ -332,7 +328,8 @@  void kernel_set_cachemode(void *addr, unsigned long size, int cmode)
 	while ((long)size > 0) {
 		pgd_dir = pgd_offset_k(virtaddr);
 		if (pgd_bad(*pgd_dir)) {
-			printk("iocachemode: bad pgd(%08lx)\n", pgd_val(*pgd_dir));
+			pr_err("iocachemode: bad pgd(%08lx)\n",
+			       pgd_val(*pgd_dir));
 			pgd_clear(pgd_dir);
 			return;
 		}
@@ -351,7 +348,8 @@  void kernel_set_cachemode(void *addr, unsigned long size, int cmode)
 		}
 
 		if (pmd_bad(*pmd_dir)) {
-			printk("iocachemode: bad pmd (%08lx)\n", pmd_val(*pmd_dir));
+			pr_err("iocachemode: bad pmd (%08lx)\n",
+			       pmd_val(*pmd_dir));
 			pmd_clear(pmd_dir);
 			return;
 		}