Some printk cleanup in mm

PATCH Superseded Mitchel Humpherys 4 participants 9 messages Mar 27, 2014 – Apr 16, 2014

Subsystems (6)

Memory management MGLRU Memory mapping MM reclaim +2 more subsystems

lore

Share & tools

lore

Mitchel Humpherys

author

2014-03-27 17:54 UTC #0 lore raw

PATCH v2 1/1 Some printk cleanup in mm

This series cleans up some printks in the mm code that were missing log levels.

Changelog:

- v2: Suggestions by Joe Perches (pr_fmt, pr_cont, pr_err, __func__, missing \n)

Mitchel Humpherys (1):
mm: convert some level-less printks to pr_*

 mm/bounce.c    |  7 +++++--
 mm/mempolicy.c |  5 ++++-
 mm/mmap.c      | 21 ++++++++++++---------
 mm/nommu.c     |  5 ++++-
 mm/slub.c      |  9 ++++++---
 mm/vmscan.c    |  5 ++++-
 6 files changed, 35 insertions(+), 17 deletions(-)

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Mitchel Humpherys

author

2014-03-27 17:54 UTC #1 lore raw

PATCH v2 1/1 mm: convert some level-less printks to pr_*

printk is meant to be used with an associated log level. There are some instances of printk scattered around the mm code where the log level is missing. Add a log level and adhere to suggestions by scripts/checkpatch.pl by moving to the pr_* macros.

Signed-off-by Mitchel Humpherys <[email protected]>

---
 mm/bounce.c    |  7 +++++--
 mm/mempolicy.c |  5 ++++-
 mm/mmap.c      | 21 ++++++++++++---------
 mm/nommu.c     |  5 ++++-
 mm/slub.c      |  9 ++++++---
 mm/vmscan.c    |  5 ++++-
 6 files changed, 35 insertions(+), 17 deletions(-)

mm/bounce.c +5 -2

index 523918b8c6..d35850895b 100644
@@ -3,6 +3,8 @@
  * - Split from highmem.c
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/mm.h>
 #include <linux/export.h>
 #include <linux/swap.h>

@@ -15,6 +17,7 @@
 #include <linux/hash.h>
 #include <linux/highmem.h>
 #include <linux/bootmem.h>
+#include <linux/printk.h>
 #include <asm/tlbflush.h>
 
 #include <trace/events/block.h>

@@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
 
 	page_pool = mempool_create_page_pool(POOL_SIZE, 0);
 	BUG_ON(!page_pool);
-	printk("bounce pool size: %d pages\n", POOL_SIZE);
+	pr_info("bounce pool size: %d pages\n", POOL_SIZE);
 
 	return 0;
 }

@@ -86,7 +89,7 @@ int init_emergency_isa_pool(void)
 				       mempool_free_pages, (void *) 0);
 	BUG_ON(!isa_page_pool);
 
-	printk("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
+	pr_info("isa bounce pool size: %d pages\n", ISA_POOL_SIZE);
 	return 0;
 }

mm/mempolicy.c +4 -1

index ae3c8f3595..aec6220485 100644
@@ -65,6 +65,8 @@
    kernel is not always grateful with that.
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/mempolicy.h>
 #include <linux/mm.h>
 #include <linux/highmem.h>

@@ -91,6 +93,7 @@
 #include <linux/ctype.h>
 #include <linux/mm_inline.h>
 #include <linux/mmu_notifier.h>
+#include <linux/printk.h>
 
 #include <asm/tlbflush.h>
 #include <asm/uaccess.h>

@@ -2751,7 +2754,7 @@ void __init numa_policy_init(void)
 		node_set(prefer, interleave_nodes);
 
 	if (do_set_mempolicy(MPOL_INTERLEAVE, 0, &interleave_nodes))
-		printk("numa_policy_init: interleaving failed\n");
+		pr_err("%s: interleaving failed\n", __func__);
 
 	check_numabalancing_enable();
 }

mm/mmap.c +12 -9

index 20ff0c3327..c7023516da 100644
@@ -6,6 +6,8 @@
  * Address space accounting code	<[email protected]>
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/backing-dev.h>

@@ -36,6 +38,7 @@
 #include <linux/sched/sysctl.h>
 #include <linux/notifier.h>
 #include <linux/memory.h>
+#include <linux/printk.h>
 
 #include <asm/uaccess.h>
 #include <asm/cacheflush.h>

