[PATCH 0/3] x86: add cpus_scnprintf function v3

[PATCH 0/3] x86: add cpus_scnprintf function v3

[Mike Travis]

  * Add cpu_sysdev_class functions to display the

	cpu_online_map
	cpu_present_map
	cpu_possible_map
	cpu_system_map

  * Cleanup usages of cpumask_scprintf in the following files and add
    another interface to use cpulist_scnprintf where appropriate.

	arch/x86/kernel/cpu/intel_cacheinfo.c
	drivers/base/cpu.c
	drivers/base/node.c
	drivers/base/topology.c
	drivers/pci/pci-sysfs.c
	drivers/pci/probe.c
	include/linux/sysdev.h
	kernel/cpuset.c
	kernel/sched_stats.h
	kernel/trace/trace.c

  - The new interface is generally the same base name but uses 'list'
    instead of 'map'.  For example, there is now:

	# cat /sys/devices/system/node/node0/cpumap
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,\
	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000003

  - And the new interface:

	# cat /sys/devices/system/node/node0/cpulist
	0-1

Based on:
	git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
    +   x86/latest          .../x86/linux-2.6-x86.git
    +   sched-devel/latest  .../mingo/linux-2.6-sched-devel.git

Signed-off-by: Mike Travis <travis@sgi.com>
---
v3:
    Removed cpus_scnprintf() and all references.

v2:
    Renamed cpuset_scnprintf() to cpus_scnprintf to avoid confusion with
    "cpusets", and changed the other names to match.

    Added a sentinel character for the cpulist type output so scripts can
    be written to handle both cases.

    Added warning in the Documentation/sysctl/kernel.txt that changing this
    option may break scripts and programs that rely on the current format.
    Also provided example on how to process the output.

    Added information to Documentation/scheduler/sched-stats.txt about
    change in version number because of new cpus_scnprintf function.

-- 

[PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Mike Travis]

[-- Attachment #1: mod-intel_cacheinfo --]
[-- Type: text/plain, Size: 2466 bytes --]

  * Removed kmalloc (or local array) in show_shared_cpu_map().

  * Added show_shared_cpu_list() function.

Based on:
	git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
    +   x86/latest          .../x86/linux-2.6-x86.git
    +   sched-devel/latest  .../mingo/linux-2.6-sched-devel.git

Signed-off-by: Mike Travis <travis@sgi.com>
---
v3:
    Removed cpus_scnprintf() and all references.
v2:
    Renamed cpuset_scnprintf() to cpus_scnprintf to avoid confusion with
    "cpusets", and changed the other names to match.
---
 arch/x86/kernel/cpu/intel_cacheinfo.c |   30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

--- linux-2.6.x86.sched.orig/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ linux-2.6.x86.sched/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -591,20 +591,34 @@ static ssize_t show_size(struct _cpuid4_
 	return sprintf (buf, "%luK\n", this_leaf->size / 1024);
 }
 
-static ssize_t show_shared_cpu_map(struct _cpuid4_info *this_leaf, char *buf)
+static ssize_t show_shared_cpu_map_func(struct _cpuid4_info *this_leaf,
+					int type, char *buf)
 {
+	ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
 	int n = 0;
-	int len = cpumask_scnprintf_len(nr_cpu_ids);
-	char *mask_str = kmalloc(len, GFP_KERNEL);
 
-	if (mask_str) {
-		cpumask_scnprintf(mask_str, len, this_leaf->shared_cpu_map);
-		n = sprintf(buf, "%s\n", mask_str);
-		kfree(mask_str);
+	if (len > 1) {
+		cpumask_t *mask = &this_leaf->shared_cpu_map;
+
+		n = type?
+			cpulist_scnprintf(buf, len-2, *mask):
+			cpumask_scnprintf(buf, len-2, *mask);
+		buf[n++] = '\n';
+		buf[n] = '\0';
 	}
 	return n;
 }
 
+static inline ssize_t show_shared_cpu_map(struct _cpuid4_info *leaf, char *buf)
+{
+	return show_shared_cpu_map_func(leaf, 0, buf);
+}
+
+static inline ssize_t show_shared_cpu_list(struct _cpuid4_info *leaf, char *buf)
+{
+	return show_shared_cpu_map_func(leaf, 1, buf);
+}
+
 static ssize_t show_type(struct _cpuid4_info *this_leaf, char *buf) {
 	switch(this_leaf->eax.split.type) {
 	    case CACHE_TYPE_DATA:
@@ -640,6 +654,7 @@ define_one_ro(ways_of_associativity);
 define_one_ro(number_of_sets);
 define_one_ro(size);
 define_one_ro(shared_cpu_map);
+define_one_ro(shared_cpu_list);
 
 static struct attribute * default_attrs[] = {
 	&type.attr,
@@ -650,6 +665,7 @@ static struct attribute * default_attrs[
 	&number_of_sets.attr,
 	&size.attr,
 	&shared_cpu_map.attr,
+	&shared_cpu_list.attr,
 	NULL
 };
 

-- 

[PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Mike Travis]

[-- Attachment #1: cleanup-cpumask_scnprintf --]
[-- Type: text/plain, Size: 9998 bytes --]

  * Cleaned up references to cpumask_scnprintf() and added new
    cpulist_scnprintf() interfaces where appropriate.

  * Fix some small bugs (or code efficiency improvments) for various uses
    of cpumask_scnprintf.

  * Clean up some checkpatch errors.

Based on:
	git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
    +   x86/latest          .../x86/linux-2.6-x86.git
    +   sched-devel/latest  .../mingo/linux-2.6-sched-devel.git

Signed-off-by: Mike Travis <travis@sgi.com>
---
v3:
    Removed cpus_scnprintf() and all references.
    Removed changes to sched_stat.h (now cleanup only).

    Three unfixable checkpatch warnings/errors:

WARNING: usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc
#49: FILE: drivers/base/node.c:29:
+	BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));

ERROR: Macros with complex values should be enclosed in parenthesis
#115: FILE: drivers/base/topology.c:59:
+static inline ssize_t show_##name(struct sys_device *dev, char *buf)	\

ERROR: Macros with complex values should be enclosed in parenthesis
#125: FILE: drivers/base/topology.c:66:
+static inline ssize_t show_##name##_list(struct sys_device *dev, char *buf) \

v2:
    Renamed cpuset_scnprintf() to cpus_scnprintf to avoid confusion with
    "cpusets", and changed the other names to match.
---
 drivers/base/node.c     |   24 +++++++++++++++++++-----
 drivers/base/topology.c |   41 ++++++++++++++++++++++++++++++++++-------
 drivers/pci/pci-sysfs.c |   20 ++++++++++++++++++--
 drivers/pci/probe.c     |   27 +++++++++++++++++++++++----
 kernel/cpuset.c         |    8 ++++++++
 kernel/sched_stats.h    |    8 ++++++--
 6 files changed, 108 insertions(+), 20 deletions(-)

--- linux-2.6.x86.sched.orig/drivers/base/node.c
+++ linux-2.6.x86.sched/drivers/base/node.c
@@ -19,22 +19,34 @@ static struct sysdev_class node_class = 
 };
 
 
-static ssize_t node_read_cpumap(struct sys_device * dev, char * buf)
+static ssize_t node_read_cpumap(struct sys_device *dev, int type, char *buf)
 {
 	struct node *node_dev = to_node(dev);
 	node_to_cpumask_ptr(mask, node_dev->sysdev.id);
 	int len;
 
-	/* 2004/06/03: buf currently PAGE_SIZE, need > 1 char per 4 bits. */
-	BUILD_BUG_ON(MAX_NUMNODES/4 > PAGE_SIZE/2);
+	/* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
+	BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
 
-	len = cpumask_scnprintf(buf, PAGE_SIZE-2, *mask);
+	len = type?
+		cpulist_scnprintf(buf, PAGE_SIZE-2, *mask):
+		cpumask_scnprintf(buf, PAGE_SIZE-2, *mask);
 	buf[len++] = '\n';
 	buf[len] = '\0';
 	return len;
 }
 
-static SYSDEV_ATTR(cpumap, S_IRUGO, node_read_cpumap, NULL);
+static inline ssize_t node_read_cpumask(struct sys_device *dev, char *buf)
+{
+	return node_read_cpumap(dev, 0, buf);
+}
+static inline ssize_t node_read_cpulist(struct sys_device *dev, char *buf)
+{
+	return node_read_cpumap(dev, 1, buf);
+}
+
+static SYSDEV_ATTR(cpumap,  S_IRUGO, node_read_cpumask, NULL);
+static SYSDEV_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
 
 #define K(x) ((x) << (PAGE_SHIFT - 10))
 static ssize_t node_read_meminfo(struct sys_device * dev, char * buf)
@@ -150,6 +162,7 @@ int register_node(struct node *node, int
 
 	if (!error){
 		sysdev_create_file(&node->sysdev, &attr_cpumap);
+		sysdev_create_file(&node->sysdev, &attr_cpulist);
 		sysdev_create_file(&node->sysdev, &attr_meminfo);
 		sysdev_create_file(&node->sysdev, &attr_numastat);
 		sysdev_create_file(&node->sysdev, &attr_distance);
@@ -167,6 +180,7 @@ int register_node(struct node *node, int
 void unregister_node(struct node *node)
 {
 	sysdev_remove_file(&node->sysdev, &attr_cpumap);
+	sysdev_remove_file(&node->sysdev, &attr_cpulist);
 	sysdev_remove_file(&node->sysdev, &attr_meminfo);
 	sysdev_remove_file(&node->sysdev, &attr_numastat);
 	sysdev_remove_file(&node->sysdev, &attr_distance);
--- linux-2.6.x86.sched.orig/drivers/base/topology.c
+++ linux-2.6.x86.sched/drivers/base/topology.c
@@ -40,15 +40,38 @@ static ssize_t show_##name(struct sys_de
 	return sprintf(buf, "%d\n", topology_##name(cpu));	\
 }
 
-#define define_siblings_show_func(name)					\
-static ssize_t show_##name(struct sys_device *dev, char *buf)		\
+static ssize_t show_cpumap(int type, cpumask_t *mask, char *buf)
+{
+	ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
+	int n = 0;
+
+	if (len > 1) {
+		n = type?
+			cpulist_scnprintf(buf, len-2, *mask):
+			cpumask_scnprintf(buf, len-2, *mask);
+		buf[n++] = '\n';
+		buf[n] = '\0';
+	}
+	return n;
+}
+
+#define define_siblings_show_map(name)					\
+static inline ssize_t show_##name(struct sys_device *dev, char *buf)	\
 {									\
-	ssize_t len = -1;						\
 	unsigned int cpu = dev->id;					\
-	len = cpumask_scnprintf(buf, NR_CPUS+1, topology_##name(cpu));	\
-	return (len + sprintf(buf + len, "\n"));			\
+	return show_cpumap(0, &(topology_##name(cpu)), buf);		\
 }
 
+#define define_siblings_show_list(name)					\
+static inline ssize_t show_##name##_list(struct sys_device *dev, char *buf) \
+{									\
+	unsigned int cpu = dev->id;					\
+	return show_cpumap(1, &(topology_##name(cpu)), buf);		\
+}
+
+#define define_siblings_show_func(name)		\
+	define_siblings_show_map(name); define_siblings_show_list(name)
+
 #ifdef	topology_physical_package_id
 define_id_show_func(physical_package_id);
 define_one_ro(physical_package_id);
@@ -68,7 +91,9 @@ define_one_ro(core_id);
 #ifdef topology_thread_siblings
 define_siblings_show_func(thread_siblings);
 define_one_ro(thread_siblings);
-#define ref_thread_siblings_attr	&attr_thread_siblings.attr,
+define_one_ro(thread_siblings_list);
+#define ref_thread_siblings_attr	\
+		&attr_thread_siblings.attr, &attr_thread_siblings_list.attr,
 #else
 #define ref_thread_siblings_attr
 #endif
@@ -76,7 +101,9 @@ define_one_ro(thread_siblings);
 #ifdef topology_core_siblings
 define_siblings_show_func(core_siblings);
 define_one_ro(core_siblings);
-#define ref_core_siblings_attr		&attr_core_siblings.attr,
+define_one_ro(core_siblings_list);
+#define ref_core_siblings_attr		\
+		&attr_core_siblings.attr, &attr_core_siblings_list.attr,
 #else
 #define ref_core_siblings_attr
 #endif
--- linux-2.6.x86.sched.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6.x86.sched/drivers/pci/pci-sysfs.c
@@ -73,8 +73,23 @@ static ssize_t local_cpus_show(struct de
 
 	mask = pcibus_to_cpumask(to_pci_dev(dev)->bus);
 	len = cpumask_scnprintf(buf, PAGE_SIZE-2, mask);
-	strcat(buf,"\n"); 
-	return 1+len;
+	buf[len++] = '\n';
+	buf[len] = '\0';
+	return len;
+}
+
+
+static ssize_t local_cpulist_show(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
+	cpumask_t mask;
+	int len;
+
+	mask = pcibus_to_cpumask(to_pci_dev(dev)->bus);
+	len = cpulist_scnprintf(buf, PAGE_SIZE-2, mask);
+	buf[len++] = '\n';
+	buf[len] = '\0';
+	return len;
 }
 
 /* show resources */
@@ -201,6 +216,7 @@ struct device_attribute pci_dev_attrs[] 
 	__ATTR_RO(class),
 	__ATTR_RO(irq),
 	__ATTR_RO(local_cpus),
+	__ATTR_RO(local_cpulist),
 	__ATTR_RO(modalias),
 #ifdef CONFIG_NUMA
 	__ATTR_RO(numa_node),
--- linux-2.6.x86.sched.orig/drivers/pci/probe.c
+++ linux-2.6.x86.sched/drivers/pci/probe.c
@@ -82,6 +82,7 @@ void pci_remove_legacy_files(struct pci_
  * PCI Bus Class Devices
  */
 static ssize_t pci_bus_show_cpuaffinity(struct device *dev,
+					int type,
 					struct device_attribute *attr,
 					char *buf)
 {
@@ -89,12 +90,30 @@ static ssize_t pci_bus_show_cpuaffinity(
 	cpumask_t cpumask;
 
 	cpumask = pcibus_to_cpumask(to_pci_bus(dev));
-	ret = cpumask_scnprintf(buf, PAGE_SIZE, cpumask);
-	if (ret < PAGE_SIZE)
-		buf[ret++] = '\n';
+	ret = type?
+		cpulist_scnprintf(buf, PAGE_SIZE-2, cpumask):
+		cpumask_scnprintf(buf, PAGE_SIZE-2, cpumask);
+	buf[ret++] = '\n';
+	buf[ret] = '\0';
 	return ret;
 }
-DEVICE_ATTR(cpuaffinity, S_IRUGO, pci_bus_show_cpuaffinity, NULL);
+
+static ssize_t inline pci_bus_show_cpumaskaffinity(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return pci_bus_show_cpuaffinity(dev, 0, attr, buf);
+}
+
+static ssize_t inline pci_bus_show_cpulistaffinity(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return pci_bus_show_cpuaffinity(dev, 1, attr, buf);
+}
+
+DEVICE_ATTR(cpuaffinity,     S_IRUGO, pci_bus_show_cpumaskaffinity, NULL);
+DEVICE_ATTR(cpulistaffinity, S_IRUGO, pci_bus_show_cpulistaffinity, NULL);
 
 /*
  * PCI Bus Class
--- linux-2.6.x86.sched.orig/kernel/cpuset.c
+++ linux-2.6.x86.sched/kernel/cpuset.c
@@ -2369,8 +2369,16 @@ void cpuset_task_status_allowed(struct s
 	m->count += cpumask_scnprintf(m->buf + m->count, m->size - m->count,
 					task->cpus_allowed);
 	seq_printf(m, "\n");
+	seq_printf(m, "Cpus_allowed_list:\t");
+	m->count += cpulist_scnprintf(m->buf + m->count, m->size - m->count,
+					task->cpus_allowed);
+	seq_printf(m, "\n");
 	seq_printf(m, "Mems_allowed:\t");
 	m->count += nodemask_scnprintf(m->buf + m->count, m->size - m->count,
 					task->mems_allowed);
 	seq_printf(m, "\n");
+	seq_printf(m, "Mems_allowed_list:\t");
+	m->count += nodelist_scnprintf(m->buf + m->count, m->size - m->count,
+					task->mems_allowed);
+	seq_printf(m, "\n");
 }
--- linux-2.6.x86.sched.orig/kernel/sched_stats.h
+++ linux-2.6.x86.sched/kernel/sched_stats.h
@@ -9,6 +9,11 @@
 static int show_schedstat(struct seq_file *seq, void *v)
 {
 	int cpu;
+	int mask_len = NR_CPUS/32 * 9;
+	char *mask_str = kmalloc(mask_len, GFP_KERNEL);
+
+	if (mask_str == NULL)
+		return -ENOMEM;
 
 	seq_printf(seq, "version %d\n", SCHEDSTAT_VERSION);
 	seq_printf(seq, "timestamp %lu\n", jiffies);
@@ -36,9 +41,8 @@ static int show_schedstat(struct seq_fil
 		preempt_disable();
 		for_each_domain(cpu, sd) {
 			enum cpu_idle_type itype;
-			char mask_str[NR_CPUS];
 
-			cpumask_scnprintf(mask_str, NR_CPUS, sd->span);
+			cpumask_scnprintf(mask_str, mask_len, sd->span);
 			seq_printf(seq, "domain%d %s", dcount++, mask_str);
 			for (itype = CPU_IDLE; itype < CPU_MAX_IDLE_TYPES;
 					itype++) {

-- 

[PATCH 3/3] cpumask: add show cpu map functions v2

[Mike Travis]

[-- Attachment #1: add-cpu-show-map-funcs --]
[-- Type: text/plain, Size: 4086 bytes --]

  * Add cpu_sysdev_class functions to display the following maps
    with cpulist_scnprintf().

	cpu_online_map
	cpu_present_map
	cpu_possible_map
	cpu_system_map

  * Small change to include/linux/sysdev.h to allow the attribute
    name and label to be different (to avoid collision with the
    "attr_online" entry for bringing cpus on- and off-line.)

Based on:
	git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
    +   x86/latest          .../x86/linux-2.6-x86.git
    +   sched-devel/latest  .../mingo/linux-2.6-sched-devel.git

# x86
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: H. Peter Anvin <hpa@zytor.com>

Signed-off-by: Mike Travis <travis@sgi.com>
---
v2:
    Removed cpus_scnprintf() and all references.
---
 drivers/base/cpu.c     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/sysdev.h |   17 ++++++++++------
 2 files changed, 61 insertions(+), 6 deletions(-)

--- linux-2.6.x86.sched.orig/drivers/base/cpu.c
+++ linux-2.6.x86.sched/drivers/base/cpu.c
@@ -103,6 +103,53 @@ static SYSDEV_ATTR(crash_notes, 0400, sh
 #endif
 
 /*
+ * Print cpu online, possible, present, and system maps
+ */
+static ssize_t print_cpus_map(char *buf, cpumask_t *map)
+{
+	int n = cpulist_scnprintf(buf, PAGE_SIZE-2, *map);
+
+	buf[n++] = '\n';
+	buf[n] = '\0';
+	return n;
+}
+
+#define	print_cpus_func(type) \
+static ssize_t print_cpus_##type(struct sysdev_class *class, char *buf)	\
+{									\
+	return print_cpus_map(buf, &cpu_##type##_map);			\
+}									\
+struct sysdev_class_attribute attr_##type##_map = 			\
+	_SYSDEV_CLASS_ATTR(type, 0444, print_cpus_##type, NULL)
+
+print_cpus_func(online);
+print_cpus_func(possible);
+print_cpus_func(present);
+print_cpus_func(system);
+
+struct sysdev_class_attribute *cpu_state_attr[] = {
+	&attr_online_map,
+	&attr_possible_map,
+	&attr_present_map,
+	&attr_system_map,
+};
+
+static int cpu_states_init(void)
+{
+	int i;
+	int err = 0;
+
+	for (i = 0;  i < ARRAY_SIZE(cpu_state_attr); i++) {
+		int ret;
+		ret = sysdev_class_create_file(&cpu_sysdev_class,
+						cpu_state_attr[i]);
+		if (!err)
+			err = ret;
+	}
+	return err;
+}
+
+/*
  * register_cpu - Setup a sysfs device for a CPU.
  * @cpu - cpu->hotpluggable field set to 1 will generate a control file in
  *	  sysfs for this CPU.
@@ -147,6 +194,9 @@ int __init cpu_dev_init(void)
 	int err;
 
 	err = sysdev_class_register(&cpu_sysdev_class);
+	if (!err)
+		err = cpu_states_init();
+
 #if defined(CONFIG_SCHED_MC) || defined(CONFIG_SCHED_SMT)
 	if (!err)
 		err = sched_create_sysfs_power_savings_entries(&cpu_sysdev_class);
--- linux-2.6.x86.sched.orig/include/linux/sysdev.h
+++ linux-2.6.x86.sched/include/linux/sysdev.h
@@ -45,12 +45,16 @@ struct sysdev_class_attribute {
 	ssize_t (*store)(struct sysdev_class *, const char *, size_t);
 };
 
-#define SYSDEV_CLASS_ATTR(_name,_mode,_show,_store) 		\
-struct sysdev_class_attribute attr_##_name = { 			\
+#define _SYSDEV_CLASS_ATTR(_name,_mode,_show,_store) 		\
+{					 			\
 	.attr = {.name = __stringify(_name), .mode = _mode },	\
 	.show	= _show,					\
 	.store	= _store,					\
-};
+}
+
+#define SYSDEV_CLASS_ATTR(_name,_mode,_show,_store) 		\
+	struct sysdev_class_attribute attr_##_name = 		\
+		_SYSDEV_CLASS_ATTR(_name,_mode,_show,_store)
 
 
 extern int sysdev_class_register(struct sysdev_class *);
@@ -100,15 +104,16 @@ struct sysdev_attribute { 
 };
 
 
-#define _SYSDEV_ATTR(_name,_mode,_show,_store)			\
+#define _SYSDEV_ATTR(_name, _mode, _show, _store)		\
 {								\
 	.attr = { .name = __stringify(_name), .mode = _mode },	\
 	.show	= _show,					\
 	.store	= _store,					\
 }
 
-#define SYSDEV_ATTR(_name,_mode,_show,_store)		\
-struct sysdev_attribute attr_##_name = _SYSDEV_ATTR(_name,_mode,_show,_store);
+#define SYSDEV_ATTR(_name, _mode, _show, _store)		\
+	struct sysdev_attribute attr_##_name =			\
+		_SYSDEV_ATTR(_name, _mode, _show, _store);
 
 extern int sysdev_create_file(struct sys_device *, struct sysdev_attribute *);
 extern void sysdev_remove_file(struct sys_device *, struct sysdev_attribute *);

-- 

Re: [PATCH 3/3] cpumask: add show cpu map functions v2

[Bert Wesarg]

On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>   * Add cpu_sysdev_class functions to display the following maps
>     with cpulist_scnprintf().
>
Could we add a cpumap_scnprintf() attribute too, just for consistency?

Bert

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>
>   * Cleanup usages of cpumask_scprintf in the following files and add
>     another interface to use cpulist_scnprintf where appropriate.
Great. Sometimes the simplest solution needs the longest way. ;-)

Thanks.
Bert

Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Bert Wesarg]

On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>  --- linux-2.6.x86.sched.orig/arch/x86/kernel/cpu/intel_cacheinfo.c
>  +++ linux-2.6.x86.sched/arch/x86/kernel/cpu/intel_cacheinfo.c
>  @@ -591,20 +591,34 @@ static ssize_t show_size(struct _cpuid4_
>         return sprintf (buf, "%luK\n", this_leaf->size / 1024);
>   }
>
>  -static ssize_t show_shared_cpu_map(struct _cpuid4_info *this_leaf, char *buf)
>  +static ssize_t show_shared_cpu_map_func(struct _cpuid4_info *this_leaf,
>  +                                       int type, char *buf)
I don't know if this is more efficient or better or cleaner, but how
about to pass the function pointer (ie. cpulist_scnprintf or
cpumap_scnprintf)? You safe at least one condition.

>  +               n = type?
>  +                       cpulist_scnprintf(buf, len-2, *mask):
>  +                       cpumask_scnprintf(buf, len-2, *mask);

Thanks.
Bert

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Bert Wesarg]

On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>  --- linux-2.6.x86.sched.orig/drivers/base/node.c
>  +++ linux-2.6.x86.sched/drivers/base/node.c
>  @@ -19,22 +19,34 @@ static struct sysdev_class node_class =
>   };
>
>
>  -static ssize_t node_read_cpumap(struct sys_device * dev, char * buf)
>  +static ssize_t node_read_cpumap(struct sys_device *dev, int type, char *buf)
>   {
>         struct node *node_dev = to_node(dev);
>         node_to_cpumask_ptr(mask, node_dev->sysdev.id);
>         int len;
>
>  -       /* 2004/06/03: buf currently PAGE_SIZE, need > 1 char per 4 bits. */
>  -       BUILD_BUG_ON(MAX_NUMNODES/4 > PAGE_SIZE/2);
>  +       /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
>  +       BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
Is this right, that you switch from MAX_NUMNODES to NR_CPUS?

Regards
Bert

Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Mike Travis]

Bert Wesarg wrote:
> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>>  --- linux-2.6.x86.sched.orig/arch/x86/kernel/cpu/intel_cacheinfo.c
>>  +++ linux-2.6.x86.sched/arch/x86/kernel/cpu/intel_cacheinfo.c
>>  @@ -591,20 +591,34 @@ static ssize_t show_size(struct _cpuid4_
>>         return sprintf (buf, "%luK\n", this_leaf->size / 1024);
>>   }
>>
>>  -static ssize_t show_shared_cpu_map(struct _cpuid4_info *this_leaf, char *buf)
>>  +static ssize_t show_shared_cpu_map_func(struct _cpuid4_info *this_leaf,
>>  +                                       int type, char *buf)
> I don't know if this is more efficient or better or cleaner, but how
> about to pass the function pointer (ie. cpulist_scnprintf or
> cpumap_scnprintf)? You safe at least one condition.
> 
>>  +               n = type?
>>  +                       cpulist_scnprintf(buf, len-2, *mask):
>>  +                       cpumask_scnprintf(buf, len-2, *mask);
> 
> Thanks.
> Bert

I have to subvert the cpumask interface a bit, but the resultant
code size is about 7 instructions smaller.

static ssize_t show_shared_cpu_map_func(struct _cpuid4_info *this_leaf,
		int (*cpumap_func)(char *, int, const cpumask_t *, int),
		char *buf)
{
	ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
	int n = 0;

	if (len > 1) {
		cpumask_t *mask = &this_leaf->shared_cpu_map;
		n = cpumap_func(buf, len-2, mask, NR_CPUS);
		buf[n++] = '\n';
		buf[n] = '\0';
	}
	return n;
}

static inline ssize_t show_shared_cpu_map(struct _cpuid4_info *leaf, char *buf)
{
	return show_shared_cpu_map_func(leaf, &__cpumask_scnprintf, buf);
}

static inline ssize_t show_shared_cpu_list(struct _cpuid4_info *leaf, char *buf)
{
	return show_shared_cpu_map_func(leaf, &__cpulist_scnprintf, buf);
}

Btw, you were asking about how to determine NR_CPUS.  Here's one way:

root@newton:~# cat /sys/devices/system/cpu/online
0-2,4-7
root@newton:~# cat /sys/devices/system/cpu/possible
0-511
root@newton:~# cat /sys/devices/system/cpu/present
0-7
root@newton:~# cat /sys/devices/system/cpu/system
0-4095

Thanks,
Mike

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Mike Travis]

Bert Wesarg wrote:
> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>>  --- linux-2.6.x86.sched.orig/drivers/base/node.c
>>  +++ linux-2.6.x86.sched/drivers/base/node.c
>>  @@ -19,22 +19,34 @@ static struct sysdev_class node_class =
>>   };
>>
>>
>>  -static ssize_t node_read_cpumap(struct sys_device * dev, char * buf)
>>  +static ssize_t node_read_cpumap(struct sys_device *dev, int type, char *buf)
>>   {
>>         struct node *node_dev = to_node(dev);
>>         node_to_cpumask_ptr(mask, node_dev->sysdev.id);
>>         int len;
>>
>>  -       /* 2004/06/03: buf currently PAGE_SIZE, need > 1 char per 4 bits. */
>>  -       BUILD_BUG_ON(MAX_NUMNODES/4 > PAGE_SIZE/2);
>>  +       /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
>>  +       BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
> Is this right, that you switch from MAX_NUMNODES to NR_CPUS?
> 
> Regards
> Bert

It's printing the number of cpus on a node, so the number of nodes is not
important, it's how many cpus can fit on the head of a node... ;-)

Thanks,
Mike

Re: [PATCH 3/3] cpumask: add show cpu map functions v2

[Mike Travis]

Bert Wesarg wrote:
> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>>   * Add cpu_sysdev_class functions to display the following maps
>>     with cpulist_scnprintf().
>>
> Could we add a cpumap_scnprintf() attribute too, just for consistency?
> 
> Bert

Sure, I could add it.  But by not having it, the user is forced to
actually read libbitmask(3) to see a different way of parsing the
output, perhaps weaning them off of cpumask_scnprintf()... ;-)

Do you mind if I add them as:

	/sys/devices/system/cpu/online_map
	/sys/devices/system/cpu/possible_map
	/sys/devices/system/cpu/present_map
	/sys/devices/system/cpu/system_map

Thanks,
Mike

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Mike Travis]

Bert Wesarg wrote:
> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>>   * Cleanup usages of cpumask_scprintf in the following files and add
>>     another interface to use cpulist_scnprintf where appropriate.
> Great. Sometimes the simplest solution needs the longest way. ;-)
> 
> Thanks.
> Bert

I agree though sometimes "acceptance" is the hardest part... ;-)

Thanks,
Mike

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

>   * Add cpu_sysdev_class functions to display the
> 
>   * Cleanup usages of cpumask_scprintf in the following files and add
>     another interface to use cpulist_scnprintf where appropriate.

thanks Mike, applied.

	Ingo

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Bert Wesarg]

On Tue, Apr 8, 2008 at 10:46 PM, Mike Travis <travis@sgi.com> wrote:
>
> Bert Wesarg wrote:
>  > On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>  >>  --- linux-2.6.x86.sched.orig/drivers/base/node.c
>  >>  +++ linux-2.6.x86.sched/drivers/base/node.c
>  >>  @@ -19,22 +19,34 @@ static struct sysdev_class node_class =
>  >>   };
>  >>
>  >>
>  >>  -static ssize_t node_read_cpumap(struct sys_device * dev, char * buf)
>  >>  +static ssize_t node_read_cpumap(struct sys_device *dev, int type, char *buf)
>  >>   {
>  >>         struct node *node_dev = to_node(dev);
>  >>         node_to_cpumask_ptr(mask, node_dev->sysdev.id);
>  >>         int len;
>  >>
>  >>  -       /* 2004/06/03: buf currently PAGE_SIZE, need > 1 char per 4 bits. */
>  >>  -       BUILD_BUG_ON(MAX_NUMNODES/4 > PAGE_SIZE/2);
>  >>  +       /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
>  >>  +       BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
>  > Is this right, that you switch from MAX_NUMNODES to NR_CPUS?
>  >
>  > Regards
>  > Bert
>
>  It's printing the number of cpus on a node, so the number of nodes is not
>  important, it's how many cpus can fit on the head of a node... ;-)
Ahh, the old code was Just Plain Wrong.

Thanks.
Bert
>
>  Thanks,
>  Mike
>

Re: [PATCH 3/3] cpumask: add show cpu map functions v2

[Bert Wesarg]

On Tue, Apr 8, 2008 at 10:52 PM, Mike Travis <travis@sgi.com> wrote:
>
> Bert Wesarg wrote:
>  > On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>  >>   * Add cpu_sysdev_class functions to display the following maps
>  >>     with cpulist_scnprintf().
>  >>
>  > Could we add a cpumap_scnprintf() attribute too, just for consistency?
>  >
>  > Bert
>
>  Sure, I could add it.  But by not having it, the user is forced to
>  actually read libbitmask(3) to see a different way of parsing the
>  output, perhaps weaning them off of cpumask_scnprintf()... ;-)
>
>  Do you mind if I add them as:
>
>         /sys/devices/system/cpu/online_map
>         /sys/devices/system/cpu/possible_map
>         /sys/devices/system/cpu/present_map
>         /sys/devices/system/cpu/system_map
Please go ahead.

Bert

PS: this is the only source I found for the libbitmask(3):

ftp://ftp.cvut.cz/kernel/people/christoph/pmig/ray-mmigrate/

but I suspect there is a more official place for this.

>
>  Thanks,
>  Mike
>

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>   * Cleanup usages of cpumask_scprintf in the following files and add
>     another interface to use cpulist_scnprintf where appropriate.

On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
>  Part of the change is readability, but also looking towards the future
>  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
>  PAGE_SIZE buffer provided (though some pathological cases will overflow
>  the range method as well.)  So we'll need some advancement in the format
>  of the printout.

Btw, I think you can now push for a deprecation of the 'old' mask
attributes, with the justification you have given above. The other
possibility is to change sysfs to provide bigger attribute buffers
(CCed Greg for this).

Bert

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Greg KH]

On Wed, Apr 09, 2008 at 07:51:23PM +0200, Bert Wesarg wrote:
> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
> >   * Cleanup usages of cpumask_scprintf in the following files and add
> >     another interface to use cpulist_scnprintf where appropriate.
> 
> On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
> >  Part of the change is readability, but also looking towards the future
> >  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
> >  PAGE_SIZE buffer provided (though some pathological cases will overflow
> >  the range method as well.)  So we'll need some advancement in the format
> >  of the printout.
> 
> Btw, I think you can now push for a deprecation of the 'old' mask
> attributes, with the justification you have given above. The other
> possibility is to change sysfs to provide bigger attribute buffers
> (CCed Greg for this).

