All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] mm: page_alloc: Object code reductions and logging fix
@ 2017-03-16  1:43 ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-mm; +Cc: linux-kernel

Joe Perches (3):
  mm: page_alloc: Reduce object size by neatening printks
  mm: page_alloc: Fix misordered logging output, reduce code size
  mm: page_alloc: Break up a long single-line printk

 mm/page_alloc.c | 248 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 127 insertions(+), 121 deletions(-)

-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] mm: page_alloc: Object code reductions and logging fix
@ 2017-03-16  1:43 ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-mm; +Cc: linux-kernel

Joe Perches (3):
  mm: page_alloc: Reduce object size by neatening printks
  mm: page_alloc: Fix misordered logging output, reduce code size
  mm: page_alloc: Break up a long single-line printk

 mm/page_alloc.c | 248 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 127 insertions(+), 121 deletions(-)

-- 
2.10.0.rc2.1.g053435c

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16  1:43 ` Joe Perches
@ 2017-03-16  1:43   ` Joe Perches
  -1 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel; +Cc: linux-mm

Function calls with large argument counts cause x86-64 register
spilling.  Reducing the number of arguments in a multi-line printk
by converting to multiple printks which saves some object code size.

$ size mm/page_alloc.o* (defconfig)
   text    data     bss     dec     hex filename
  35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
  36018    1699     628   38345    95c9 mm/page_alloc.o.old

Miscellanea:

o Remove line leading spaces from the formerly multi-line printks
  commit a25700a53f71 ("mm: show bounce pages in oom killer output")
  back in 2007 started the leading space when a single long line
  was split into multiple lines but the leading space was likely
  mistakenly kept and subsequent commits followed suit.
o Align arguments in a few more printks

Signed-off-by: Joe Perches <joe@perches.com>
---
 mm/page_alloc.c | 237 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 118 insertions(+), 119 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f749b7ff7c50..5db9710cb932 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4505,79 +4505,79 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 			free_pcp += per_cpu_ptr(zone->pageset, cpu)->pcp.count;
 	}
 
-	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
-		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
-		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
-		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
-		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
-		" free:%lu free_pcp:%lu free_cma:%lu\n",
-		global_node_page_state(NR_ACTIVE_ANON),
-		global_node_page_state(NR_INACTIVE_ANON),
-		global_node_page_state(NR_ISOLATED_ANON),
-		global_node_page_state(NR_ACTIVE_FILE),
-		global_node_page_state(NR_INACTIVE_FILE),
-		global_node_page_state(NR_ISOLATED_FILE),
-		global_node_page_state(NR_UNEVICTABLE),
-		global_node_page_state(NR_FILE_DIRTY),
-		global_node_page_state(NR_WRITEBACK),
-		global_node_page_state(NR_UNSTABLE_NFS),
-		global_page_state(NR_SLAB_RECLAIMABLE),
-		global_page_state(NR_SLAB_UNRECLAIMABLE),
-		global_node_page_state(NR_FILE_MAPPED),
-		global_node_page_state(NR_SHMEM),
-		global_page_state(NR_PAGETABLE),
-		global_page_state(NR_BOUNCE),
-		global_page_state(NR_FREE_PAGES),
-		free_pcp,
-		global_page_state(NR_FREE_CMA_PAGES));
+	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
+	       global_node_page_state(NR_ACTIVE_ANON),
+	       global_node_page_state(NR_INACTIVE_ANON),
+	       global_node_page_state(NR_ISOLATED_ANON));
+	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
+	       global_node_page_state(NR_ACTIVE_FILE),
+	       global_node_page_state(NR_INACTIVE_FILE),
+	       global_node_page_state(NR_ISOLATED_FILE));
+	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
+	       global_node_page_state(NR_UNEVICTABLE),
+	       global_node_page_state(NR_FILE_DIRTY),
+	       global_node_page_state(NR_WRITEBACK),
+	       global_node_page_state(NR_UNSTABLE_NFS));
+	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
+	       global_page_state(NR_SLAB_RECLAIMABLE),
+	       global_page_state(NR_SLAB_UNRECLAIMABLE));
+	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
+	       global_node_page_state(NR_FILE_MAPPED),
+	       global_node_page_state(NR_SHMEM),
+	       global_page_state(NR_PAGETABLE),
+	       global_page_state(NR_BOUNCE));
+	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
+	       global_page_state(NR_FREE_PAGES),
+	       free_pcp,
+	       global_page_state(NR_FREE_CMA_PAGES));
 
 	for_each_online_pgdat(pgdat) {
 		if (show_mem_node_skip(filter, pgdat->node_id, nodemask))
 			continue;
 
 		printk("Node %d"
-			" active_anon:%lukB"
-			" inactive_anon:%lukB"
-			" active_file:%lukB"
-			" inactive_file:%lukB"
-			" unevictable:%lukB"
-			" isolated(anon):%lukB"
-			" isolated(file):%lukB"
-			" mapped:%lukB"
-			" dirty:%lukB"
-			" writeback:%lukB"
-			" shmem:%lukB"
+		       " active_anon:%lukB"
+		       " inactive_anon:%lukB"
+		       " active_file:%lukB"
+		       " inactive_file:%lukB"
+		       " unevictable:%lukB"
+		       " isolated(anon):%lukB"
+		       " isolated(file):%lukB"
+		       " mapped:%lukB"
+		       " dirty:%lukB"
+		       " writeback:%lukB"
+		       " shmem:%lukB"
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			" shmem_thp: %lukB"
-			" shmem_pmdmapped: %lukB"
-			" anon_thp: %lukB"
+		       " shmem_thp: %lukB"
+		       " shmem_pmdmapped: %lukB"
+		       " anon_thp: %lukB"
 #endif
-			" writeback_tmp:%lukB"
-			" unstable:%lukB"
-			" all_unreclaimable? %s"
-			"\n",
-			pgdat->node_id,
-			K(node_page_state(pgdat, NR_ACTIVE_ANON)),
-			K(node_page_state(pgdat, NR_INACTIVE_ANON)),
-			K(node_page_state(pgdat, NR_ACTIVE_FILE)),
-			K(node_page_state(pgdat, NR_INACTIVE_FILE)),
-			K(node_page_state(pgdat, NR_UNEVICTABLE)),
-			K(node_page_state(pgdat, NR_ISOLATED_ANON)),
-			K(node_page_state(pgdat, NR_ISOLATED_FILE)),
-			K(node_page_state(pgdat, NR_FILE_MAPPED)),
-			K(node_page_state(pgdat, NR_FILE_DIRTY)),
-			K(node_page_state(pgdat, NR_WRITEBACK)),
+		       " writeback_tmp:%lukB"
+		       " unstable:%lukB"
+		       " all_unreclaimable? %s"
+		       "\n",
+		       pgdat->node_id,
+		       K(node_page_state(pgdat, NR_ACTIVE_ANON)),
+		       K(node_page_state(pgdat, NR_INACTIVE_ANON)),
+		       K(node_page_state(pgdat, NR_ACTIVE_FILE)),
+		       K(node_page_state(pgdat, NR_INACTIVE_FILE)),
+		       K(node_page_state(pgdat, NR_UNEVICTABLE)),
+		       K(node_page_state(pgdat, NR_ISOLATED_ANON)),
+		       K(node_page_state(pgdat, NR_ISOLATED_FILE)),
+		       K(node_page_state(pgdat, NR_FILE_MAPPED)),
+		       K(node_page_state(pgdat, NR_FILE_DIRTY)),
+		       K(node_page_state(pgdat, NR_WRITEBACK)),
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
-			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
-					* HPAGE_PMD_NR),
-			K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
+			 * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
 #endif
-			K(node_page_state(pgdat, NR_SHMEM)),
-			K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
-			K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
-			pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
-				"yes" : "no");
+		       K(node_page_state(pgdat, NR_SHMEM)),
+		       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
+		       K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
+		       pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
+		       "yes" : "no");
 	}
 
 	for_each_populated_zone(zone) {
@@ -4592,51 +4592,51 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 
 		show_node(zone);
 		printk(KERN_CONT
-			"%s"
-			" free:%lukB"
-			" min:%lukB"
-			" low:%lukB"
-			" high:%lukB"
-			" active_anon:%lukB"
-			" inactive_anon:%lukB"
-			" active_file:%lukB"
-			" inactive_file:%lukB"
-			" unevictable:%lukB"
-			" writepending:%lukB"
-			" present:%lukB"
-			" managed:%lukB"
-			" mlocked:%lukB"
-			" slab_reclaimable:%lukB"
-			" slab_unreclaimable:%lukB"
-			" kernel_stack:%lukB"
-			" pagetables:%lukB"
-			" bounce:%lukB"
-			" free_pcp:%lukB"
-			" local_pcp:%ukB"
-			" free_cma:%lukB"
-			"\n",
-			zone->name,
-			K(zone_page_state(zone, NR_FREE_PAGES)),
-			K(min_wmark_pages(zone)),
-			K(low_wmark_pages(zone)),
-			K(high_wmark_pages(zone)),
-			K(zone_page_state(zone, NR_ZONE_ACTIVE_ANON)),
-			K(zone_page_state(zone, NR_ZONE_INACTIVE_ANON)),
-			K(zone_page_state(zone, NR_ZONE_ACTIVE_FILE)),
-			K(zone_page_state(zone, NR_ZONE_INACTIVE_FILE)),
-			K(zone_page_state(zone, NR_ZONE_UNEVICTABLE)),
-			K(zone_page_state(zone, NR_ZONE_WRITE_PENDING)),
-			K(zone->present_pages),
-			K(zone->managed_pages),
-			K(zone_page_state(zone, NR_MLOCK)),
-			K(zone_page_state(zone, NR_SLAB_RECLAIMABLE)),
-			K(zone_page_state(zone, NR_SLAB_UNRECLAIMABLE)),
-			zone_page_state(zone, NR_KERNEL_STACK_KB),
-			K(zone_page_state(zone, NR_PAGETABLE)),
-			K(zone_page_state(zone, NR_BOUNCE)),
-			K(free_pcp),
-			K(this_cpu_read(zone->pageset->pcp.count)),
-			K(zone_page_state(zone, NR_FREE_CMA_PAGES)));
+		       "%s"
+		       " free:%lukB"
+		       " min:%lukB"
+		       " low:%lukB"
+		       " high:%lukB"
+		       " active_anon:%lukB"
+		       " inactive_anon:%lukB"
+		       " active_file:%lukB"
+		       " inactive_file:%lukB"
+		       " unevictable:%lukB"
+		       " writepending:%lukB"
+		       " present:%lukB"
+		       " managed:%lukB"
+		       " mlocked:%lukB"
+		       " slab_reclaimable:%lukB"
+		       " slab_unreclaimable:%lukB"
+		       " kernel_stack:%lukB"
+		       " pagetables:%lukB"
+		       " bounce:%lukB"
+		       " free_pcp:%lukB"
+		       " local_pcp:%ukB"
+		       " free_cma:%lukB"
+		       "\n",
+		       zone->name,
+		       K(zone_page_state(zone, NR_FREE_PAGES)),
+		       K(min_wmark_pages(zone)),
+		       K(low_wmark_pages(zone)),
+		       K(high_wmark_pages(zone)),
+		       K(zone_page_state(zone, NR_ZONE_ACTIVE_ANON)),
+		       K(zone_page_state(zone, NR_ZONE_INACTIVE_ANON)),
+		       K(zone_page_state(zone, NR_ZONE_ACTIVE_FILE)),
+		       K(zone_page_state(zone, NR_ZONE_INACTIVE_FILE)),
+		       K(zone_page_state(zone, NR_ZONE_UNEVICTABLE)),
+		       K(zone_page_state(zone, NR_ZONE_WRITE_PENDING)),
+		       K(zone->present_pages),
+		       K(zone->managed_pages),
+		       K(zone_page_state(zone, NR_MLOCK)),
+		       K(zone_page_state(zone, NR_SLAB_RECLAIMABLE)),
+		       K(zone_page_state(zone, NR_SLAB_UNRECLAIMABLE)),
+		       zone_page_state(zone, NR_KERNEL_STACK_KB),
+		       K(zone_page_state(zone, NR_PAGETABLE)),
+		       K(zone_page_state(zone, NR_BOUNCE)),
+		       K(free_pcp),
+		       K(this_cpu_read(zone->pageset->pcp.count)),
+		       K(zone_page_state(zone, NR_FREE_CMA_PAGES)));
 		printk("lowmem_reserve[]:");
 		for (i = 0; i < MAX_NR_ZONES; i++)
 			printk(KERN_CONT " %ld", zone->lowmem_reserve[i]);
