On Wed, 26 May 2021 09:25:37 +0100 "Song Bao Hua (Barry Song)" song.bao.hua@hisilicon.com wrote:
-----Original Message----- From: tiantao (H) Sent: Wednesday, May 26, 2021 7:26 PM To: Song Bao Hua (Barry Song) song.bao.hua@hisilicon.com; Jonathan Cameron jonathan.cameron@huawei.com Cc: linuxarm@openeuler.org; tiantao (H) tiantao6@hisilicon.com Subject: [PATCH v6] topology: use bin_attribute to avoid buff overflow
Reading sys/devices/system/cpu/cpu0/topology/ returns cpu topology. However, the size of this file is limited to PAGE_SIZE because of the limitation for sysfs attribute. so we use bin_attribute instead of attribute to avoid 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 v3: remove the temp buffer. v4: rewrite the function name##_read v5: rewrite the patch v6: update the commit message
drivers/base/topology.c | 135
drivers/base/node.c also needs this change.
+++++++++++++++++++++++++++++------------------- 1 file changed, 83 insertions(+), 52 deletions(-)
diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 4d254fc..c46b217 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -14,6 +14,9 @@ #include <linux/hardirq.h> #include <linux/topology.h>
+/* BUFF_SIZE need 9 chars per 32 bits. */ +#define BUFF_SIZE ((NR_CPUS/32)*9)
this is only true for bitmap, but not true for list.
#define define_id_show_func(name) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ @@ -21,25 +24,41 @@ 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_func(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); \
- int len; \
- void *data; \
- data = kvmalloc(BUFF_SIZE, GFP_KERNEL); \
- if (!data) \
return -ENOMEM; \- len = scnprintf(data, BUFF_SIZE, "%*pb\n", nr_cpu_ids, \
cpumask_bits(topology_##mask(dev->id))); \- len = memory_read_from_buffer(buf, count, &off, data, len); \
- kvfree(data); \
- return len; \
+} \
\+static ssize_t name##_list_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); \
- int len; \
- void *data; \
- data = kvmalloc(BUFF_SIZE, GFP_KERNEL); \
- if (!data) \
return -ENOMEM; \- len = scnprintf(data, BUFF_SIZE, "%*pbl\n", nr_cpu_ids, \
cpumask_bits(topology_##mask(dev->id))); \List could be more than BUFF_SIZE. In the worst case, For example, a system has 4096 cpus, the list could be:
0,1,3,5,7,9,---------4095 2,4,6,8,10,12.........4094
In most case, it could be simply like: 0-99, 100-199....
Just use the approach in kvasprintf(). Call, scnprintf(NULL,.... Which will helpfully tell you how big the buffer needs to be, allocate the buffer then print into it. That way, no need to be clever working out the size.
J
- len = memory_read_from_buffer(buf, count, &off, data, len); \
- kvfree(data); \
- return len; \
}
-#define define_siblings_show_list(name, mask) \ -static ssize_t name##_list_show(struct device *dev, \
struct device_attribute *attr, \char *buf) \-{ \
- return cpumap_print_to_pagebuf(true, buf, topology_##mask(dev->id));\
-}
-#define define_siblings_show_func(name, mask) \
- define_siblings_show_map(name, mask); \
- define_siblings_show_list(name, mask)
define_id_show_func(physical_package_id); static DEVICE_ATTR_RO(physical_package_id);
@@ -49,71 +68,83 @@ static DEVICE_ATTR_RO(die_id); 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 DEVICE_ATTR_RO(thread_siblings_list); +define_siblings_read_func(thread_siblings, sibling_cpumask); +static BIN_ATTR_RO(thread_siblings, 0); +static BIN_ATTR_RO(thread_siblings_list, 0);
+define_siblings_read_func(core_cpus, sibling_cpumask); +static BIN_ATTR_RO(core_cpus, 0); +static BIN_ATTR_RO(core_cpus_list, 0);
-define_siblings_show_func(core_cpus, sibling_cpumask); -static DEVICE_ATTR_RO(core_cpus); -static DEVICE_ATTR_RO(core_cpus_list); +define_siblings_read_func(core_siblings, core_cpumask); +static BIN_ATTR_RO(core_siblings, 0); +static BIN_ATTR_RO(core_siblings_list, 0);
-define_siblings_show_func(core_siblings, core_cpumask); -static DEVICE_ATTR_RO(core_siblings); -static DEVICE_ATTR_RO(core_siblings_list);
-define_siblings_show_func(die_cpus, die_cpumask); -static DEVICE_ATTR_RO(die_cpus); -static DEVICE_ATTR_RO(die_cpus_list); +define_siblings_read_func(die_cpus, die_cpumask); +static BIN_ATTR_RO(die_cpus, 0); +static BIN_ATTR_RO(die_cpus_list, 0);
+define_siblings_read_func(package_cpus, core_cpumask); +static BIN_ATTR_RO(package_cpus, 0); +static BIN_ATTR_RO(package_cpus_list, 0);
-define_siblings_show_func(package_cpus, core_cpumask); -static DEVICE_ATTR_RO(package_cpus); -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 DEVICE_ATTR_RO(book_siblings_list); +define_siblings_read_func(book_siblings, book_cpumask); +static BIN_ATTR_RO(book_siblings, 0); +static BIN_ATTR_RO(book_siblings_list, 0); #endif
#ifdef CONFIG_SCHED_DRAWER 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 DEVICE_ATTR_RO(drawer_siblings_list); +define_siblings_read_func(drawer_siblings, drawer_cpumask); +static BIN_ATTR_RO(drawer_siblings, 0); +static BIN_ATTR_RO(drawer_siblings_list, 0); +#endif
+static struct bin_attribute *bin_attrs[] = {
- &bin_attr_core_cpus,
- &bin_attr_core_cpus_list,
- &bin_attr_thread_siblings,
- &bin_attr_thread_siblings_list,
- &bin_attr_core_siblings,
- &bin_attr_core_siblings_list,
- &bin_attr_die_cpus,
- &bin_attr_die_cpus_list,
- &bin_attr_package_cpus,
- &bin_attr_package_cpus_list,
+#ifdef CONFIG_SCHED_BOOK
- &bin_attr_book_siblings,
- &bin_attr_book_siblings_list,
+#endif +#ifdef CONFIG_SCHED_DRAWER
- &bin_attr_drawer_siblings,
- &bin_attr_drawer_siblings_list,
#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 };
static const struct attribute_group topology_attr_group = { .attrs = default_attrs,
- .bin_attrs = bin_attrs, .name = "topology"
};
-- 2.7.4
Thanks Barry