Huh?

sysfs is "one value per file", if you are getting close to PAGE_SIZE in
any sysfs file, then you are doing something very wrong.

What sysfs file currently is trying to output data this big?

thanks,

greg k-h

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Wed, Apr 9, 2008 at 10:39 PM, Greg KH <gregkh@suse.de> wrote:
>
> On Wed, Apr 09, 2008 at 07:51:23PM +0200, Bert Wesarg wrote:
>  > On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>  > >   * Cleanup usages of cpumask_scprintf in the following files and add
>  > >     another interface to use cpulist_scnprintf where appropriate.
>  >
>  > On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
>  > >  Part of the change is readability, but also looking towards the future
>  > >  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
>  > >  PAGE_SIZE buffer provided (though some pathological cases will overflow
>  > >  the range method as well.)  So we'll need some advancement in the format
>  > >  of the printout.
>  >
>  > Btw, I think you can now push for a deprecation of the 'old' mask
>  > attributes, with the justification you have given above. The other
>  > possibility is to change sysfs to provide bigger attribute buffers
>  > (CCed Greg for this).
>
>  Huh?
>
>  sysfs is "one value per file", if you are getting close to PAGE_SIZE in
>  any sysfs file, then you are doing something very wrong.
>
>  What sysfs file currently is trying to output data this big?
Currently none. But if NR_CPUS >= 16K and PAGE_SIZE == 4K, than any
file with an cpumask_scnprintf().