@@ -4679,7 +4679,8 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 
 	hugetlb_show_meminfo();
 
-	printk("%ld total pagecache pages\n", global_node_page_state(NR_FILE_PAGES));
+	printk("%ld total pagecache pages\n",
+	       global_node_page_state(NR_FILE_PAGES));
 
 	show_swap_cache_info();
 }
@@ -5516,8 +5517,7 @@ static __meminit void zone_pcp_init(struct zone *zone)
 
 	if (populated_zone(zone))
 		printk(KERN_DEBUG "  %s zone: %lu pages, LIFO batch:%u\n",
-			zone->name, zone->present_pages,
-					 zone_batchsize(zone));
+		       zone->name, zone->present_pages, zone_batchsize(zone));
 }
 
 int __meminit init_currently_empty_zone(struct zone *zone,
@@ -5891,8 +5891,8 @@ static void __meminit calculate_node_totalpages(struct pglist_data *pgdat,
 
 	pgdat->node_spanned_pages = totalpages;
 	pgdat->node_present_pages = realtotalpages;
-	printk(KERN_DEBUG "On node %d totalpages: %lu\n", pgdat->node_id,
-							realtotalpages);
+	printk(KERN_DEBUG "On node %d totalpages: %lu\n",
+	       pgdat->node_id, realtotalpages);
 }
 
 #ifndef CONFIG_SPARSEMEM
@@ -6042,8 +6042,7 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 			if (freesize >= memmap_pages) {
 				freesize -= memmap_pages;
 				if (memmap_pages)
-					printk(KERN_DEBUG
-					       "  %s zone: %lu pages used for memmap\n",
+					printk(KERN_DEBUG "  %s zone: %lu pages used for memmap\n",
 					       zone_names[j], memmap_pages);
 			} else
 				pr_warn("  %s zone: %lu pages exceeds freesize %lu\n",
@@ -6054,7 +6053,7 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 		if (j == 0 && freesize > dma_reserve) {
 			freesize -= dma_reserve;
 			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
-					zone_names[0], dma_reserve);
+			       zone_names[0], dma_reserve);
 		}
 
 		if (!is_highmem_idx(j))
@@ -6163,9 +6162,9 @@ void __paginginit free_area_init_node(int nid, unsigned long *zones_size,
 
 	alloc_node_mem_map(pgdat);
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
-	printk(KERN_DEBUG "free_area_init_node: node %d, pgdat %08lx, node_mem_map %08lx\n",
-		nid, (unsigned long)pgdat,
-		(unsigned long)pgdat->node_mem_map);
+	printk(KERN_DEBUG "%s: node %d, pgdat %08lx, node_mem_map %08lx\n",
+	       __func__, nid, (unsigned long)pgdat,
+	       (unsigned long)pgdat->node_mem_map);
 #endif
 
 	free_area_init_core(pgdat);
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-16  1:43   ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel; +Cc: linux-mm

Function calls with large argument counts cause x86-64 register
spilling.  Reducing the number of arguments in a multi-line printk
by converting to multiple printks which saves some object code size.

$ size mm/page_alloc.o* (defconfig)
   text    data     bss     dec     hex filename
  35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
  36018    1699     628   38345    95c9 mm/page_alloc.o.old

Miscellanea:

o Remove line leading spaces from the formerly multi-line printks
  commit a25700a53f71 ("mm: show bounce pages in oom killer output")
  back in 2007 started the leading space when a single long line
  was split into multiple lines but the leading space was likely
  mistakenly kept and subsequent commits followed suit.
o Align arguments in a few more printks

Signed-off-by: Joe Perches <joe@perches.com>
---
 mm/page_alloc.c | 237 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 118 insertions(+), 119 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f749b7ff7c50..5db9710cb932 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4505,79 +4505,79 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 			free_pcp += per_cpu_ptr(zone->pageset, cpu)->pcp.count;
 	}
 
-	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
-		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
-		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
-		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
-		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
-		" free:%lu free_pcp:%lu free_cma:%lu\n",
-		global_node_page_state(NR_ACTIVE_ANON),
-		global_node_page_state(NR_INACTIVE_ANON),
-		global_node_page_state(NR_ISOLATED_ANON),
-		global_node_page_state(NR_ACTIVE_FILE),
-		global_node_page_state(NR_INACTIVE_FILE),
-		global_node_page_state(NR_ISOLATED_FILE),
-		global_node_page_state(NR_UNEVICTABLE),
-		global_node_page_state(NR_FILE_DIRTY),
-		global_node_page_state(NR_WRITEBACK),
-		global_node_page_state(NR_UNSTABLE_NFS),
-		global_page_state(NR_SLAB_RECLAIMABLE),
-		global_page_state(NR_SLAB_UNRECLAIMABLE),
-		global_node_page_state(NR_FILE_MAPPED),
-		global_node_page_state(NR_SHMEM),
-		global_page_state(NR_PAGETABLE),
-		global_page_state(NR_BOUNCE),
-		global_page_state(NR_FREE_PAGES),
-		free_pcp,
-		global_page_state(NR_FREE_CMA_PAGES));
+	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
+	       global_node_page_state(NR_ACTIVE_ANON),
+	       global_node_page_state(NR_INACTIVE_ANON),
+	       global_node_page_state(NR_ISOLATED_ANON));
+	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
+	       global_node_page_state(NR_ACTIVE_FILE),
+	       global_node_page_state(NR_INACTIVE_FILE),
+	       global_node_page_state(NR_ISOLATED_FILE));
+	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
+	       global_node_page_state(NR_UNEVICTABLE),
+	       global_node_page_state(NR_FILE_DIRTY),
+	       global_node_page_state(NR_WRITEBACK),
+	       global_node_page_state(NR_UNSTABLE_NFS));
+	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
+	       global_page_state(NR_SLAB_RECLAIMABLE),
+	       global_page_state(NR_SLAB_UNRECLAIMABLE));
+	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
+	       global_node_page_state(NR_FILE_MAPPED),
+	       global_node_page_state(NR_SHMEM),
+	       global_page_state(NR_PAGETABLE),
+	       global_page_state(NR_BOUNCE));
+	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
+	       global_page_state(NR_FREE_PAGES),
+	       free_pcp,
+	       global_page_state(NR_FREE_CMA_PAGES));
 
 	for_each_online_pgdat(pgdat) {
 		if (show_mem_node_skip(filter, pgdat->node_id, nodemask))
 			continue;
 
 		printk("Node %d"
-			" active_anon:%lukB"
-			" inactive_anon:%lukB"
-			" active_file:%lukB"
-			" inactive_file:%lukB"
-			" unevictable:%lukB"
-			" isolated(anon):%lukB"
-			" isolated(file):%lukB"
-			" mapped:%lukB"
-			" dirty:%lukB"
-			" writeback:%lukB"
-			" shmem:%lukB"
+		       " active_anon:%lukB"
+		       " inactive_anon:%lukB"
+		       " active_file:%lukB"
+		       " inactive_file:%lukB"
+		       " unevictable:%lukB"
+		       " isolated(anon):%lukB"
+		       " isolated(file):%lukB"
+		       " mapped:%lukB"
+		       " dirty:%lukB"
+		       " writeback:%lukB"
+		       " shmem:%lukB"
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			" shmem_thp: %lukB"
-			" shmem_pmdmapped: %lukB"
-			" anon_thp: %lukB"
+		       " shmem_thp: %lukB"
+		       " shmem_pmdmapped: %lukB"
+		       " anon_thp: %lukB"
 #endif
-			" writeback_tmp:%lukB"
-			" unstable:%lukB"
-			" all_unreclaimable? %s"
-			"\n",
-			pgdat->node_id,
-			K(node_page_state(pgdat, NR_ACTIVE_ANON)),
-			K(node_page_state(pgdat, NR_INACTIVE_ANON)),
-			K(node_page_state(pgdat, NR_ACTIVE_FILE)),
-			K(node_page_state(pgdat, NR_INACTIVE_FILE)),
-			K(node_page_state(pgdat, NR_UNEVICTABLE)),
-			K(node_page_state(pgdat, NR_ISOLATED_ANON)),
-			K(node_page_state(pgdat, NR_ISOLATED_FILE)),
-			K(node_page_state(pgdat, NR_FILE_MAPPED)),
-			K(node_page_state(pgdat, NR_FILE_DIRTY)),
-			K(node_page_state(pgdat, NR_WRITEBACK)),
+		       " writeback_tmp:%lukB"
+		       " unstable:%lukB"
+		       " all_unreclaimable? %s"
+		       "\n",
+		       pgdat->node_id,
+		       K(node_page_state(pgdat, NR_ACTIVE_ANON)),
+		       K(node_page_state(pgdat, NR_INACTIVE_ANON)),
+		       K(node_page_state(pgdat, NR_ACTIVE_FILE)),
+		       K(node_page_state(pgdat, NR_INACTIVE_FILE)),
+		       K(node_page_state(pgdat, NR_UNEVICTABLE)),
+		       K(node_page_state(pgdat, NR_ISOLATED_ANON)),
+		       K(node_page_state(pgdat, NR_ISOLATED_FILE)),
+		       K(node_page_state(pgdat, NR_FILE_MAPPED)),
+		       K(node_page_state(pgdat, NR_FILE_DIRTY)),
+		       K(node_page_state(pgdat, NR_WRITEBACK)),
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
-			K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
-					* HPAGE_PMD_NR),
-			K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
+			 * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
 #endif
-			K(node_page_state(pgdat, NR_SHMEM)),
-			K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
-			K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
-			pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
-				"yes" : "no");
+		       K(node_page_state(pgdat, NR_SHMEM)),
+		       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
+		       K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
+		       pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
+		       "yes" : "no");
 	}
 
 	for_each_populated_zone(zone) {
@@ -4592,51 +4592,51 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 
 		show_node(zone);
 		printk(KERN_CONT
-			"%s"
-			" free:%lukB"
-			" min:%lukB"
-			" low:%lukB"
-			" high:%lukB"
-			" active_anon:%lukB"
-			" inactive_anon:%lukB"
-			" active_file:%lukB"
-			" inactive_file:%lukB"
-			" unevictable:%lukB"
-			" writepending:%lukB"
-			" present:%lukB"
-			" managed:%lukB"
-			" mlocked:%lukB"
-			" slab_reclaimable:%lukB"
-			" slab_unreclaimable:%lukB"
-			" kernel_stack:%lukB"
-			" pagetables:%lukB"
-			" bounce:%lukB"
-			" free_pcp:%lukB"
-			" local_pcp:%ukB"
-			" free_cma:%lukB"
-			"\n",
-			zone->name,
-			K(zone_page_state(zone, NR_FREE_PAGES)),
-			K(min_wmark_pages(zone)),
-			K(low_wmark_pages(zone)),
-			K(high_wmark_pages(zone)),
-			K(zone_page_state(zone, NR_ZONE_ACTIVE_ANON)),
-			K(zone_page_state(zone, NR_ZONE_INACTIVE_ANON)),
-			K(zone_page_state(zone, NR_ZONE_ACTIVE_FILE)),
-			K(zone_page_state(zone, NR_ZONE_INACTIVE_FILE)),
-			K(zone_page_state(zone, NR_ZONE_UNEVICTABLE)),
-			K(zone_page_state(zone, NR_ZONE_WRITE_PENDING)),
-			K(zone->present_pages),
-			K(zone->managed_pages),
-			K(zone_page_state(zone, NR_MLOCK)),
-			K(zone_page_state(zone, NR_SLAB_RECLAIMABLE)),
-			K(zone_page_state(zone, NR_SLAB_UNRECLAIMABLE)),
-			zone_page_state(zone, NR_KERNEL_STACK_KB),
-			K(zone_page_state(zone, NR_PAGETABLE)),
-			K(zone_page_state(zone, NR_BOUNCE)),
-			K(free_pcp),
-			K(this_cpu_read(zone->pageset->pcp.count)),
-			K(zone_page_state(zone, NR_FREE_CMA_PAGES)));
+		       "%s"
+		       " free:%lukB"
+		       " min:%lukB"
+		       " low:%lukB"
+		       " high:%lukB"
+		       " active_anon:%lukB"
+		       " inactive_anon:%lukB"
+		       " active_file:%lukB"
+		       " inactive_file:%lukB"
+		       " unevictable:%lukB"
+		       " writepending:%lukB"
+		       " present:%lukB"
+		       " managed:%lukB"
+		       " mlocked:%lukB"
+		       " slab_reclaimable:%lukB"
+		       " slab_unreclaimable:%lukB"
+		       " kernel_stack:%lukB"
+		       " pagetables:%lukB"
+		       " bounce:%lukB"
+		       " free_pcp:%lukB"
+		       " local_pcp:%ukB"
+		       " free_cma:%lukB"
+		       "\n",
+		       zone->name,
+		       K(zone_page_state(zone, NR_FREE_PAGES)),
+		       K(min_wmark_pages(zone)),
+		       K(low_wmark_pages(zone)),
+		       K(high_wmark_pages(zone)),
+		       K(zone_page_state(zone, NR_ZONE_ACTIVE_ANON)),
+		       K(zone_page_state(zone, NR_ZONE_INACTIVE_ANON)),
+		       K(zone_page_state(zone, NR_ZONE_ACTIVE_FILE)),
+		       K(zone_page_state(zone, NR_ZONE_INACTIVE_FILE)),
+		       K(zone_page_state(zone, NR_ZONE_UNEVICTABLE)),
+		       K(zone_page_state(zone, NR_ZONE_WRITE_PENDING)),
+		       K(zone->present_pages),
+		       K(zone->managed_pages),
+		       K(zone_page_state(zone, NR_MLOCK)),
+		       K(zone_page_state(zone, NR_SLAB_RECLAIMABLE)),
+		       K(zone_page_state(zone, NR_SLAB_UNRECLAIMABLE)),
+		       zone_page_state(zone, NR_KERNEL_STACK_KB),
+		       K(zone_page_state(zone, NR_PAGETABLE)),
+		       K(zone_page_state(zone, NR_BOUNCE)),
+		       K(free_pcp),
+		       K(this_cpu_read(zone->pageset->pcp.count)),
+		       K(zone_page_state(zone, NR_FREE_CMA_PAGES)));
 		printk("lowmem_reserve[]:");
 		for (i = 0; i < MAX_NR_ZONES; i++)
 			printk(KERN_CONT " %ld", zone->lowmem_reserve[i]);
