mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Tejun Heo <tj@kernel.org>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH 10/12] sched: use {cpu|node}mask pr_cont and seq output functions
Date: Mon, 15 Dec 2014 11:39:15 +0100	[thread overview]
Message-ID: <20141215103915.GX29390@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1418226774-30215-11-git-send-email-tj@kernel.org>

On Wed, Dec 10, 2014 at 10:52:52AM -0500, Tejun Heo wrote:
> Convert the existing user of cpu{mask|list}_scnprintf() and
> node{mask|list}_scnprintf() which use them just to printk or
> seq_printf() the resulting buffer to use the following functions
> instead respectively.
> 
> * For printk: cpu{mask|list}_pr_cont() and node{mask|list}_pr_cont().
> 
> * For seq_file: seq_cpumask[_list]() and seq_nodemask[_list]().
> 
> Because these conversions usually break up a single output function
> call into multiple, the reduction is LOC isn't too big but it removes
> unnecessary complexity and/or arbitrary limit on the length printed.
> 
> This patch is dependent on the previous patch ("bitmap, cpumask,
> nodemask: implement pr_cont variants of formatting functions") which
> is planned to go through -mm.  It'd be the easiest to route this
> together.  If this should go through the subsystem tree, please wait
> till the forementioned patch is merged to mainline.
> 

I would very much prefer something like the below instead..

I'm not a great fan of pr_cont, it makes a mess of things if there's
multiple cpus printing bits.

---
Subject: lib/vsprintf: Provide %pc to print cpumasks
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Fri May 11 00:35:15 CEST 2012


Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/mm/numa.c  |    6 ++----
 kernel/sched/core.c |    8 ++------
 lib/vsprintf.c      |    5 +++++
 3 files changed, 9 insertions(+), 10 deletions(-)

--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -737,7 +737,6 @@ int early_cpu_to_node(int cpu)
 void debug_cpumask_set_cpu(int cpu, int node, bool enable)
 {
 	struct cpumask *mask;
-	char buf[64];
 
 	if (node == NUMA_NO_NODE) {
 		/* early_cpu_to_node() already emits a warning and trace */
@@ -755,10 +754,9 @@ void debug_cpumask_set_cpu(int cpu, int
 	else
 		cpumask_clear_cpu(cpu, mask);
 
-	cpulist_scnprintf(buf, sizeof(buf), mask);
-	printk(KERN_DEBUG "%s cpu %d node %d: mask now %s\n",
+	printk(KERN_DEBUG "%s cpu %d node %d: mask now %pc\n",
 		enable ? "numa_add_cpu" : "numa_remove_cpu",
-		cpu, node, buf);
+		cpu, node, mask);
 	return;
 }
 
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4944,9 +4944,7 @@ static int sched_domain_debug_one(struct
 				  struct cpumask *groupmask)
 {
 	struct sched_group *group = sd->groups;
-	char str[256];
 
-	cpulist_scnprintf(str, sizeof(str), sched_domain_span(sd));
 	cpumask_clear(groupmask);
 
 	printk(KERN_DEBUG "%*s domain %d: ", level, "", level);
@@ -4959,7 +4957,7 @@ static int sched_domain_debug_one(struct
 		return -1;
 	}
 
-	printk(KERN_CONT "span %s level %s\n", str, sd->name);
+	printk(KERN_CONT "span %pc level %s\n", sched_domain_span(sd), sd->name);
 
 	if (!cpumask_test_cpu(cpu, sched_domain_span(sd))) {
 		printk(KERN_ERR "ERROR: domain->span does not contain "
@@ -5005,9 +5003,7 @@ static int sched_domain_debug_one(struct
 
 		cpumask_or(groupmask, groupmask, sched_group_cpus(group));
 
-		cpulist_scnprintf(str, sizeof(str), sched_group_cpus(group));
-
-		printk(KERN_CONT " %s", str);
+		printk(KERN_CONT " %pc", sched_group_cpus(group));
 		if (group->sgp->power != SCHED_POWER_SCALE) {
 			printk(KERN_CONT " (cpu_power = %d)",
 				group->sgp->power);
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -27,6 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/ioport.h>
 #include <linux/dcache.h>
+#include <linux/cpumask.h>
 #include <net/addrconf.h>
 
 #include <asm/page.h>		/* for PAGE_SIZE */
@@ -1218,6 +1219,7 @@ int kptr_restrict __read_mostly;
  *            The maximum supported length is 64 bytes of the input. Consider
  *            to use print_hex_dump() for the larger input.
  * - 'a' For a phys_addr_t type and its derivative types (passed by reference)
+ * - 'c' For a cpumask list
  *
  * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
  * function pointers are really function descriptors, which contain a
@@ -1335,6 +1337,8 @@ char *pointer(const char *fmt, char *buf
 		return dentry_name(buf, end,
 				   ((const struct file *)ptr)->f_path.dentry,
 				   spec, fmt);
+	case 'c':
+		return buf + cpulist_scnprintf(buf, end - buf, ptr);
 	}
 	spec.flags |= SMALL;
 	if (spec.field_width == -1) {
@@ -1576,6 +1580,7 @@ int format_decode(const char *fmt, struc
  *   case.
  * %*ph[CDN] a variable-length hex string with a separator (supports up to 64
  *           bytes of the input)
+ * %pc print a cpumask as comma-separated list
  * %n is ignored
  *
  * ** Please update Documentation/printk-formats.txt when making changes **

  reply	other threads:[~2014-12-15 10:39 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-10 15:52 [PATCHSET -mm] bitmap, cpumask, nodemask: implement pr_cont functions and use di" Tejun Heo
2014-12-10 15:52 ` [PATCH 01/12] bitmap: restructure bitmap_sn[list]printf() Tejun Heo
2014-12-10 15:52 ` [PATCH 02/12] bitmap, cpumask, nodemask: implement pr_cont variants of formatting functions Tejun Heo
2014-12-10 15:52 ` [PATCH 03/12] mips: use {cpu|node}mask pr_cont and seq output functions Tejun Heo
2014-12-10 15:52 ` [PATCH 04/12] powerpc: " Tejun Heo
2014-12-10 15:52 ` [PATCH 05/12] tile: " Tejun Heo
2014-12-15 16:20   ` Chris Metcalf
2014-12-10 15:52 ` [PATCH 06/12] x86: " Tejun Heo
2014-12-10 15:52 ` [PATCH 07/12] xtensa: " Tejun Heo
2014-12-10 18:38   ` Max Filippov
2014-12-10 15:52 ` [PATCH 08/12] cpuset: " Tejun Heo
2014-12-10 15:52 ` [PATCH 09/12] rcu: " Tejun Heo
2014-12-10 16:29   ` Paul E. McKenney
2014-12-10 15:52 ` [PATCH 10/12] sched: " Tejun Heo
2014-12-15 10:39   ` Peter Zijlstra [this message]
2015-01-12 13:31     ` Tejun Heo
2015-01-12 13:50       ` Peter Zijlstra
2015-01-12 13:55         ` Tejun Heo
2015-01-12 14:00           ` Tejun Heo
2014-12-10 15:52 ` [PATCH 11/12] timer: " Tejun Heo
2014-12-10 15:52 ` [PATCH 12/12] percpu: " Tejun Heo
2014-12-10 21:49 ` [PATCHSET -mm] bitmap, cpumask, nodemask: implement pr_cont functions and use di" Andrew Morton
2014-12-10 22:12   ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141215103915.GX29390@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome