Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1601837 > unrolled thread
| Started by | Joe Perches <joe@perches.com> |
|---|---|
| First post | 2017-03-16 02:50 +0100 |
| Last post | 2017-03-20 14:30 +0100 |
| Articles | 13 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] mm: page_alloc: Object code reductions and logging fix Joe Perches <joe@perches.com> - 2017-03-16 02:50 +0100
[PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size Joe Perches <joe@perches.com> - 2017-03-16 02:50 +0100
Re: [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size Michal Hocko <mhocko@kernel.org> - 2017-03-16 12:00 +0100
[PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Joe Perches <joe@perches.com> - 2017-03-16 02:50 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Michal Hocko <mhocko@kernel.org> - 2017-03-16 12:00 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Joe Perches <joe@perches.com> - 2017-03-16 21:40 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Michal Hocko <mhocko@kernel.org> - 2017-03-17 08:40 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-16 12:40 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Joe Perches <joe@perches.com> - 2017-03-16 19:40 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Andrew Morton <akpm@linux-foundation.org> - 2017-03-17 00:00 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-03-17 03:10 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Joe Perches <joe@perches.com> - 2017-03-18 20:40 +0100
Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks Petr Mladek <pmladek@suse.com> - 2017-03-20 14:30 +0100
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-03-16 02:50 +0100 |
| Subject | [PATCH 0/3] mm: page_alloc: Object code reductions and logging fix |
| Message-ID | <tlsI9-7Ou-3@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-03-16 02:50 +0100 |
| Subject | [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size |
| Message-ID | <tlsI9-7Ou-13@gated-at.bofh.it> |
| In reply to | #1601837 |
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
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-03-16 12:00 +0100 |
| Subject | Re: [PATCH 2/3] mm: page_alloc: Fix misordered logging output, reduce code size |
| Message-ID | <tlBiq-5vO-17@gated-at.bofh.it> |
| In reply to | #1601840 |
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
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-03-16 02:50 +0100 |
| Subject | [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlsI9-7Ou-9@gated-at.bofh.it> |
| In reply to | #1601837 |
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
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-03-16 12:00 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlBiq-5vO-19@gated-at.bofh.it> |
| In reply to | #1601841 |
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
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-03-16 21:40 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlKlI-3E4-3@gated-at.bofh.it> |
| In reply to | #1602176 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-03-17 08:40 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlUEq-2QJ-11@gated-at.bofh.it> |
| In reply to | #1602827 |
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
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2017-03-16 12:40 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlBV7-62w-1@gated-at.bofh.it> |
| In reply to | #1601841 |
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
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-03-16 19:40 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlItA-2iT-21@gated-at.bofh.it> |
| In reply to | #1602207 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| Date | 2017-03-17 00:00 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlMxb-56x-1@gated-at.bofh.it> |
| In reply to | #1602761 |
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?
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2017-03-17 03:10 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tlPv3-7nQ-5@gated-at.bofh.it> |
| In reply to | #1602761 |
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
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-03-18 20:40 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tmsmJ-2D3-3@gated-at.bofh.it> |
| In reply to | #1602990 |
(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.
[toc] | [prev] | [next] | [standalone]
| From | Petr Mladek <pmladek@suse.com> |
|---|---|
| Date | 2017-03-20 14:30 +0100 |
| Subject | Re: [PATCH 1/3] mm: page_alloc: Reduce object size by neatening printks |
| Message-ID | <tn5xM-5ik-29@gated-at.bofh.it> |
| In reply to | #1603847 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web