@@ -4679,7 +4679,8 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 
 	hugetlb_show_meminfo();
 
-	printk("%ld total pagecache pages\n", global_node_page_state(NR_FILE_PAGES));
+	printk("%ld total pagecache pages\n",
+	       global_node_page_state(NR_FILE_PAGES));
 
 	show_swap_cache_info();
 }
@@ -5516,8 +5517,7 @@ static __meminit void zone_pcp_init(struct zone *zone)
 
 	if (populated_zone(zone))
 		printk(KERN_DEBUG "  %s zone: %lu pages, LIFO batch:%u\n",
-			zone->name, zone->present_pages,
-					 zone_batchsize(zone));
+		       zone->name, zone->present_pages, zone_batchsize(zone));
 }
 
 int __meminit init_currently_empty_zone(struct zone *zone,
@@ -5891,8 +5891,8 @@ static void __meminit calculate_node_totalpages(struct pglist_data *pgdat,
 
 	pgdat->node_spanned_pages = totalpages;
 	pgdat->node_present_pages = realtotalpages;
-	printk(KERN_DEBUG "On node %d totalpages: %lu\n", pgdat->node_id,
-							realtotalpages);
+	printk(KERN_DEBUG "On node %d totalpages: %lu\n",
+	       pgdat->node_id, realtotalpages);
 }
 
 #ifndef CONFIG_SPARSEMEM
@@ -6042,8 +6042,7 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 			if (freesize >= memmap_pages) {
 				freesize -= memmap_pages;
 				if (memmap_pages)
-					printk(KERN_DEBUG
-					       "  %s zone: %lu pages used for memmap\n",
+					printk(KERN_DEBUG "  %s zone: %lu pages used for memmap\n",
 					       zone_names[j], memmap_pages);
 			} else
 				pr_warn("  %s zone: %lu pages exceeds freesize %lu\n",
@@ -6054,7 +6053,7 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 		if (j == 0 && freesize > dma_reserve) {
 			freesize -= dma_reserve;
 			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
-					zone_names[0], dma_reserve);
+			       zone_names[0], dma_reserve);
 		}
 
 		if (!is_highmem_idx(j))
@@ -6163,9 +6162,9 @@ void __paginginit free_area_init_node(int nid, unsigned long *zones_size,
 
 	alloc_node_mem_map(pgdat);
 #ifdef CONFIG_FLAT_NODE_MEM_MAP
-	printk(KERN_DEBUG "free_area_init_node: node %d, pgdat %08lx, node_mem_map %08lx\n",
-		nid, (unsigned long)pgdat,
-		(unsigned long)pgdat->node_mem_map);
+	printk(KERN_DEBUG "%s: node %d, pgdat %08lx, node_mem_map %08lx\n",
+	       __func__, nid, (unsigned long)pgdat,
+	       (unsigned long)pgdat->node_mem_map);
 #endif
 
 	free_area_init_core(pgdat);
-- 
2.10.0.rc2.1.g053435c

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size
  2017-03-16  1:43 ` Joe Perches
@ 2017-03-16  1:43   ` Joe Perches
  -1 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel; +Cc: linux-mm

When CONFIG_TRANSPARENT_HUGEPAGE is set, there is an output defect
where the values emitted do not match the textual descriptions.

Reorder the arguments appropriately.

As with commit f5f93a2657ab ("mm: page_alloc: Reduce object size
by neatening printks"), register spilling occurs when there are
a large number of arguments to a function call.

$ size mm/page_alloc.o* (defconfig)
   text    data     bss     dec     hex filename
  35874    1699     628   38201    9539 mm/page_alloc.o.new
  35914    1699     628   38241    9561 mm/page_alloc.o.old

Miscellanea:

o Break up the long printk into multiple printk and printk(KERN_CONT
  calls to avoid register spilling

Signed-off-by: Joe Perches <joe@perches.com>
---
 mm/page_alloc.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5db9710cb932..6816bb167394 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4540,40 +4540,41 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 		       " inactive_anon:%lukB"
 		       " active_file:%lukB"
 		       " inactive_file:%lukB"
-		       " unevictable:%lukB"
-		       " isolated(anon):%lukB"
-		       " isolated(file):%lukB"
-		       " mapped:%lukB"
-		       " dirty:%lukB"
-		       " writeback:%lukB"
-		       " shmem:%lukB"
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		       " shmem_thp: %lukB"
-		       " shmem_pmdmapped: %lukB"
-		       " anon_thp: %lukB"
-#endif
-		       " writeback_tmp:%lukB"
-		       " unstable:%lukB"
-		       " all_unreclaimable? %s"
-		       "\n",
+		       " unevictable:%lukB",
 		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_ACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_INACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_ACTIVE_FILE)),
 		       K(node_page_state(pgdat, NR_INACTIVE_FILE)),
-		       K(node_page_state(pgdat, NR_UNEVICTABLE)),
+		       K(node_page_state(pgdat, NR_UNEVICTABLE)));
+		printk(KERN_CONT
+		       " isolated(anon):%lukB"
+		       " isolated(file):%lukB"
+		       " mapped:%lukB"
+		       " dirty:%lukB"
+		       " writeback:%lukB"
+		       " shmem:%lukB",
 		       K(node_page_state(pgdat, NR_ISOLATED_ANON)),
 		       K(node_page_state(pgdat, NR_ISOLATED_FILE)),
 		       K(node_page_state(pgdat, NR_FILE_MAPPED)),
 		       K(node_page_state(pgdat, NR_FILE_DIRTY)),
 		       K(node_page_state(pgdat, NR_WRITEBACK)),
+		       K(node_page_state(pgdat, NR_SHMEM)));
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
+		printk(KERN_CONT
+		       " shmem_thp: %lukB"
+		       " shmem_pmdmapped: %lukB"
+		       " anon_thp: %lukB",
 		       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
 		       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
 			 * HPAGE_PMD_NR),
-		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR));
 #endif
-		       K(node_page_state(pgdat, NR_SHMEM)),
+		printk(KERN_CONT
+		       " writeback_tmp:%lukB"
+		       " unstable:%lukB"
+		       " all_unreclaimable? %s"
+		       "\n",
 		       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
 		       K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
 		       pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size
@ 2017-03-16  1:43   ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel; +Cc: linux-mm

When CONFIG_TRANSPARENT_HUGEPAGE is set, there is an output defect
where the values emitted do not match the textual descriptions.

Reorder the arguments appropriately.

As with commit f5f93a2657ab ("mm: page_alloc: Reduce object size
by neatening printks"), register spilling occurs when there are
a large number of arguments to a function call.

$ size mm/page_alloc.o* (defconfig)
   text    data     bss     dec     hex filename
  35874    1699     628   38201    9539 mm/page_alloc.o.new
  35914    1699     628   38241    9561 mm/page_alloc.o.old

Miscellanea:

o Break up the long printk into multiple printk and printk(KERN_CONT
  calls to avoid register spilling

Signed-off-by: Joe Perches <joe@perches.com>
---
 mm/page_alloc.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5db9710cb932..6816bb167394 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4540,40 +4540,41 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 		       " inactive_anon:%lukB"
 		       " active_file:%lukB"
 		       " inactive_file:%lukB"
-		       " unevictable:%lukB"
-		       " isolated(anon):%lukB"
-		       " isolated(file):%lukB"
-		       " mapped:%lukB"
-		       " dirty:%lukB"
-		       " writeback:%lukB"
-		       " shmem:%lukB"
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		       " shmem_thp: %lukB"
-		       " shmem_pmdmapped: %lukB"
-		       " anon_thp: %lukB"
-#endif
-		       " writeback_tmp:%lukB"
-		       " unstable:%lukB"
-		       " all_unreclaimable? %s"
-		       "\n",
+		       " unevictable:%lukB",
 		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_ACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_INACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_ACTIVE_FILE)),
 		       K(node_page_state(pgdat, NR_INACTIVE_FILE)),
-		       K(node_page_state(pgdat, NR_UNEVICTABLE)),
+		       K(node_page_state(pgdat, NR_UNEVICTABLE)));
+		printk(KERN_CONT
+		       " isolated(anon):%lukB"
+		       " isolated(file):%lukB"
+		       " mapped:%lukB"
+		       " dirty:%lukB"
+		       " writeback:%lukB"
+		       " shmem:%lukB",
 		       K(node_page_state(pgdat, NR_ISOLATED_ANON)),
 		       K(node_page_state(pgdat, NR_ISOLATED_FILE)),
 		       K(node_page_state(pgdat, NR_FILE_MAPPED)),
 		       K(node_page_state(pgdat, NR_FILE_DIRTY)),
 		       K(node_page_state(pgdat, NR_WRITEBACK)),
+		       K(node_page_state(pgdat, NR_SHMEM)));
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
+		printk(KERN_CONT
+		       " shmem_thp: %lukB"
+		       " shmem_pmdmapped: %lukB"
+		       " anon_thp: %lukB",
 		       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
 		       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
 			 * HPAGE_PMD_NR),
-		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR),
+		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR));
 #endif
-		       K(node_page_state(pgdat, NR_SHMEM)),
+		printk(KERN_CONT
+		       " writeback_tmp:%lukB"
+		       " unstable:%lukB"
+		       " all_unreclaimable? %s"
+		       "\n",
 		       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
 		       K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
 		       pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
-- 
2.10.0.rc2.1.g053435c

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] mm: page_alloc: Break up a long single-line printk
  2017-03-16  1:43 ` Joe Perches
@ 2017-03-16  1:43   ` Joe Perches
  -1 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel; +Cc: linux-mm

Blocked multiple line output is easier to read than an
extremely long single line.

Miscellanea:

o Add "Node" prefix to each new line of the block

Signed-off-by: Joe Perches <joe@perches.com>
---
 mm/page_alloc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6816bb167394..2d3c10734874 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4540,20 +4540,23 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 		       " inactive_anon:%lukB"
 		       " active_file:%lukB"
 		       " inactive_file:%lukB"
-		       " unevictable:%lukB",
+		       " unevictable:%lukB"
+		       "\n",
 		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_ACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_INACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_ACTIVE_FILE)),
 		       K(node_page_state(pgdat, NR_INACTIVE_FILE)),
 		       K(node_page_state(pgdat, NR_UNEVICTABLE)));
-		printk(KERN_CONT
+		printk("Node %d"
 		       " isolated(anon):%lukB"
 		       " isolated(file):%lukB"
 		       " mapped:%lukB"
 		       " dirty:%lukB"
 		       " writeback:%lukB"
-		       " shmem:%lukB",
+		       " shmem:%lukB"
+		       "\n",
+		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_ISOLATED_ANON)),
 		       K(node_page_state(pgdat, NR_ISOLATED_FILE)),
 		       K(node_page_state(pgdat, NR_FILE_MAPPED)),
@@ -4561,20 +4564,23 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 		       K(node_page_state(pgdat, NR_WRITEBACK)),
 		       K(node_page_state(pgdat, NR_SHMEM)));
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		printk(KERN_CONT
+		printk("Node %d"
 		       " shmem_thp: %lukB"
 		       " shmem_pmdmapped: %lukB"
-		       " anon_thp: %lukB",
+		       " anon_thp: %lukB"
+		       "\n",
+		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
 		       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
 			 * HPAGE_PMD_NR),
 		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR));
 #endif
-		printk(KERN_CONT
+		printk("Node %d"
 		       " writeback_tmp:%lukB"
 		       " unstable:%lukB"
 		       " all_unreclaimable? %s"
 		       "\n",
+		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
 		       K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
 		       pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] mm: page_alloc: Break up a long single-line printk
@ 2017-03-16  1:43   ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16  1:43 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel; +Cc: linux-mm

Blocked multiple line output is easier to read than an
extremely long single line.

Miscellanea:

o Add "Node" prefix to each new line of the block

Signed-off-by: Joe Perches <joe@perches.com>
---
 mm/page_alloc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6816bb167394..2d3c10734874 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4540,20 +4540,23 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 		       " inactive_anon:%lukB"
 		       " active_file:%lukB"
 		       " inactive_file:%lukB"
-		       " unevictable:%lukB",
+		       " unevictable:%lukB"
+		       "\n",
 		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_ACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_INACTIVE_ANON)),
 		       K(node_page_state(pgdat, NR_ACTIVE_FILE)),
 		       K(node_page_state(pgdat, NR_INACTIVE_FILE)),
 		       K(node_page_state(pgdat, NR_UNEVICTABLE)));
-		printk(KERN_CONT
+		printk("Node %d"
 		       " isolated(anon):%lukB"
 		       " isolated(file):%lukB"
 		       " mapped:%lukB"
 		       " dirty:%lukB"
 		       " writeback:%lukB"
-		       " shmem:%lukB",
+		       " shmem:%lukB"
+		       "\n",
+		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_ISOLATED_ANON)),
 		       K(node_page_state(pgdat, NR_ISOLATED_FILE)),
 		       K(node_page_state(pgdat, NR_FILE_MAPPED)),
@@ -4561,20 +4564,23 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 		       K(node_page_state(pgdat, NR_WRITEBACK)),
 		       K(node_page_state(pgdat, NR_SHMEM)));
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		printk(KERN_CONT
+		printk("Node %d"
 		       " shmem_thp: %lukB"
 		       " shmem_pmdmapped: %lukB"
-		       " anon_thp: %lukB",
+		       " anon_thp: %lukB"
+		       "\n",
+		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_SHMEM_THPS) * HPAGE_PMD_NR),
 		       K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED)
 			 * HPAGE_PMD_NR),
 		       K(node_page_state(pgdat, NR_ANON_THPS) * HPAGE_PMD_NR));
 #endif
