On Wed, 19 May 2021 21:26:30 +0800 Tian Tao tiantao6@hisilicon.com wrote:

Use bin_attribute to display thread_siblings/core_cpus/core_siblings, die_cpus/package_cpus/book_siblings/drawer_siblings,avoiding NR_CPUS too big to cause buff overflow.

This patch is based on the following discussion. https://www.spinics.net/lists/linux-doc/msg95921.html

Signed-off-by: Tian Tao tiantao6@hisilicon.com

v2: rewrite the function name##_read

drivers/base/topology.c | 57 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 4d254fc..1b74ff5 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -14,6 +14,8 @@ #include <linux/hardirq.h> #include <linux/topology.h>

+#define BUFF_SIZE ((NR_CPUS/32)*9)

Needs a comment form why this particular size - probably the pathological case that that would require this much space.

It is also potentially a very large buffer to have on the stack.

#define define_id_show_func(name) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ @@ -21,11 +23,16 @@ static ssize_t name##_show(struct device *dev, \ return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ }

-#define define_siblings_show_map(name, mask) \ -static ssize_t name##_show(struct device *dev, \

• 	   struct device_attribute *attr, char *buf)	\
  

-{ \

• return cpumap_print_to_pagebuf(false, buf, topology_##mask(dev->id));\

+#define define_siblings_read(name, mask) \ +static ssize_t name##_read(struct file *file, struct kobject *kobj, \

• 		  struct bin_attribute *attr, char *buf,	\
  

• 		  loff_t off, size_t count)			\
  

+{ \

• struct device *dev = kobj_to_dev(kobj); \
• char temp[BUFF_SIZE] = {0}; \
• scnprintf(temp, BUFF_SIZE, "%*pb\n", nr_cpu_ids, \

• 	      cpumask_bits(topology_##mask(dev->id)));          \
  

Can we not cache this somewhere? The print is far from trivial given you may well have userspace reading this say 1 byte at a time.

• return memory_read_from_buffer(buf, count, &off, temp, BUFF_SIZE); \

}

#define define_siblings_show_list(name, mask) \ @@ -37,7 +44,7 @@ static ssize_t name##_list_show(struct device *dev, \ }

#define define_siblings_show_func(name, mask) \

• define_siblings_show_map(name, mask); \

• define_siblings_read(name, mask); \ define_siblings_show_list(name, mask)

define_id_show_func(physical_package_id); @@ -50,30 +57,30 @@ define_id_show_func(core_id); static DEVICE_ATTR_RO(core_id);

define_siblings_show_func(thread_siblings, sibling_cpumask); -static DEVICE_ATTR_RO(thread_siblings); +static BIN_ATTR_RO(thread_siblings, 0); static DEVICE_ATTR_RO(thread_siblings_list);

define_siblings_show_func(core_cpus, sibling_cpumask); -static DEVICE_ATTR_RO(core_cpus); +static BIN_ATTR_RO(core_cpus, 0); static DEVICE_ATTR_RO(core_cpus_list);

define_siblings_show_func(core_siblings, core_cpumask); -static DEVICE_ATTR_RO(core_siblings); +static BIN_ATTR_RO(core_siblings, 0); static DEVICE_ATTR_RO(core_siblings_list);

define_siblings_show_func(die_cpus, die_cpumask); -static DEVICE_ATTR_RO(die_cpus); +static BIN_ATTR_RO(die_cpus, 0); static DEVICE_ATTR_RO(die_cpus_list);

define_siblings_show_func(package_cpus, core_cpumask); -static DEVICE_ATTR_RO(package_cpus); +static BIN_ATTR_RO(package_cpus, 0); static DEVICE_ATTR_RO(package_cpus_list);

#ifdef CONFIG_SCHED_BOOK define_id_show_func(book_id); static DEVICE_ATTR_RO(book_id); define_siblings_show_func(book_siblings, book_cpumask); -static DEVICE_ATTR_RO(book_siblings); +static BIN_ATTR_RO(book_siblings, 0); static DEVICE_ATTR_RO(book_siblings_list); #endif

@@ -81,32 +88,41 @@ static DEVICE_ATTR_RO(book_siblings_list); define_id_show_func(drawer_id); static DEVICE_ATTR_RO(drawer_id); define_siblings_show_func(drawer_siblings, drawer_cpumask); -static DEVICE_ATTR_RO(drawer_siblings); +static BIN_ATTR_RO(drawer_siblings, 0); static DEVICE_ATTR_RO(drawer_siblings_list); #endif

+static struct bin_attribute *bin_attrs[] = {

• &bin_attr_thread_siblings,
• &bin_attr_core_cpus,
• &bin_attr_core_siblings,
• &bin_attr_die_cpus,
• &bin_attr_package_cpus,

+#ifdef CONFIG_SCHED_BOOK

• &bin_attr_book_siblings,

+#endif +#ifdef CONFIG_SCHED_DRAWER

• &bin_attr_drawer_siblings,

+#endif

• NULL,

+};

static struct attribute *default_attrs[] = { &dev_attr_physical_package_id.attr, &dev_attr_die_id.attr, &dev_attr_core_id.attr,

• &dev_attr_thread_siblings.attr, &dev_attr_thread_siblings_list.attr,
• &dev_attr_core_cpus.attr, &dev_attr_core_cpus_list.attr,
• &dev_attr_core_siblings.attr, &dev_attr_core_siblings_list.attr,
• &dev_attr_die_cpus.attr, &dev_attr_die_cpus_list.attr,
• &dev_attr_package_cpus.attr, &dev_attr_package_cpus_list.attr,

#ifdef CONFIG_SCHED_BOOK &dev_attr_book_id.attr,

• &dev_attr_book_siblings.attr, &dev_attr_book_siblings_list.attr,

#endif #ifdef CONFIG_SCHED_DRAWER &dev_attr_drawer_id.attr,

• &dev_attr_drawer_siblings.attr, &dev_attr_drawer_siblings_list.attr,

#endif NULL @@ -114,6 +130,7 @@ static struct attribute *default_attrs[] = {

static const struct attribute_group topology_attr_group = { .attrs = default_attrs,

• .bin_attrs = bin_attrs, .name = "topology"

};