Thanks,
Bert
>
>  thanks,
>
>  greg k-h
>

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Mike Travis]

Greg KH wrote:
> On Wed, Apr 09, 2008 at 07:51:23PM +0200, Bert Wesarg wrote:
>> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>>>   * Cleanup usages of cpumask_scprintf in the following files and add
>>>     another interface to use cpulist_scnprintf where appropriate.
>> On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
>>>  Part of the change is readability, but also looking towards the future
>>>  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
>>>  PAGE_SIZE buffer provided (though some pathological cases will overflow
>>>  the range method as well.)  So we'll need some advancement in the format
>>>  of the printout.
>> Btw, I think you can now push for a deprecation of the 'old' mask
>> attributes, with the justification you have given above. The other
>> possibility is to change sysfs to provide bigger attribute buffers
>> (CCed Greg for this).
> 
> Huh?
> 
> sysfs is "one value per file", if you are getting close to PAGE_SIZE in
> any sysfs file, then you are doing something very wrong.
> 
> What sysfs file currently is trying to output data this big?
> 
> thanks,
> 
> greg k-h

Hi Greg,

There's none at the moment.  The increase is coming from printing the
cpuset for various attributes, like cpus on a node, etc.  Since it uses
cpumask_scnprintf(), this prints a bit map representing a cpumask_t.