-		printk(KERN_CONT
+		printk("Node %d"
 		       " writeback_tmp:%lukB"
 		       " unstable:%lukB"
 		       " all_unreclaimable? %s"
 		       "\n",
+		       pgdat->node_id,
 		       K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
 		       K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
 		       pgdat->kswapd_failures >= MAX_RECLAIM_RETRIES ?
-- 
2.10.0.rc2.1.g053435c

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16  1:43   ` Joe Perches
@ 2017-03-16 10:56     ` Michal Hocko
  -1 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-16 10:56 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Wed 15-03-17 18:43:13, Joe Perches wrote:
> Function calls with large argument counts cause x86-64 register
> spilling.  Reducing the number of arguments in a multi-line printk
> by converting to multiple printks which saves some object code size.
> 
> $ size mm/page_alloc.o* (defconfig)
>    text    data     bss     dec     hex filename
>   35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
>   36018    1699     628   38345    95c9 mm/page_alloc.o.old
> 
> Miscellanea:
> 
> o Remove line leading spaces from the formerly multi-line printks
>   commit a25700a53f71 ("mm: show bounce pages in oom killer output")
>   back in 2007 started the leading space when a single long line
>   was split into multiple lines but the leading space was likely
>   mistakenly kept and subsequent commits followed suit.
> o Align arguments in a few more printks

This is really hard to review. Could you just drop all the whitespace
changes please?
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-16 10:56     ` Michal Hocko
  0 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-16 10:56 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Wed 15-03-17 18:43:13, Joe Perches wrote:
> Function calls with large argument counts cause x86-64 register
> spilling.  Reducing the number of arguments in a multi-line printk
> by converting to multiple printks which saves some object code size.
> 
> $ size mm/page_alloc.o* (defconfig)
>    text    data     bss     dec     hex filename
>   35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
>   36018    1699     628   38345    95c9 mm/page_alloc.o.old
> 
> Miscellanea:
> 
> o Remove line leading spaces from the formerly multi-line printks
>   commit a25700a53f71 ("mm: show bounce pages in oom killer output")
>   back in 2007 started the leading space when a single long line
>   was split into multiple lines but the leading space was likely
>   mistakenly kept and subsequent commits followed suit.
> o Align arguments in a few more printks

This is really hard to review. Could you just drop all the whitespace
changes please?
-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size
  2017-03-16  1:43   ` Joe Perches
@ 2017-03-16 10:57     ` Michal Hocko
  -1 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-16 10:57 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Wed 15-03-17 18:43:14, Joe Perches wrote:
> When CONFIG_TRANSPARENT_HUGEPAGE is set, there is an output defect
> where the values emitted do not match the textual descriptions.

please separate this out to one patch without all other changes.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size
@ 2017-03-16 10:57     ` Michal Hocko
  0 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-16 10:57 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Wed 15-03-17 18:43:14, Joe Perches wrote:
> When CONFIG_TRANSPARENT_HUGEPAGE is set, there is an output defect
> where the values emitted do not match the textual descriptions.

please separate this out to one patch without all other changes.
-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mm: page_alloc: Break up a long single-line printk
  2017-03-16  1:43   ` Joe Perches
@ 2017-03-16 10:58     ` Michal Hocko
  -1 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-16 10:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Wed 15-03-17 18:43:15, Joe Perches wrote:
> Blocked multiple line output is easier to read than an
> extremely long single line.

I am not really sure this is an improvemnt. If anything add an output
before and after to the changelog.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3/3] mm: page_alloc: Break up a long single-line printk
@ 2017-03-16 10:58     ` Michal Hocko
  0 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-16 10:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Wed 15-03-17 18:43:15, Joe Perches wrote:
> Blocked multiple line output is easier to read than an
> extremely long single line.

I am not really sure this is an improvemnt. If anything add an output
before and after to the changelog.
-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16  1:43   ` Joe Perches
@ 2017-03-16 11:30     ` Sergey Senozhatsky
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Senozhatsky @ 2017-03-16 11:30 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On (03/15/17 18:43), Joe Perches wrote:
[..]
> -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> -		global_node_page_state(NR_ACTIVE_ANON),
> -		global_node_page_state(NR_INACTIVE_ANON),
> -		global_node_page_state(NR_ISOLATED_ANON),
> -		global_node_page_state(NR_ACTIVE_FILE),
> -		global_node_page_state(NR_INACTIVE_FILE),
> -		global_node_page_state(NR_ISOLATED_FILE),
> -		global_node_page_state(NR_UNEVICTABLE),
> -		global_node_page_state(NR_FILE_DIRTY),
> -		global_node_page_state(NR_WRITEBACK),
> -		global_node_page_state(NR_UNSTABLE_NFS),
> -		global_page_state(NR_SLAB_RECLAIMABLE),
> -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> -		global_node_page_state(NR_FILE_MAPPED),
> -		global_node_page_state(NR_SHMEM),
> -		global_page_state(NR_PAGETABLE),
> -		global_page_state(NR_BOUNCE),
> -		global_page_state(NR_FREE_PAGES),
> -		free_pcp,
> -		global_page_state(NR_FREE_CMA_PAGES));
> +	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
> +	       global_node_page_state(NR_ACTIVE_ANON),
> +	       global_node_page_state(NR_INACTIVE_ANON),
> +	       global_node_page_state(NR_ISOLATED_ANON));
> +	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
> +	       global_node_page_state(NR_ACTIVE_FILE),
> +	       global_node_page_state(NR_INACTIVE_FILE),
> +	       global_node_page_state(NR_ISOLATED_FILE));
> +	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
> +	       global_node_page_state(NR_UNEVICTABLE),
> +	       global_node_page_state(NR_FILE_DIRTY),
> +	       global_node_page_state(NR_WRITEBACK),
> +	       global_node_page_state(NR_UNSTABLE_NFS));
> +	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
> +	       global_page_state(NR_SLAB_RECLAIMABLE),
> +	       global_page_state(NR_SLAB_UNRECLAIMABLE));
> +	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
> +	       global_node_page_state(NR_FILE_MAPPED),
> +	       global_node_page_state(NR_SHMEM),
> +	       global_page_state(NR_PAGETABLE),
> +	       global_page_state(NR_BOUNCE));
> +	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
> +	       global_page_state(NR_FREE_PAGES),
> +	       free_pcp,
> +	       global_page_state(NR_FREE_CMA_PAGES));

a side note:

this can make it harder to read, in _the worst case_. one printk()
guaranteed that we would see a single line in the serial log/etc.
the sort of a problem with multiple printks is that printks coming
from other CPUs will split that "previously single" line.

just a notice. up to MM people to decide.

	-ss

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-16 11:30     ` Sergey Senozhatsky
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Senozhatsky @ 2017-03-16 11:30 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On (03/15/17 18:43), Joe Perches wrote:
[..]
> -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> -		global_node_page_state(NR_ACTIVE_ANON),
> -		global_node_page_state(NR_INACTIVE_ANON),
> -		global_node_page_state(NR_ISOLATED_ANON),
> -		global_node_page_state(NR_ACTIVE_FILE),
> -		global_node_page_state(NR_INACTIVE_FILE),
> -		global_node_page_state(NR_ISOLATED_FILE),
> -		global_node_page_state(NR_UNEVICTABLE),
> -		global_node_page_state(NR_FILE_DIRTY),
> -		global_node_page_state(NR_WRITEBACK),
> -		global_node_page_state(NR_UNSTABLE_NFS),
> -		global_page_state(NR_SLAB_RECLAIMABLE),
> -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> -		global_node_page_state(NR_FILE_MAPPED),
> -		global_node_page_state(NR_SHMEM),
> -		global_page_state(NR_PAGETABLE),
> -		global_page_state(NR_BOUNCE),
> -		global_page_state(NR_FREE_PAGES),
> -		free_pcp,
> -		global_page_state(NR_FREE_CMA_PAGES));
> +	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
> +	       global_node_page_state(NR_ACTIVE_ANON),
> +	       global_node_page_state(NR_INACTIVE_ANON),
> +	       global_node_page_state(NR_ISOLATED_ANON));
> +	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
> +	       global_node_page_state(NR_ACTIVE_FILE),
> +	       global_node_page_state(NR_INACTIVE_FILE),
> +	       global_node_page_state(NR_ISOLATED_FILE));
> +	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
> +	       global_node_page_state(NR_UNEVICTABLE),
> +	       global_node_page_state(NR_FILE_DIRTY),
> +	       global_node_page_state(NR_WRITEBACK),
> +	       global_node_page_state(NR_UNSTABLE_NFS));
> +	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
> +	       global_page_state(NR_SLAB_RECLAIMABLE),
> +	       global_page_state(NR_SLAB_UNRECLAIMABLE));
> +	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
> +	       global_node_page_state(NR_FILE_MAPPED),
> +	       global_node_page_state(NR_SHMEM),
> +	       global_page_state(NR_PAGETABLE),
> +	       global_page_state(NR_BOUNCE));
> +	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
> +	       global_page_state(NR_FREE_PAGES),
> +	       free_pcp,
> +	       global_page_state(NR_FREE_CMA_PAGES));

a side note:

this can make it harder to read, in _the worst case_. one printk()
guaranteed that we would see a single line in the serial log/etc.
the sort of a problem with multiple printks is that printks coming
from other CPUs will split that "previously single" line.

just a notice. up to MM people to decide.

	-ss

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16 11:30     ` Sergey Senozhatsky
@ 2017-03-16 18:37       ` Joe Perches
  -1 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16 18:37 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, linux-kernel, linux-mm

On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> On (03/15/17 18:43), Joe Perches wrote:
> [..]
> > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > -		global_node_page_state(NR_ACTIVE_ANON),
> > -		global_node_page_state(NR_INACTIVE_ANON),
> > -		global_node_page_state(NR_ISOLATED_ANON),
> > -		global_node_page_state(NR_ACTIVE_FILE),
> > -		global_node_page_state(NR_INACTIVE_FILE),
> > -		global_node_page_state(NR_ISOLATED_FILE),
> > -		global_node_page_state(NR_UNEVICTABLE),
> > -		global_node_page_state(NR_FILE_DIRTY),
> > -		global_node_page_state(NR_WRITEBACK),
> > -		global_node_page_state(NR_UNSTABLE_NFS),
> > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > -		global_node_page_state(NR_FILE_MAPPED),
> > -		global_node_page_state(NR_SHMEM),
> > -		global_page_state(NR_PAGETABLE),
> > -		global_page_state(NR_BOUNCE),
> > -		global_page_state(NR_FREE_PAGES),
> > -		free_pcp,
> > -		global_page_state(NR_FREE_CMA_PAGES));
> > +	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
> > +	       global_node_page_state(NR_ACTIVE_ANON),
> > +	       global_node_page_state(NR_INACTIVE_ANON),
> > +	       global_node_page_state(NR_ISOLATED_ANON));
> > +	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
> > +	       global_node_page_state(NR_ACTIVE_FILE),
> > +	       global_node_page_state(NR_INACTIVE_FILE),
> > +	       global_node_page_state(NR_ISOLATED_FILE));
> > +	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
> > +	       global_node_page_state(NR_UNEVICTABLE),
> > +	       global_node_page_state(NR_FILE_DIRTY),
> > +	       global_node_page_state(NR_WRITEBACK),
> > +	       global_node_page_state(NR_UNSTABLE_NFS));
> > +	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
> > +	       global_page_state(NR_SLAB_RECLAIMABLE),
> > +	       global_page_state(NR_SLAB_UNRECLAIMABLE));
> > +	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
> > +	       global_node_page_state(NR_FILE_MAPPED),
> > +	       global_node_page_state(NR_SHMEM),
> > +	       global_page_state(NR_PAGETABLE),
> > +	       global_page_state(NR_BOUNCE));
> > +	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
> > +	       global_page_state(NR_FREE_PAGES),
> > +	       free_pcp,
> > +	       global_page_state(NR_FREE_CMA_PAGES));
> 
> a side note:
> 
> this can make it harder to read, in _the worst case_. one printk()
> guaranteed that we would see a single line in the serial log/etc.
> the sort of a problem with multiple printks is that printks coming
> from other CPUs will split that "previously single" line.

Not true.  Note the multiple \n uses in the original code.

> just a notice. up to MM people to decide.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-16 18:37       ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16 18:37 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, linux-kernel, linux-mm

On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> On (03/15/17 18:43), Joe Perches wrote:
> [..]
> > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > -		global_node_page_state(NR_ACTIVE_ANON),
> > -		global_node_page_state(NR_INACTIVE_ANON),
> > -		global_node_page_state(NR_ISOLATED_ANON),
> > -		global_node_page_state(NR_ACTIVE_FILE),
> > -		global_node_page_state(NR_INACTIVE_FILE),
> > -		global_node_page_state(NR_ISOLATED_FILE),
> > -		global_node_page_state(NR_UNEVICTABLE),
> > -		global_node_page_state(NR_FILE_DIRTY),
> > -		global_node_page_state(NR_WRITEBACK),
> > -		global_node_page_state(NR_UNSTABLE_NFS),
> > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > -		global_node_page_state(NR_FILE_MAPPED),
> > -		global_node_page_state(NR_SHMEM),
> > -		global_page_state(NR_PAGETABLE),
> > -		global_page_state(NR_BOUNCE),
> > -		global_page_state(NR_FREE_PAGES),
> > -		free_pcp,
> > -		global_page_state(NR_FREE_CMA_PAGES));
> > +	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
> > +	       global_node_page_state(NR_ACTIVE_ANON),
> > +	       global_node_page_state(NR_INACTIVE_ANON),
> > +	       global_node_page_state(NR_ISOLATED_ANON));
> > +	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
> > +	       global_node_page_state(NR_ACTIVE_FILE),
> > +	       global_node_page_state(NR_INACTIVE_FILE),
> > +	       global_node_page_state(NR_ISOLATED_FILE));
> > +	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
> > +	       global_node_page_state(NR_UNEVICTABLE),
> > +	       global_node_page_state(NR_FILE_DIRTY),
> > +	       global_node_page_state(NR_WRITEBACK),
> > +	       global_node_page_state(NR_UNSTABLE_NFS));
> > +	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
> > +	       global_page_state(NR_SLAB_RECLAIMABLE),
> > +	       global_page_state(NR_SLAB_UNRECLAIMABLE));
> > +	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
> > +	       global_node_page_state(NR_FILE_MAPPED),
> > +	       global_node_page_state(NR_SHMEM),
> > +	       global_page_state(NR_PAGETABLE),
> > +	       global_page_state(NR_BOUNCE));
> > +	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
> > +	       global_page_state(NR_FREE_PAGES),
> > +	       free_pcp,
> > +	       global_page_state(NR_FREE_CMA_PAGES));
> 
> a side note:
> 
> this can make it harder to read, in _the worst case_. one printk()
> guaranteed that we would see a single line in the serial log/etc.
> the sort of a problem with multiple printks is that printks coming
> from other CPUs will split that "previously single" line.