@@ -360,20 +363,20 @@ static int browse_rb(struct rb_root *root)
 		struct vm_area_struct *vma;
 		vma = rb_entry(nd, struct vm_area_struct, vm_rb);
 		if (vma->vm_start < prev) {
-			printk("vm_start %lx prev %lx\n", vma->vm_start, prev);
+			pr_info("vm_start %lx prev %lx\n", vma->vm_start, prev);
 			bug = 1;
 		}
 		if (vma->vm_start < pend) {
-			printk("vm_start %lx pend %lx\n", vma->vm_start, pend);
+			pr_info("vm_start %lx pend %lx\n", vma->vm_start, pend);
 			bug = 1;
 		}
 		if (vma->vm_start > vma->vm_end) {
-			printk("vm_end %lx < vm_start %lx\n",
+			pr_info("vm_end %lx < vm_start %lx\n",
 				vma->vm_end, vma->vm_start);
 			bug = 1;
 		}
 		if (vma->rb_subtree_gap != vma_compute_subtree_gap(vma)) {
-			printk("free gap %lx, correct %lx\n",
+			pr_info("free gap %lx, correct %lx\n",
 			       vma->rb_subtree_gap,
 			       vma_compute_subtree_gap(vma));
 			bug = 1;

@@ -387,7 +390,7 @@ static int browse_rb(struct rb_root *root)
 	for (nd = pn; nd; nd = rb_prev(nd))
 		j++;
 	if (i != j) {
-		printk("backwards %d, forwards %d\n", j, i);
+		pr_info("backwards %d, forwards %d\n", j, i);
 		bug = 1;
 	}
 	return bug ? -1 : i;

@@ -422,17 +425,17 @@ void validate_mm(struct mm_struct *mm)
 		i++;
 	}
 	if (i != mm->map_count) {
-		printk("map_count %d vm_next %d\n", mm->map_count, i);
+		pr_info("map_count %d vm_next %d\n", mm->map_count, i);
 		bug = 1;
 	}
 	if (highest_address != mm->highest_vm_end) {
-		printk("mm->highest_vm_end %lx, found %lx\n",
+		pr_info("mm->highest_vm_end %lx, found %lx\n",
 		       mm->highest_vm_end, highest_address);
 		bug = 1;
 	}
 	i = browse_rb(&mm->mm_rb);
 	if (i != mm->map_count) {
-		printk("map_count %d rb %d\n", mm->map_count, i);
+		pr_info("map_count %d rb %d\n", mm->map_count, i);
 		bug = 1;
 	}
 	BUG_ON(bug);

@@ -3237,7 +3240,7 @@ static struct notifier_block reserve_mem_nb = {
 static int __meminit init_reserve_notifier(void)
 {
 	if (register_hotmemory_notifier(&reserve_mem_nb))
-		printk("Failed registering memory add/remove notifier for admin reserve");
+		pr_err("Failed registering memory add/remove notifier for admin reserve\n");
 
 	return 0;
 }

mm/nommu.c +4 -1

index 8740213b16..1ff42f87dd 100644
@@ -13,6 +13,8 @@
  *  Copyright (c) 2007-2010 Paul Mundt <[email protected]>
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/export.h>
 #include <linux/mm.h>
 #include <linux/mman.h>

@@ -30,6 +32,7 @@
 #include <linux/syscalls.h>
 #include <linux/audit.h>
 #include <linux/sched/sysctl.h>
+#include <linux/printk.h>
 
 #include <asm/uaccess.h>
 #include <asm/tlb.h>

@@ -1241,7 +1244,7 @@ error_free:
 	return ret;
 
 enomem:
-	printk("Allocation of length %lu from process %d (%s) failed\n",
+	pr_err("Allocation of length %lu from process %d (%s) failed\n",
 	       len, current->pid, current->comm);
 	show_free_areas(0);
 	return -ENOMEM;

mm/slub.c +6 -3

index 25f14ad8f8..9f109e6756 100644
@@ -9,6 +9,8 @@
  * (C) 2011 Linux Foundation, Christoph Lameter
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/mm.h>
 #include <linux/swap.h> /* struct reclaim_state */
 #include <linux/module.h>

@@ -33,6 +35,7 @@
 #include <linux/stacktrace.h>
 #include <linux/prefetch.h>
 #include <linux/memcontrol.h>
+#include <linux/printk.h>
 
 #include <trace/events/kmem.h>

@@ -1774,15 +1777,15 @@ static inline void note_cmpxchg_failure(const char *n,
 
 #ifdef CONFIG_PREEMPT
 	if (tid_to_cpu(tid) != tid_to_cpu(actual_tid))
-		printk("due to cpu change %d -> %d\n",
+		pr_cont("due to cpu change %d -> %d\n",
 			tid_to_cpu(tid), tid_to_cpu(actual_tid));
 	else
 #endif
 	if (tid_to_event(tid) != tid_to_event(actual_tid))
-		printk("due to cpu running other code. Event %ld->%ld\n",
+		pr_cont("due to cpu running other code. Event %ld->%ld\n",
 			tid_to_event(tid), tid_to_event(actual_tid));
 	else
-		printk("for unknown reason: actual=%lx was=%lx target=%lx\n",
+		pr_cont("for unknown reason: actual=%lx was=%lx target=%lx\n",
 			actual_tid, tid, next_tid(tid));
 #endif
 	stat(s, CMPXCHG_DOUBLE_CPU_FAIL);

mm/vmscan.c +4 -1

index a9c74b4096..394dd2b644 100644
@@ -11,6 +11,8 @@
  *  Multiqueue VM started 5.8.00, Rik van Riel.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/gfp.h>

@@ -43,6 +45,7 @@
 #include <linux/sysctl.h>
 #include <linux/oom.h>
 #include <linux/prefetch.h>
+#include <linux/printk.h>
 
 #include <asm/tlbflush.h>
 #include <asm/div64.h>

@@ -477,7 +480,7 @@ static pageout_t pageout(struct page *page, struct address_space *mapping,
 		if (page_has_private(page)) {
 			if (try_to_free_buffers(page)) {
 				ClearPageDirty(page);
-				printk("%s: orphaned page\n", __func__);
+				pr_info("%s: orphaned page\n", __func__);
 				return PAGE_CLEAN;
 			}
 		}

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2014-03-31 18:35 UTC #2 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

mm/slub.c line 12

Quoted 9 lines · Mitchel Humpherys

mm/slub.c +1 -0

index 25f14ad8f8..9f109e6756 100644
@@ -9,6 +9,8 @@
  * (C) 2011 Linux Foundation, Christoph Lameter
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

This is implicitly used by some macros? If so then please define this elsewhere. I do not see any use in slub.c of this one.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Joe Perches

2014-03-31 18:44 UTC #3 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

Quoted 3 lines · Christoph Lameter

mm/slub.c
Quoted 1 line · Mitchel Humpherys
mm/slub.c +0 -0

[]

Quoted 8 lines

mm/slub.c line 12
Quoted 5 lines
mm/slub.c +1 -0
@@ -9,6 +9,8 @@
  * (C) 2011 Linux Foundation, Christoph Lameter
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
This is implicitly used by some macros? If so then please define this elsewhere. I do not see any use in slub.c of this one.

Hi Christoph

All the pr_<level> macros use it.

from include/linux/printk.h:

#ifndef pr_fmt
#define pr_fmt(fmt) fmt
#endif

#define pr_emerg(fmt, ...) \
	printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)

etc...

Christoph Lameter

2014-04-03 16:33 UTC #4 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

Quoted 14 lines · Joe Perches

Quoted 3 lines · Christoph Lameter
mm/slub.c
Quoted 1 line · Mitchel Humpherys
mm/slub.c +0 -0
[]
Quoted 8 lines
mm/slub.c line 12
Quoted 5 lines
mm/slub.c +1 -0
@@ -9,6 +9,8 @@
  * (C) 2011 Linux Foundation, Christoph Lameter
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
This is implicitly used by some macros? If so then please define this elsewhere. I do not see any use in slub.c of this one.

Quoted 5 lines

Hi Christoph
All the pr_<level> macros use it.
from include/linux/printk.h:

Ok then why do you add the definition to slub.c?

Mitchel Humpherys

author

2014-04-03 16:57 UTC #5 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

Quoted 16 lines · Christoph Lameter

Quoted 14 lines · Joe Perches
Quoted 3 lines · Christoph Lameter
mm/slub.c
Quoted 1 line · Mitchel Humpherys
mm/slub.c +0 -0
[]
Quoted 8 lines
mm/slub.c line 12
Quoted 5 lines
mm/slub.c +1 -0
@@ -9,6 +9,8 @@
  * (C) 2011 Linux Foundation, Christoph Lameter
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
This is implicitly used by some macros? If so then please define this elsewhere. I do not see any use in slub.c of this one.

Quoted 7 lines

Quoted 5 lines
Hi Christoph
All the pr_<level> macros use it.
from include/linux/printk.h:
Ok then why do you add the definition to slub.c?

Ah that was an oversight on my part after changing to pr_cont. I'll send a v3 that removes the pr_fmt. Or I could send a v3 that leaves the pr_fmt but changes the printk that the pr_cont's are continuing (at the top of note_cmpxchg_failure) to pr_info, but that wouldn't be consistent with the rest of the file, which is using hand-tagged printk's.

I don't know if it's worthwhile to convert all of the hand-tagged printk's to the pr_ macros, but if so I can do that in a separate patch.

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andrew Morton

maintainer

2014-04-14 22:55 UTC #6 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

In reply to Mitchel Humpherys

Quoted 5 lines · On Thu, 27 Mar 2014 10:54:19 -0700 Mitchel Humpherys <[email protected]>…

printk is meant to be used with an associated log level. There are some instances of printk scattered around the mm code where the log level is missing. Add a log level and adhere to suggestions by scripts/checkpatch.pl by moving to the pr_* macros.

hm, this is a functional change.

mm/bounce.c line 6

Quoted 7 lines

mm/bounce.c +1 -0

@@ -3,6 +3,8 @@
  * - Split from highmem.c
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Because of this.

Quoted 12 lines

mm/bounce.c +1 -0

 #include <linux/mm.h>
 #include <linux/export.h>
 #include <linux/swap.h>

@@ -15,6 +17,7 @@
 #include <linux/hash.h>
 #include <linux/highmem.h>
 #include <linux/bootmem.h>
+#include <linux/printk.h>
 #include <asm/tlbflush.h>
 
 #include <trace/events/block.h>

mm/bounce.c static __init int init_emergency_pool(void) · line 40

Quoted 5 lines

mm/bounce.c +1 -1

@@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
 
 	page_pool = mempool_create_page_pool(POOL_SIZE, 0);
 	BUG_ON(!page_pool);
-	printk("bounce pool size: %d pages\n", POOL_SIZE);
+	pr_info("bounce pool size: %d pages\n", POOL_SIZE);

This used to print "bounce pool size: N pages" but will now print "bounce: bounce pool size: N pages".

It isn't necessarily a *bad* change but perhaps a little more thought could be put into it. In this example it would be better remove the redundancy by using

	pr_info("pool size: %d pages\n"...);

And all of this should be described and justified in the changelog, please.

Mitchel Humpherys

author

2014-04-15 23:58 UTC #7 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

Quoted 22 lines · Andrew Morton

Quoted 12 lines · On Thu, 27 Mar 2014 10:54:19 -0700 Mitchel Humpherys <[email protected]>…
#include <linux/mm.h>
#include <linux/export.h>
#include <linux/swap.h>
mm/bounce.c +1 -0
@@ -15,6 +17,7 @@
 #include <linux/hash.h>
 #include <linux/highmem.h>
 #include <linux/bootmem.h>
+#include <linux/printk.h>
 #include <asm/tlbflush.h>
 
 #include <trace/events/block.h>
mm/bounce.c static __init int init_emergency_pool(void) · line 40
Quoted 5 lines
mm/bounce.c +1 -1
@@ -34,7 +37,7 @@ static __init int init_emergency_pool(void)
 
 	page_pool = mempool_create_page_pool(POOL_SIZE, 0);
 	BUG_ON(!page_pool);
-	printk("bounce pool size: %d pages\n", POOL_SIZE);
+	pr_info("bounce pool size: %d pages\n", POOL_SIZE);
This used to print "bounce pool size: N pages" but will now print "bounce: bounce pool size: N pages".

Quoted 5 lines

It isn't necessarily a *bad* change but perhaps a little more thought could be put into it. In this example it would be better remove the redundancy by using
	pr_info("pool size: %d pages\n"...);

Yes I noticed this in my boot-test... I'll change it to remove the redundancy. The others all seem okay.

Quoted 3 lines

And all of this should be described and justified in the changelog, please.

Will send a v3 shortly. Thanks for your comments.

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Mitchel Humpherys

author

2014-04-16 00:05 UTC #8 lore raw

Re: [PATCH v2] mm: convert some level-less printks to pr_*

Quoted 5 lines · Mitchel Humpherys

Quoted 2 lines · Andrew Morton
And all of this should be described and justified in the changelog, please.
Will send a v3 shortly. Thanks for your comments.

Make that a v4, I actually already sent a v3. You'd think I could get a printk change right on v1 :). We'll get there.

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>