With the increase to 4096 cpus, this string is now 1152 bytes long.  The
next iteration will have 16384 cpus which will need 4608 bytes to fully
display, overflowing a standard page.  I've added alternate interfaces
that use cpulist_scnprintf() which has the advantage of collapsing the
bits into ranges.  This though can result in a much larger output size
if, for example only every other bit is set.

Btw, where does one value per file come from?  I see outputs like:

# cat /proc/self/stat
4313 (cat) R 4218 4313 4218 34816 4313 4194304 207 0 0 0 0 0 0 0 20 0 1 0 6802916 5672960 131 18446744073709551615 4194304 4212948 140735962676160 18446744073709551615 140499600349840 0 0 0 0 0 0 0 17 3 0 0 0 0 0

Thanks,
Mike

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Wed, Apr 9, 2008 at 10:59 PM, Mike Travis <travis@sgi.com> wrote:
>  Btw, where does one value per file come from?  I see outputs like:
>
>  # cat /proc/self/stat
>  4313 (cat) R 4218 4313 4218 34816 4313 4194304 207 0 0 0 0 0 0 0 20 0 1 0 6802916 5672960 131 18446744073709551615 4194304 4212948 140735962676160 18446744073709551615 140499600349840 0 0 0 0 0 0 0 17 3 0 0 0 0 0
This isn't sysfs.

Bert
>
>  Thanks,
>  Mike
>

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Greg KH]

On Wed, Apr 09, 2008 at 10:52:58PM +0200, Bert Wesarg wrote:
> On Wed, Apr 9, 2008 at 10:39 PM, Greg KH <gregkh@suse.de> wrote:
> >
> > On Wed, Apr 09, 2008 at 07:51:23PM +0200, Bert Wesarg wrote:
> >  > On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
> >  > >   * Cleanup usages of cpumask_scprintf in the following files and add
> >  > >     another interface to use cpulist_scnprintf where appropriate.
> >  >
> >  > On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
> >  > >  Part of the change is readability, but also looking towards the future
> >  > >  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
> >  > >  PAGE_SIZE buffer provided (though some pathological cases will overflow
> >  > >  the range method as well.)  So we'll need some advancement in the format
> >  > >  of the printout.
> >  >
> >  > Btw, I think you can now push for a deprecation of the 'old' mask
> >  > attributes, with the justification you have given above. The other
> >  > possibility is to change sysfs to provide bigger attribute buffers
> >  > (CCed Greg for this).
> >
> >  Huh?
> >
> >  sysfs is "one value per file", if you are getting close to PAGE_SIZE in
> >  any sysfs file, then you are doing something very wrong.
> >
> >  What sysfs file currently is trying to output data this big?
> Currently none. But if NR_CPUS >= 16K and PAGE_SIZE == 4K, than any
> file with an cpumask_scnprintf().

Do we have such sysfs files today?

thanks,

greg k-h

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Greg KH]

On Wed, Apr 09, 2008 at 01:59:39PM -0700, Mike Travis wrote:
> Greg KH wrote:
> > On Wed, Apr 09, 2008 at 07:51:23PM +0200, Bert Wesarg wrote:
> >> On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
> >>>   * Cleanup usages of cpumask_scprintf in the following files and add
> >>>     another interface to use cpulist_scnprintf where appropriate.
> >> On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
> >>>  Part of the change is readability, but also looking towards the future
> >>>  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
> >>>  PAGE_SIZE buffer provided (though some pathological cases will overflow
> >>>  the range method as well.)  So we'll need some advancement in the format
> >>>  of the printout.
> >> Btw, I think you can now push for a deprecation of the 'old' mask
> >> attributes, with the justification you have given above. The other
> >> possibility is to change sysfs to provide bigger attribute buffers
> >> (CCed Greg for this).
> > 
> > Huh?
> > 
> > sysfs is "one value per file", if you are getting close to PAGE_SIZE in
> > any sysfs file, then you are doing something very wrong.
> > 
> > What sysfs file currently is trying to output data this big?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> There's none at the moment.  The increase is coming from printing the
> cpuset for various attributes, like cpus on a node, etc.  Since it uses
> cpumask_scnprintf(), this prints a bit map representing a cpumask_t.
> 
> With the increase to 4096 cpus, this string is now 1152 bytes long.  The
> next iteration will have 16384 cpus which will need 4608 bytes to fully
> display, overflowing a standard page.  I've added alternate interfaces
> that use cpulist_scnprintf() which has the advantage of collapsing the
> bits into ranges.  This though can result in a much larger output size
> if, for example only every other bit is set.
> 
> Btw, where does one value per file come from?

Documentation/filesystems/sysfs.txt, while a bit outdated, still
contains this rule.

> I see outputs like:
> 
> # cat /proc/self/stat
> 4313 (cat) R 4218 4313 4218 34816 4313 4194304 207 0 0 0 0 0 0 0 20 0 1 0 6802916 5672960 131 18446744073709551615 4194304 4212948 140735962676160 18446744073709551615 140499600349840 0 0 0 0 0 0 0 17 3 0 0 0 0 0

That's fine, it's /proc/, not /sys/ :)

thanks,

greg k-h

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Wed, Apr 9, 2008 at 11:14 PM, Greg KH <gregkh@suse.de> wrote:
>
> On Wed, Apr 09, 2008 at 10:52:58PM +0200, Bert Wesarg wrote:
>  > On Wed, Apr 9, 2008 at 10:39 PM, Greg KH <gregkh@suse.de> wrote:
>  > >
>  > > On Wed, Apr 09, 2008 at 07:51:23PM +0200, Bert Wesarg wrote:
>  > >  > On Tue, Apr 8, 2008 at 8:43 PM, Mike Travis <travis@sgi.com> wrote:
>  > >  > >   * Cleanup usages of cpumask_scprintf in the following files and add
>  > >  > >     another interface to use cpulist_scnprintf where appropriate.
>  > >  >
>  > >  > On Mon, Apr 7, 2008 at 8:22 PM, Mike Travis <travis@sgi.com> wrote:
>  > >  > >  Part of the change is readability, but also looking towards the future
>  > >  > >  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
>  > >  > >  PAGE_SIZE buffer provided (though some pathological cases will overflow
>  > >  > >  the range method as well.)  So we'll need some advancement in the format
>  > >  > >  of the printout.
>  > >  >
>  > >  > Btw, I think you can now push for a deprecation of the 'old' mask
>  > >  > attributes, with the justification you have given above. The other
>  > >  > possibility is to change sysfs to provide bigger attribute buffers
>  > >  > (CCed Greg for this).
>  > >
>  > >  Huh?
>  > >
>  > >  sysfs is "one value per file", if you are getting close to PAGE_SIZE in
>  > >  any sysfs file, then you are doing something very wrong.
>  > >
>  > >  What sysfs file currently is trying to output data this big?
>  > Currently none. But if NR_CPUS >= 16K and PAGE_SIZE == 4K, than any
>  > file with an cpumask_scnprintf().
>
>  Do we have such sysfs files today?
Files with cpumask_scnprintf()? Sure, the cpu topology exports plenty
of cpu masks.

If you ask for files with that big NR_CPUS, than no. We are talking
only for future scenarios.

Bert
>
>  thanks,
>
>  greg k-h
>

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Paul Jackson]

Mike wrote:
> Btw, where does one value per file come from?  I see outputs like:
> ... /proc/self/stat

The one value per file rule comes in part from files such as this
stat file.  We've been learning the hard way that complex ascii file
formats tend to be a pain to parse, and a pain to evolve.  So we're
trying to do better in future files.  Greg K-H has been especially
clear on the need to do this in /sys files.  We can't change the format
of old files very well, but we can do better with new files we add.
The one most common exception to this rule that I find persuasive is
for a vector of values of the same type and semantics.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 3/3] cpumask: add show cpu map functions v2