Not true.  Note the multiple \n uses in the original code.

> just a notice. up to MM people to decide.


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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16 10:56     ` Michal Hocko
@ 2017-03-16 20:32       ` Joe Perches
  -1 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16 20:32 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Andrew Morton, linux-kernel, linux-mm

On Thu, 2017-03-16 at 11:56 +0100, Michal Hocko wrote:
> On Wed 15-03-17 18:43:13, Joe Perches wrote:
> > Function calls with large argument counts cause x86-64 register
> > spilling.  Reducing the number of arguments in a multi-line printk
> > by converting to multiple printks which saves some object code size.
> > 
> > $ size mm/page_alloc.o* (defconfig)
> >    text    data     bss     dec     hex filename
> >   35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
> >   36018    1699     628   38345    95c9 mm/page_alloc.o.old
> > 
> > Miscellanea:
> > 
> > o Remove line leading spaces from the formerly multi-line printks
> >   commit a25700a53f71 ("mm: show bounce pages in oom killer output")
> >   back in 2007 started the leading space when a single long line
> >   was split into multiple lines but the leading space was likely
> >   mistakenly kept and subsequent commits followed suit.
> > o Align arguments in a few more printks
> 
> This is really hard to review. Could you just drop all the whitespace
> changes please?

It's a single, simple change.  It's IMO trivial to review.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-16 20:32       ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-16 20:32 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Andrew Morton, linux-kernel, linux-mm

On Thu, 2017-03-16 at 11:56 +0100, Michal Hocko wrote:
> On Wed 15-03-17 18:43:13, Joe Perches wrote:
> > Function calls with large argument counts cause x86-64 register
> > spilling.  Reducing the number of arguments in a multi-line printk
> > by converting to multiple printks which saves some object code size.
> > 
> > $ size mm/page_alloc.o* (defconfig)
> >    text    data     bss     dec     hex filename
> >   35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
> >   36018    1699     628   38345    95c9 mm/page_alloc.o.old
> > 
> > Miscellanea:
> > 
> > o Remove line leading spaces from the formerly multi-line printks
> >   commit a25700a53f71 ("mm: show bounce pages in oom killer output")
> >   back in 2007 started the leading space when a single long line
> >   was split into multiple lines but the leading space was likely
> >   mistakenly kept and subsequent commits followed suit.
> > o Align arguments in a few more printks
> 
> This is really hard to review. Could you just drop all the whitespace
> changes please?

It's a single, simple change.  It's IMO trivial to review.

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16 18:37       ` Joe Perches
@ 2017-03-16 22:53         ` Andrew Morton
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Morton @ 2017-03-16 22:53 UTC (permalink / raw)
  To: Joe Perches; +Cc: Sergey Senozhatsky, linux-kernel, linux-mm

On Thu, 16 Mar 2017 11:37:56 -0700 Joe Perches <joe@perches.com> wrote:

> > this can make it harder to read, in _the worst case_. one printk()
> > guaranteed that we would see a single line in the serial log/etc.
> > the sort of a problem with multiple printks is that printks coming
> > from other CPUs will split that "previously single" line.
> 
> Not true.  Note the multiple \n uses in the original code.

hm?  Won't printk("a\na") atomically emit all three chars into the log
buffer?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-16 22:53         ` Andrew Morton
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Morton @ 2017-03-16 22:53 UTC (permalink / raw)
  To: Joe Perches; +Cc: Sergey Senozhatsky, linux-kernel, linux-mm

On Thu, 16 Mar 2017 11:37:56 -0700 Joe Perches <joe@perches.com> wrote:

> > this can make it harder to read, in _the worst case_. one printk()
> > guaranteed that we would see a single line in the serial log/etc.
> > the sort of a problem with multiple printks is that printks coming
> > from other CPUs will split that "previously single" line.
> 
> Not true.  Note the multiple \n uses in the original code.

hm?  Won't printk("a\na") atomically emit all three chars into the log
buffer?

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16 18:37       ` Joe Perches
@ 2017-03-17  1:56         ` Sergey Senozhatsky
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Senozhatsky @ 2017-03-17  1:56 UTC (permalink / raw)
  To: Joe Perches; +Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm

On (03/16/17 11:37), Joe Perches wrote:
> On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> > On (03/15/17 18:43), Joe Perches wrote:
> > [..]
> > > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > > -		global_node_page_state(NR_ACTIVE_ANON),
> > > -		global_node_page_state(NR_INACTIVE_ANON),
> > > -		global_node_page_state(NR_ISOLATED_ANON),
> > > -		global_node_page_state(NR_ACTIVE_FILE),
> > > -		global_node_page_state(NR_INACTIVE_FILE),
> > > -		global_node_page_state(NR_ISOLATED_FILE),
> > > -		global_node_page_state(NR_UNEVICTABLE),
> > > -		global_node_page_state(NR_FILE_DIRTY),
> > > -		global_node_page_state(NR_WRITEBACK),
> > > -		global_node_page_state(NR_UNSTABLE_NFS),
> > > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > > -		global_node_page_state(NR_FILE_MAPPED),
> > > -		global_node_page_state(NR_SHMEM),
> > > -		global_page_state(NR_PAGETABLE),
> > > -		global_page_state(NR_BOUNCE),
> > > -		global_page_state(NR_FREE_PAGES),
> > > -		free_pcp,
> > > -		global_page_state(NR_FREE_CMA_PAGES));
> > > +	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
> > > +	       global_node_page_state(NR_ACTIVE_ANON),
> > > +	       global_node_page_state(NR_INACTIVE_ANON),
> > > +	       global_node_page_state(NR_ISOLATED_ANON));
> > > +	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
> > > +	       global_node_page_state(NR_ACTIVE_FILE),
> > > +	       global_node_page_state(NR_INACTIVE_FILE),
> > > +	       global_node_page_state(NR_ISOLATED_FILE));
> > > +	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
> > > +	       global_node_page_state(NR_UNEVICTABLE),
> > > +	       global_node_page_state(NR_FILE_DIRTY),
> > > +	       global_node_page_state(NR_WRITEBACK),
> > > +	       global_node_page_state(NR_UNSTABLE_NFS));
> > > +	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
> > > +	       global_page_state(NR_SLAB_RECLAIMABLE),
> > > +	       global_page_state(NR_SLAB_UNRECLAIMABLE));
> > > +	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
> > > +	       global_node_page_state(NR_FILE_MAPPED),
> > > +	       global_node_page_state(NR_SHMEM),
> > > +	       global_page_state(NR_PAGETABLE),
> > > +	       global_page_state(NR_BOUNCE));
> > > +	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
> > > +	       global_page_state(NR_FREE_PAGES),
> > > +	       free_pcp,
> > > +	       global_page_state(NR_FREE_CMA_PAGES));
> > 
> > a side note:
> > 
> > this can make it harder to read, in _the worst case_. one printk()
> > guaranteed that we would see a single line in the serial log/etc.
> > the sort of a problem with multiple printks is that printks coming
> > from other CPUs will split that "previously single" line.
> 
> Not true.  Note the multiple \n uses in the original code.

one printk call ends up in logbuf as a single entry and, thus, we print
it to the serial console in one shot (what is the correct english word
to use here?). multiple printks result in multiple logbuf entries, and
printks from other CPUs can mix in.

so the difference is:


	CPU0						CPU1
							printk(foo\n)
printk(..isolated_anon\n...isolated_file\n...)
							printk(bar\n)

vs

	CPU0						CPU1
printk(..isolated_anon\n)
							printk(foo\n)
printk(...isolated_file\n)
							printk(bar\n)
printk(...\n)

not the same thing.

and the slower the serial console is the more messages potentially
can appear between "..isolated_anon\n" and "...isolated_file\n".

	-ss

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-17  1:56         ` Sergey Senozhatsky
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Senozhatsky @ 2017-03-17  1:56 UTC (permalink / raw)
  To: Joe Perches; +Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm

On (03/16/17 11:37), Joe Perches wrote:
> On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> > On (03/15/17 18:43), Joe Perches wrote:
> > [..]
> > > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > > -		global_node_page_state(NR_ACTIVE_ANON),
> > > -		global_node_page_state(NR_INACTIVE_ANON),
> > > -		global_node_page_state(NR_ISOLATED_ANON),
> > > -		global_node_page_state(NR_ACTIVE_FILE),
> > > -		global_node_page_state(NR_INACTIVE_FILE),
> > > -		global_node_page_state(NR_ISOLATED_FILE),
> > > -		global_node_page_state(NR_UNEVICTABLE),
> > > -		global_node_page_state(NR_FILE_DIRTY),
> > > -		global_node_page_state(NR_WRITEBACK),
> > > -		global_node_page_state(NR_UNSTABLE_NFS),
> > > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > > -		global_node_page_state(NR_FILE_MAPPED),
> > > -		global_node_page_state(NR_SHMEM),
> > > -		global_page_state(NR_PAGETABLE),
> > > -		global_page_state(NR_BOUNCE),
> > > -		global_page_state(NR_FREE_PAGES),
> > > -		free_pcp,
> > > -		global_page_state(NR_FREE_CMA_PAGES));
> > > +	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n",
> > > +	       global_node_page_state(NR_ACTIVE_ANON),
> > > +	       global_node_page_state(NR_INACTIVE_ANON),
> > > +	       global_node_page_state(NR_ISOLATED_ANON));
> > > +	printk("active_file:%lu inactive_file:%lu isolated_file:%lu\n",
> > > +	       global_node_page_state(NR_ACTIVE_FILE),
> > > +	       global_node_page_state(NR_INACTIVE_FILE),
> > > +	       global_node_page_state(NR_ISOLATED_FILE));
> > > +	printk("unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n",
> > > +	       global_node_page_state(NR_UNEVICTABLE),
> > > +	       global_node_page_state(NR_FILE_DIRTY),
> > > +	       global_node_page_state(NR_WRITEBACK),
> > > +	       global_node_page_state(NR_UNSTABLE_NFS));
> > > +	printk("slab_reclaimable:%lu slab_unreclaimable:%lu\n",
> > > +	       global_page_state(NR_SLAB_RECLAIMABLE),
> > > +	       global_page_state(NR_SLAB_UNRECLAIMABLE));
> > > +	printk("mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n",
> > > +	       global_node_page_state(NR_FILE_MAPPED),
> > > +	       global_node_page_state(NR_SHMEM),
> > > +	       global_page_state(NR_PAGETABLE),
> > > +	       global_page_state(NR_BOUNCE));
> > > +	printk("free:%lu free_pcp:%lu free_cma:%lu\n",
> > > +	       global_page_state(NR_FREE_PAGES),
> > > +	       free_pcp,
> > > +	       global_page_state(NR_FREE_CMA_PAGES));
> > 
> > a side note:
> > 
> > this can make it harder to read, in _the worst case_. one printk()
> > guaranteed that we would see a single line in the serial log/etc.
> > the sort of a problem with multiple printks is that printks coming
> > from other CPUs will split that "previously single" line.
> 
> Not true.  Note the multiple \n uses in the original code.

one printk call ends up in logbuf as a single entry and, thus, we print
it to the serial console in one shot (what is the correct english word
to use here?). multiple printks result in multiple logbuf entries, and
printks from other CPUs can mix in.

so the difference is:


	CPU0						CPU1
							printk(foo\n)
printk(..isolated_anon\n...isolated_file\n...)
							printk(bar\n)

vs

	CPU0						CPU1
printk(..isolated_anon\n)
							printk(foo\n)
printk(...isolated_file\n)
							printk(bar\n)
printk(...\n)

not the same thing.

and the slower the serial console is the more messages potentially
can appear between "..isolated_anon\n" and "...isolated_file\n".

	-ss

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-16 20:32       ` Joe Perches
@ 2017-03-17  7:39         ` Michal Hocko
  -1 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-17  7:39 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Thu 16-03-17 13:32:23, Joe Perches wrote:
> On Thu, 2017-03-16 at 11:56 +0100, Michal Hocko wrote:
> > On Wed 15-03-17 18:43:13, Joe Perches wrote:
> > > Function calls with large argument counts cause x86-64 register
> > > spilling.  Reducing the number of arguments in a multi-line printk
> > > by converting to multiple printks which saves some object code size.
> > > 
> > > $ size mm/page_alloc.o* (defconfig)
> > >    text    data     bss     dec     hex filename
> > >   35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
> > >   36018    1699     628   38345    95c9 mm/page_alloc.o.old
> > > 
> > > Miscellanea:
> > > 
> > > o Remove line leading spaces from the formerly multi-line printks
> > >   commit a25700a53f71 ("mm: show bounce pages in oom killer output")
> > >   back in 2007 started the leading space when a single long line
> > >   was split into multiple lines but the leading space was likely
> > >   mistakenly kept and subsequent commits followed suit.
> > > o Align arguments in a few more printks
> > 
> > This is really hard to review. Could you just drop all the whitespace
> > changes please?
> 
> It's a single, simple change. 

no it adds a lot of whitespace noise to an actual change. It takes to
check every single line to see whether some typo or unintended change
has been made.

> It's IMO trivial to review.

it's not IMNSHO.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-17  7:39         ` Michal Hocko
  0 siblings, 0 replies; 30+ messages in thread
From: Michal Hocko @ 2017-03-17  7:39 UTC (permalink / raw)
  To: Joe Perches; +Cc: Andrew Morton, linux-kernel, linux-mm

On Thu 16-03-17 13:32:23, Joe Perches wrote:
> On Thu, 2017-03-16 at 11:56 +0100, Michal Hocko wrote:
> > On Wed 15-03-17 18:43:13, Joe Perches wrote:
> > > Function calls with large argument counts cause x86-64 register
> > > spilling.  Reducing the number of arguments in a multi-line printk
> > > by converting to multiple printks which saves some object code size.
> > > 
> > > $ size mm/page_alloc.o* (defconfig)
> > >    text    data     bss     dec     hex filename
> > >   35914	   1699	    628	  38241	   9561	mm/page_alloc.o.new
> > >   36018    1699     628   38345    95c9 mm/page_alloc.o.old
> > > 
> > > Miscellanea:
> > > 
> > > o Remove line leading spaces from the formerly multi-line printks
> > >   commit a25700a53f71 ("mm: show bounce pages in oom killer output")
> > >   back in 2007 started the leading space when a single long line
> > >   was split into multiple lines but the leading space was likely
> > >   mistakenly kept and subsequent commits followed suit.
> > > o Align arguments in a few more printks
> > 
> > This is really hard to review. Could you just drop all the whitespace
> > changes please?
> 
> It's a single, simple change. 

no it adds a lot of whitespace noise to an actual change. It takes to
check every single line to see whether some typo or unintended change
has been made.

> It's IMO trivial to review.

it's not IMNSHO.
-- 
Michal Hocko
SUSE Labs

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-17  1:56         ` Sergey Senozhatsky
@ 2017-03-18 19:31           ` Joe Perches
  -1 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-18 19:31 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, linux-kernel, linux-mm, Petr Mladek, Steven Rostedt

(adding Petr and Steven to cc's)

On Fri, 2017-03-17 at 10:56 +0900, Sergey Senozhatsky wrote:
> On (03/16/17 11:37), Joe Perches wrote:
> > On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> > > On (03/15/17 18:43), Joe Perches wrote:
> > > [..]
> > > > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > > > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > > > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > > > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > > > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > > > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > > > -		global_node_page_state(NR_ACTIVE_ANON),
> > > > -		global_node_page_state(NR_INACTIVE_ANON),
> > > > -		global_node_page_state(NR_ISOLATED_ANON),
> > > > -		global_node_page_state(NR_ACTIVE_FILE),
> > > > -		global_node_page_state(NR_INACTIVE_FILE),
> > > > -		global_node_page_state(NR_ISOLATED_FILE),
> > > > -		global_node_page_state(NR_UNEVICTABLE),
> > > > -		global_node_page_state(NR_FILE_DIRTY),
> > > > -		global_node_page_state(NR_WRITEBACK),
> > > > -		global_node_page_state(NR_UNSTABLE_NFS),
> > > > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > > > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > > > -		global_node_page_state(NR_FILE_MAPPED),
> > > > -		global_node_page_state(NR_SHMEM),
> > > > -		global_page_state(NR_PAGETABLE),
> > > > -		global_page_state(NR_BOUNCE),
> > > > -		global_page_state(NR_FREE_PAGES),
> > > > -		free_pcp,
> > > > -		global_page_state(NR_FREE_CMA_PAGES));
[]
> > > > a side note:
> > > 
> > > this can make it harder to read, in _the worst case_. one printk()
> > > guaranteed that we would see a single line in the serial log/etc.
> > > the sort of a problem with multiple printks is that printks coming
> > > from other CPUs will split that "previously single" line.
> > 
> > Not true.  Note the multiple \n uses in the original code.
> 
> one printk call ends up in logbuf as a single entry and, thus, we print
> it to the serial console in one shot (what is the correct english word
> to use here?). multiple printks result in multiple logbuf entries, and
> printks from other CPUs can mix in.
> 
> so the difference is:
> 
> 
> 	CPU0						CPU1
> 							printk(foo\n)
> printk(..isolated_anon\n...isolated_file\n...)
> 							printk(bar\n)
> 
> vs
> 
> 	CPU0						CPU1
> printk(..isolated_anon\n)
> 							printk(foo\n)
> printk(...isolated_file\n)
> 							printk(bar\n)
> printk(...\n)
> 
> not the same thing.
> 
> and the slower the serial console is the more messages potentially
> can appear between "..isolated_anon\n" and "...isolated_file\n".

Right.  For the definition of "single line", meaning "contiguous
block" and not single line.

Perhaps there would be some value in having a generic mechanism
for the dump_stack use of "atomic_t dump_lock", where a thread
can grab exclusive use of the printk subsystem for a short period
to keep messages from being interleaved by other processes.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-18 19:31           ` Joe Perches
  0 siblings, 0 replies; 30+ messages in thread
From: Joe Perches @ 2017-03-18 19:31 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, linux-kernel, linux-mm, Petr Mladek, Steven Rostedt

(adding Petr and Steven to cc's)

On Fri, 2017-03-17 at 10:56 +0900, Sergey Senozhatsky wrote:
> On (03/16/17 11:37), Joe Perches wrote:
> > On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> > > On (03/15/17 18:43), Joe Perches wrote:
> > > [..]
> > > > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > > > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > > > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > > > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > > > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > > > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > > > -		global_node_page_state(NR_ACTIVE_ANON),
> > > > -		global_node_page_state(NR_INACTIVE_ANON),
> > > > -		global_node_page_state(NR_ISOLATED_ANON),
> > > > -		global_node_page_state(NR_ACTIVE_FILE),
> > > > -		global_node_page_state(NR_INACTIVE_FILE),
> > > > -		global_node_page_state(NR_ISOLATED_FILE),
> > > > -		global_node_page_state(NR_UNEVICTABLE),
> > > > -		global_node_page_state(NR_FILE_DIRTY),
> > > > -		global_node_page_state(NR_WRITEBACK),
> > > > -		global_node_page_state(NR_UNSTABLE_NFS),
> > > > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > > > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > > > -		global_node_page_state(NR_FILE_MAPPED),
> > > > -		global_node_page_state(NR_SHMEM),
> > > > -		global_page_state(NR_PAGETABLE),
> > > > -		global_page_state(NR_BOUNCE),
> > > > -		global_page_state(NR_FREE_PAGES),
> > > > -		free_pcp,
> > > > -		global_page_state(NR_FREE_CMA_PAGES));
[]
> > > > a side note:
> > > 
> > > this can make it harder to read, in _the worst case_. one printk()
> > > guaranteed that we would see a single line in the serial log/etc.
> > > the sort of a problem with multiple printks is that printks coming
> > > from other CPUs will split that "previously single" line.
> > 
> > Not true.  Note the multiple \n uses in the original code.
> 
> one printk call ends up in logbuf as a single entry and, thus, we print
> it to the serial console in one shot (what is the correct english word
> to use here?). multiple printks result in multiple logbuf entries, and
> printks from other CPUs can mix in.
> 
> so the difference is:
> 
> 
> 	CPU0						CPU1
> 							printk(foo\n)
> printk(..isolated_anon\n...isolated_file\n...)
> 							printk(bar\n)
> 
> vs
> 
> 	CPU0						CPU1
> printk(..isolated_anon\n)
> 							printk(foo\n)
> printk(...isolated_file\n)
> 							printk(bar\n)
> printk(...\n)
> 
> not the same thing.
> 
> and the slower the serial console is the more messages potentially
> can appear between "..isolated_anon\n" and "...isolated_file\n".

Right.  For the definition of "single line", meaning "contiguous
block" and not single line.

Perhaps there would be some value in having a generic mechanism
for the dump_stack use of "atomic_t dump_lock", where a thread
can grab exclusive use of the printk subsystem for a short period
to keep messages from being interleaved by other processes.

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
  2017-03-18 19:31           ` Joe Perches