[Paul Jackson]

Bert wrote:
> PS: this is the only source I found for the libbitmask(3):

Mike has been mentioning libbitmask as if it were readily
available outside SGI.  That's not entirely true, yet.

SGI delivers libbitmask as part of its products to its
customers under an LGPL license, and I provide libbitmask
sources to others on request.  Real Soon Now (tm) I will
be publishing libbitmask sources on our oss.sgi.com web
site for one and all to make use of, under LGPL license.
But I haven't done that ... yet.

Libbitmask is a user level library, similar in purpose to,
but quite different in internal implementation and API
details from, the bitmap, cpumask and nodemask types in
the kernel.

A second library, libcpuset, is also delivered by SGI under
LGPL license, and provides a rich C library for use by
major applications requiring a robust cpuset interface.
This library too should become more easily obtained, once
I get around to publishing it on oss.sgi.com.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Paul Jackson]

Mike wrote:
> added new cpulist_scnprintf() interfaces where appropriate.

Nice - thanks.

When I get off vacation tomorrow, I will try to review this
more closely.  However I definitely approve of the idea of
the separate, list formatted, /sys and /proc files.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 3/3] cpumask: add show cpu map functions v2

[Bert Wesarg]

On Thu, Apr 10, 2008 at 2:29 AM, Paul Jackson <pj@sgi.com> wrote:
> Bert wrote:
>  > PS: this is the only source I found for the libbitmask(3):
>
>  Mike has been mentioning libbitmask as if it were readily
>  available outside SGI.  That's not entirely true, yet.
>
>  SGI delivers libbitmask as part of its products to its
>  customers under an LGPL license, and I provide libbitmask
>  sources to others on request.  Real Soon Now (tm) I will
>  be publishing libbitmask sources on our oss.sgi.com web
>  site for one and all to make use of, under LGPL license.
>  But I haven't done that ... yet.
>
>  Libbitmask is a user level library, similar in purpose to,
>  but quite different in internal implementation and API
>  details from, the bitmap, cpumask and nodemask types in
>  the kernel.
>
>  A second library, libcpuset, is also delivered by SGI under
>  LGPL license, and provides a rich C library for use by
>  major applications requiring a robust cpuset interface.
>  This library too should become more easily obtained, once
>  I get around to publishing it on oss.sgi.com.
Thank you for this information.

Bert
>
>  --
>                   I won't rest till it's the best ...
>                   Programmer, Linux Scalability
>                   Paul Jackson <pj@sgi.com> 1.940.382.4214
>

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Paul Jackson]

Mike wrote:
>  Part of the change is readability, but also looking towards the future
>  of 16k/64k/??? # of cpus, the straight mask approach will overflow the
>  PAGE_SIZE buffer provided (though some pathological cases will overflow
>  the range method as well.)

Bert wrote:
> Btw, I think you can now push for a deprecation of the 'old' mask
> attributes, with the justification you have given above. The other
> possibility is to change sysfs to provide bigger attribute buffers
> (CCed Greg for this).

Note what Mike said -- some pathological cases will overflow the range
(what I call "list") format as well.

Indeed, the worst case "list" format is worse than the worst case "mask"
format.  Masks take a constant 9 chars per 32 bits, or 9/32 chars/bit.

Worst case lists involve every other CPU or node (all the even ones, or
all the odd ones.)  For CPUs or Nodes that take five decimal digits
(10000 to 65535 -- some of these are u16 kernel types internall) this
amounts to 6 chars every 2 possible values, or 3 chars/bit, which is
quite a bit more than 9/32 chars/bit.

For example, the list of odd CPUs from 1 to 65535 takes 191053
characters:

  1,3,5,7,9,11,13,15,17,19,...,65517,65519,65521,65523,65525,65527,65529,65531,65533,65535

This will overflow any ordinary page size.  The corresponding mask
takes only 18432 characters:

  AAAAAAAA,AAAAAAAA,AAAAAAAA,AAAAAAAA,AAAAAAAA,AAAAAAAA,...

Deprecating the mask in favor of the list on account of the
mask not fitting makes little sense to me, because worst case,
the list is even bigger ;).

Granted, the above examples consider the extreme case of
NR_CPUS == 65536 or some such.  But, as Mike notes, NR_CPUS
of 16384 might be needed; and the above quandry still applies
in that case.

Hmmm ... there are even more pathological list cases.  Take two
out of every three (drop those congruent to 0 mod 3):

  1,2,4,5,7,8,10,11,13,14,...,65521,65522,65524,65525,65527,65528,65530,65531,65533,65534

This requires 254736 characters ;).

Even for less insane values, of say two out of three CPUs when
NR_CPUS == 4096, it takes 12912 characters:

  1,2,4,5,7,8,10,11,13,14,...,4082,4084,4085,4087,4088,4090,4091,4093,4094

Whereas the mask format for 4096 NR_CPUS takes just 1152 characters.

On a system with 4K page size, the above two out of three list would
not actually show as that 12912 character string.  With the current
code, it would show as a 4094 character string, plus the trailing
newline and nul char, ending with (if I did my math right):

  ,1436,1438,1439,1441,1442,1444,1445,1447,1448,14

This is obviously not perfect from an ideal perspective.

However, I can't see that these pathological cases are enough of a
practical problem that we should actually spend code addressing them
at present.

On the other hand, and my main point of this message, I can't
see deprecating the mask format files on account of this sort
of analysis.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Paul Jackson]

Mike wrote:
> added new cpulist_scnprintf() interfaces where appropriate.

Mike - could you actually list the new *list files added
by this patch?  I suspect that you can start with the
private email I sent you analyzing v2 of this patch to
obtain such a list.  If we're going to add some new /proc
and /sys files, we should list them, explicitly, by name,
and not just wave our hands with a "where appropriate."

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Paul Jackson]

Bert, responding to Mike:
> >  It's printing the number of cpus on a node, so the number of nodes is not
> >  important, it's how many cpus can fit on the head of a node... ;-)
> Ahh, the old code was Just Plain Wrong.

Looks like the code originally had NR_CPUS, and that was changed to
MAX_NUMNODES, perhaps as unintended collateral damage from some
x86_64 node SLIT changes done in the patch:

  http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.10-rc3/2.6.10-rc3-mm1/broken-out/x86_64-add-x86_64-support-for-jack-steiners-slit-sysfs.patch

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Paul Jackson]

Mike wrote:
+		n = type?
+			cpulist_scnprintf(buf, len-2, *mask):
+			cpumask_scnprintf(buf, len-2, *mask);

I suspect most of us would find the following variant easier to read:

		if (type)
			n = cpulist_scnprintf(buf, len - 2, *mask);
		else
			n = cpumask_scnprintf(buf, len - 2, *mask);


Then, going further, the rather too vague "type" parameter name,
without comment and taking just bare constant values 0 or 1, seems
more opaque than necessary.

I can imagine this being easier to read as something like:


typedef enum { print_as_mask, print_as_list } map_printer_t;

static ssize_t show_shared_cpu_map_func(struct _cpuid4_info *this_leaf,
				       map_printer_t mpt, char *buf)
{
       ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
       int n = 0;

       if (len > 1) {
		cpumask_t *mask = &this_leaf->shared_cpu_map;

		if (mpt == print_as_mask)
			n = cpumask_scnprintf(buf, len - 2, *mask);
		else
			n = cpulist_scnprintf(buf, len - 2, *mask);
		buf[n++] = '\n';
		buf[n] = '\0';
       }
       return n;
}

static inline ssize_t show_shared_cpu_map(struct _cpuid4_info *leaf, char *buf)
{
       return show_shared_cpu_map_func(leaf, print_as_mask, buf);
}

static inline ssize_t show_shared_cpu_list(struct _cpuid4_info *leaf, char *buf)
{
       return show_shared_cpu_map_func(leaf, print_as_list, buf);
}



-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Paul Jackson]

Mike, in response to a suggestion from Bert, wrote:
> I have to subvert the cpumask interface a bit, but the resultant
> code size is about 7 instructions smaller.

I should have read ahead to this discussion, before responding
a minute ago with my suggestion using:

  typedef enum { print_as_mask, print_as_list } map_printer_t;

I slightly dislike the subversion of the cpumask interface,
and slightly -do- like the 7 instructions saved.

Either of these variants (my typedef or Berts function pointer)
would be fine by me, as you prefer, Mike.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 2/3] cpumask: use new cpus_scnprintf function v3

[Paul Jackson]

Mike wrote:
+	if (len > 1) {
+		n = type?
+			cpulist_scnprintf(buf, len-2, *mask):
+			cpumask_scnprintf(buf, len-2, *mask);

I suppose that the discussion of PATCH 1/3 applies to this code too.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 3/3] cpumask: add show cpu map functions v2

[Paul Jackson]

Mike wrote:
>   * Add cpu_sysdev_class functions to display the following maps
>     with cpulist_scnprintf().

Nice - thanks.

Reviewed-by: Paul Jackson <pj@sgi.com>

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Paul Jackson]

Mike wrote:
> I agree though sometimes "acceptance" is the hardest part... ;-)

For changes that would have changed several kernel interfaces
incompatibly, the bar of acceptance should be higher than for
changes that only affect kernel internals.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Thu, Apr 10, 2008 at 2:10 PM, Paul Jackson <pj@sgi.com> wrote:
> Bert wrote:
>  > Btw, I think you can now push for a deprecation of the 'old' mask
>  > attributes, with the justification you have given above. The other
>  > possibility is to change sysfs to provide bigger attribute buffers
>  > (CCed Greg for this).
>
>  On the other hand, and my main point of this message, I can't
>  see deprecating the mask format files on account of this sort
>  of analysis.
>
My statement from above doesn't reflect my opinion. I'm still in
flavor with the mask output. And from this discussion, I found a new
point for the mask output: its bounded ;-)

I just wanted to note, that these new list attributes would be the
only way to 'change' the api, ie. introduce a new api and deprecate
the old one, and not change the format of the present api.

Unfortunately, to support the mask attributes beyond 4k cpus, sysfs
has to support greater attribute buffers.

Bert

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Paul Jackson]

Bert wrote:
> My statement from above doesn't reflect my opinion. I'm still in
> flavor with the mask output. And from this discussion, I found a new
> point for the mask output: its bounded ;-)

Ok

> I just wanted to note, that these new list attributes would be the
> only way to 'change' the api, ie. introduce a new api and deprecate
> the old one, and not change the format of the present api.

Agreed

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Bert Wesarg]

On Tue, Apr 8, 2008 at 10:44 PM, Mike Travis <travis@sgi.com> wrote:
>  Btw, you were asking about how to determine NR_CPUS.  Here's one way:
>
>  root@newton:~# cat /sys/devices/system/cpu/online
>  0-2,4-7
>  root@newton:~# cat /sys/devices/system/cpu/possible
>  0-511
>  root@newton:~# cat /sys/devices/system/cpu/present
>  0-7
>  root@newton:~# cat /sys/devices/system/cpu/system
>  0-4095
I think its easier to count the commas from a cpumask file. Here you
must parse a complete list expression, just to get the max id.

Either way, I think this isn't a very clean way.

Bert
>  Thanks,
>  Mike
>

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Greg KH]

On Thu, Apr 10, 2008 at 06:14:55PM +0200, Bert Wesarg wrote:
> On Thu, Apr 10, 2008 at 2:10 PM, Paul Jackson <pj@sgi.com> wrote:
> > Bert wrote:
> >  > Btw, I think you can now push for a deprecation of the 'old' mask
> >  > attributes, with the justification you have given above. The other
> >  > possibility is to change sysfs to provide bigger attribute buffers
> >  > (CCed Greg for this).
> >
> >  On the other hand, and my main point of this message, I can't
> >  see deprecating the mask format files on account of this sort
> >  of analysis.
> >
> My statement from above doesn't reflect my opinion. I'm still in
> flavor with the mask output. And from this discussion, I found a new
> point for the mask output: its bounded ;-)
> 
> I just wanted to note, that these new list attributes would be the
> only way to 'change' the api, ie. introduce a new api and deprecate
> the old one, and not change the format of the present api.
> 
> Unfortunately, to support the mask attributes beyond 4k cpus, sysfs
> has to support greater attribute buffers.

Well, it does already today, you just have to work at it :)

What we can do for these types of files, is to use the "binary
attribute" file format.  With that, you get full control over the buffer
size and other operations.

So someone should just wrap up the cpu mask sysfs file usage in a
function that uses the binary attribute instead.  Then everyone who uses
the cpu mask in a sysfs file can use that function instead.

Sound reasonable?

thanks,

greg k-h

Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3

[Paul Jackson]

Bert wrote:
> I think its easier to count the commas from a cpumask file. 

My user level code determines NR_CPUS and MAX_NUMNODES from 32/9 times
the strlen() of the values of the Cpus_allowed and Mems_allowed fields
in /proc/self/status (being careful to -include- the trailing newline
in the strlen.)

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH 0/3] x86: add cpus_scnprintf function v3

[Bert Wesarg]

On Thu, Apr 10, 2008 at 7:30 PM, Greg KH <gregkh@suse.de> wrote:
>
> On Thu, Apr 10, 2008 at 06:14:55PM +0200, Bert Wesarg wrote:
>  > On Thu, Apr 10, 2008 at 2:10 PM, Paul Jackson <pj@sgi.com> wrote:
>  > > Bert wrote:
>  > >  > Btw, I think you can now push for a deprecation of the 'old' mask
>  > >  > attributes, with the justification you have given above. The other
>  > >  > possibility is to change sysfs to provide bigger attribute buffers
>  > >  > (CCed Greg for this).
>  > >
>  > >  On the other hand, and my main point of this message, I can't
>  > >  see deprecating the mask format files on account of this sort
>  > >  of analysis.
>  > >
>  > My statement from above doesn't reflect my opinion. I'm still in
>  > flavor with the mask output. And from this discussion, I found a new
>  > point for the mask output: its bounded ;-)
>  >
>  > I just wanted to note, that these new list attributes would be the
>  > only way to 'change' the api, ie. introduce a new api and deprecate
>  > the old one, and not change the format of the present api.
>  >
>  > Unfortunately, to support the mask attributes beyond 4k cpus, sysfs
>  > has to support greater attribute buffers.
>
>  Well, it does already today, you just have to work at it :)
>
>  What we can do for these types of files, is to use the "binary
>  attribute" file format.  With that, you get full control over the buffer
>  size and other operations.
>
>  So someone should just wrap up the cpu mask sysfs file usage in a
>  function that uses the binary attribute instead.  Then everyone who uses
>  the cpu mask in a sysfs file can use that function instead.
>
>  Sound reasonable?
Very. Thanks.

Bert
>
>  thanks,
>
>  greg k-h
>