@ 2017-03-20 13:00             ` Petr Mladek
  -1 siblings, 0 replies; 30+ messages in thread
From: Petr Mladek @ 2017-03-20 13:00 UTC (permalink / raw)
  To: Joe Perches
  Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm,
	Steven Rostedt

On Sat 2017-03-18 12:31:35, Joe Perches wrote:
> (adding Petr and Steven to cc's)
> 
> On Fri, 2017-03-17 at 10:56 +0900, Sergey Senozhatsky wrote:
> > On (03/16/17 11:37), Joe Perches wrote:
> > > On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> > > > On (03/15/17 18:43), Joe Perches wrote:
> > > > [..]
> > > > > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > > > > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > > > > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > > > > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > > > > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > > > > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > > > > -		global_node_page_state(NR_ACTIVE_ANON),
> > > > > -		global_node_page_state(NR_INACTIVE_ANON),
> > > > > -		global_node_page_state(NR_ISOLATED_ANON),
> > > > > -		global_node_page_state(NR_ACTIVE_FILE),
> > > > > -		global_node_page_state(NR_INACTIVE_FILE),
> > > > > -		global_node_page_state(NR_ISOLATED_FILE),
> > > > > -		global_node_page_state(NR_UNEVICTABLE),
> > > > > -		global_node_page_state(NR_FILE_DIRTY),
> > > > > -		global_node_page_state(NR_WRITEBACK),
> > > > > -		global_node_page_state(NR_UNSTABLE_NFS),
> > > > > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > > > > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > > > > -		global_node_page_state(NR_FILE_MAPPED),
> > > > > -		global_node_page_state(NR_SHMEM),
> > > > > -		global_page_state(NR_PAGETABLE),
> > > > > -		global_page_state(NR_BOUNCE),
> > > > > -		global_page_state(NR_FREE_PAGES),
> > > > > -		free_pcp,
> > > > > -		global_page_state(NR_FREE_CMA_PAGES));
> []
> > > > > a side note:
> > > > 
> > > > this can make it harder to read, in _the worst case_. one printk()
> > > > guaranteed that we would see a single line in the serial log/etc.
> > > > the sort of a problem with multiple printks is that printks coming
> > > > from other CPUs will split that "previously single" line.
> > > 
> > > Not true.  Note the multiple \n uses in the original code.
> > 
> > one printk call ends up in logbuf as a single entry and, thus, we print
> > it to the serial console in one shot (what is the correct english word
> > to use here?). multiple printks result in multiple logbuf entries, and
> > printks from other CPUs can mix in.
> > 
> > so the difference is:
> > 
> > 
> > 	CPU0						CPU1
> > 							printk(foo\n)
> > printk(..isolated_anon\n...isolated_file\n...)
> > 							printk(bar\n)
> > 
> > vs
> > 
> > 	CPU0						CPU1
> > printk(..isolated_anon\n)
> > 							printk(foo\n)
> > printk(...isolated_file\n)
> > 							printk(bar\n)
> > printk(...\n)
> > 
> > not the same thing.
> > 
> > and the slower the serial console is the more messages potentially
> > can appear between "..isolated_anon\n" and "...isolated_file\n".
> 
> Right.  For the definition of "single line", meaning "contiguous
> block" and not single line.
> 
> Perhaps there would be some value in having a generic mechanism
> for the dump_stack use of "atomic_t dump_lock", where a thread
> can grab exclusive use of the printk subsystem for a short period
> to keep messages from being interleaved by other processes.

This sounds a bit scary to me. A globally blocking chain of
printk() calls might open another can of deadlocks. Also, IMHO,
dumping stack is a non-trivial operation, especially when
we need to read debuginfo.

Another solution would be to somehow reuse the per-CPU buffers
used by vprintk_safe(). An API for buffering printk messages
would be useful also for continuous lines. But this need to
be well designed.

Anyway, this should probably be discussed separately. We are too
far from the original problem. The fact is that printk() does
not prevent interleaving lines from different CPUs and probably
won't be in a near future. I am not sure in which situations
the affected messages are printed and if such an interleaving
is probable or not.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks
@ 2017-03-20 13:00             ` Petr Mladek
  0 siblings, 0 replies; 30+ messages in thread
From: Petr Mladek @ 2017-03-20 13:00 UTC (permalink / raw)
  To: Joe Perches
  Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm,
	Steven Rostedt

On Sat 2017-03-18 12:31:35, Joe Perches wrote:
> (adding Petr and Steven to cc's)
> 
> On Fri, 2017-03-17 at 10:56 +0900, Sergey Senozhatsky wrote:
> > On (03/16/17 11:37), Joe Perches wrote:
> > > On Thu, 2017-03-16 at 20:30 +0900, Sergey Senozhatsky wrote:
> > > > On (03/15/17 18:43), Joe Perches wrote:
> > > > [..]
> > > > > -	printk("active_anon:%lu inactive_anon:%lu isolated_anon:%lu\n"
> > > > > -		" active_file:%lu inactive_file:%lu isolated_file:%lu\n"
> > > > > -		" unevictable:%lu dirty:%lu writeback:%lu unstable:%lu\n"
> > > > > -		" slab_reclaimable:%lu slab_unreclaimable:%lu\n"
> > > > > -		" mapped:%lu shmem:%lu pagetables:%lu bounce:%lu\n"
> > > > > -		" free:%lu free_pcp:%lu free_cma:%lu\n",
> > > > > -		global_node_page_state(NR_ACTIVE_ANON),
> > > > > -		global_node_page_state(NR_INACTIVE_ANON),
> > > > > -		global_node_page_state(NR_ISOLATED_ANON),
> > > > > -		global_node_page_state(NR_ACTIVE_FILE),
> > > > > -		global_node_page_state(NR_INACTIVE_FILE),
> > > > > -		global_node_page_state(NR_ISOLATED_FILE),
> > > > > -		global_node_page_state(NR_UNEVICTABLE),
> > > > > -		global_node_page_state(NR_FILE_DIRTY),
> > > > > -		global_node_page_state(NR_WRITEBACK),
> > > > > -		global_node_page_state(NR_UNSTABLE_NFS),
> > > > > -		global_page_state(NR_SLAB_RECLAIMABLE),
> > > > > -		global_page_state(NR_SLAB_UNRECLAIMABLE),
> > > > > -		global_node_page_state(NR_FILE_MAPPED),
> > > > > -		global_node_page_state(NR_SHMEM),
> > > > > -		global_page_state(NR_PAGETABLE),
> > > > > -		global_page_state(NR_BOUNCE),
> > > > > -		global_page_state(NR_FREE_PAGES),
> > > > > -		free_pcp,
> > > > > -		global_page_state(NR_FREE_CMA_PAGES));
> []
> > > > > a side note:
> > > > 
> > > > this can make it harder to read, in _the worst case_. one printk()
> > > > guaranteed that we would see a single line in the serial log/etc.
> > > > the sort of a problem with multiple printks is that printks coming
> > > > from other CPUs will split that "previously single" line.
> > > 
> > > Not true.  Note the multiple \n uses in the original code.
> > 
> > one printk call ends up in logbuf as a single entry and, thus, we print
> > it to the serial console in one shot (what is the correct english word
> > to use here?). multiple printks result in multiple logbuf entries, and
> > printks from other CPUs can mix in.
> > 
> > so the difference is:
> > 
> > 
> > 	CPU0						CPU1
> > 							printk(foo\n)
> > printk(..isolated_anon\n...isolated_file\n...)
> > 							printk(bar\n)
> > 
> > vs
> > 
> > 	CPU0						CPU1
> > printk(..isolated_anon\n)
> > 							printk(foo\n)
> > printk(...isolated_file\n)
> > 							printk(bar\n)
> > printk(...\n)
> > 
> > not the same thing.
> > 
> > and the slower the serial console is the more messages potentially
> > can appear between "..isolated_anon\n" and "...isolated_file\n".
> 
> Right.  For the definition of "single line", meaning "contiguous
> block" and not single line.
> 
> Perhaps there would be some value in having a generic mechanism
> for the dump_stack use of "atomic_t dump_lock", where a thread
> can grab exclusive use of the printk subsystem for a short period
> to keep messages from being interleaved by other processes.

This sounds a bit scary to me. A globally blocking chain of
printk() calls might open another can of deadlocks. Also, IMHO,
dumping stack is a non-trivial operation, especially when
we need to read debuginfo.

Another solution would be to somehow reuse the per-CPU buffers
used by vprintk_safe(). An API for buffering printk messages
would be useful also for continuous lines. But this need to
be well designed.

Anyway, this should probably be discussed separately. We are too
far from the original problem. The fact is that printk() does
not prevent interleaving lines from different CPUs and probably
won't be in a near future. I am not sure in which situations
the affected messages are printed and if such an interleaving
is probable or not.

Best Regards,
Petr

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2017-03-20 13:25 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-16  1:43 [PATCH 0/3] mm: page_alloc: Object code reductions and logging fix Joe Perches
2017-03-16  1:43 ` Joe Perches
2017-03-16  1:43 ` [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Joe Perches
2017-03-16  1:43   ` Joe Perches
2017-03-16 10:56   ` Michal Hocko
2017-03-16 10:56     ` Michal Hocko
2017-03-16 20:32     ` Joe Perches
2017-03-16 20:32       ` Joe Perches
2017-03-17  7:39       ` Michal Hocko
2017-03-17  7:39         ` Michal Hocko
2017-03-16 11:30   ` Sergey Senozhatsky
2017-03-16 11:30     ` Sergey Senozhatsky
2017-03-16 18:37     ` Joe Perches
2017-03-16 18:37       ` Joe Perches
2017-03-16 22:53       ` Andrew Morton
2017-03-16 22:53         ` Andrew Morton
2017-03-17  1:56       ` Sergey Senozhatsky
2017-03-17  1:56         ` Sergey Senozhatsky
2017-03-18 19:31         ` Joe Perches
2017-03-18 19:31           ` Joe Perches
2017-03-20 13:00           ` Petr Mladek
2017-03-20 13:00             ` Petr Mladek
2017-03-16  1:43 ` [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size Joe Perches
2017-03-16  1:43   ` Joe Perches
2017-03-16 10:57   ` Michal Hocko
2017-03-16 10:57     ` Michal Hocko
2017-03-16  1:43 ` [PATCH 3/3] mm: page_alloc: Break up a long single-line printk Joe Perches
2017-03-16  1:43   ` Joe Perches
2017-03-16 10:58   ` Michal Hocko
2017-03-16 10:58     ` Michal Hocko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.