> -----Original Message-----
> From: tiantao (H)
> Sent: Tuesday, April 20, 2021 11:49 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua(a)hisilicon.com>; Jonathan Cameron
> <jonathan.cameron(a)huawei.com>
> Cc: linuxarm(a)openeuler.org; tiantao (H) <tiantao6(a)hisilicon.com>
> Subject: [PATCH] lib: bitmap: check the overflow of pagebuf
>
> Move the detection of pagebuf overflow from node_read_cpumap to the
> more appropriate bitmap_print_to_pagebuf function. Other interfaces
> call bitmap_print_to_pagebuf to print without pagebuf overflow due
> to too many cpus.
This is far from a good commit log to describe the background.
>
> Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com>
> ---
> drivers/base/node.c | 3 ---
> lib/bitmap.c | 3 +++
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 2c36f61d..e24530c3 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -33,9 +33,6 @@ static ssize_t node_read_cpumap(struct device *dev, bool list,
> char *buf)
> cpumask_var_t mask;
> struct node *node_dev = to_node(dev);
>
> - /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
> - BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
> -
> if (!alloc_cpumask_var(&mask, GFP_KERNEL))
> return 0;
>
> diff --git a/lib/bitmap.c b/lib/bitmap.c
> index 74ceb02..170fc48 100644
> --- a/lib/bitmap.c
> +++ b/lib/bitmap.c
> @@ -482,6 +482,9 @@ int bitmap_print_to_pagebuf(bool list, char *buf, const
> unsigned long *maskp,
> {
> ptrdiff_t len = PAGE_SIZE - offset_in_page(buf);
>
> + /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
> + BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
> +
I don't think it is the right place. expecting cpumap_print_to_pagebuf.
BTW, it is not 2008/04/07 now, so just remove it.
> return list ? scnprintf(buf, len, "%*pbl\n", nmaskbits, maskp) :
> scnprintf(buf, len, "%*pb\n", nmaskbits, maskp);
> }
> --
> 2.7.4
I would prefer we send two patches as a series
"clarify and cleanup CPU and NUMA topology ABIs" with a cover
letter with the below as 1/2. 2/2 would be the
patch moving the place of cpu topology ABI doc.
From b32c0c00a187d4fe4c49d54d30650b0cacb2c351 Mon Sep 17 00:00:00 2001
From: Tian Tao <tiantao6(a)hisilicon.com>
Date: Wed, 21 Apr 2021 14:36:11 +1200
Subject: [PATCH 1/2] CPU, NUMA topology ABIs: clarify the overflow issue of sysfs
pagebuf
Both numa node and cpu use cpu bitmap like 3,ffffffff to expose hardware
topology. When cpu number is large, the page buffer of sysfs will over-
flow. This doesn't really happen nowadays as the maximum NR_CPUS is 8196
for X86_64 and 4096 for ARM64 since 8196 * 9 / 32 = 2305 which is still
smaller than 4KB page size.
So this BUILD_BUG_ON() is pretty much preventing future problems similar
with Y2K.
On the other hand, it should be more sensible to move the guard to common
code which can protect both cpu and numa:
/sys/devices/system/cpu/cpu0/topology/die_cpus etc.
/sys/devices/system/node/node0/cpumap etc.
Topology bitmaps shouldn't be larger than PAGE_SIZE as lstopo and numactl
depend on them. But other ABIs exposing cpu lists are not really used by
common applications, so this patch also marks those lists could be
trimmed.
Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com>
Signed-off-by: Barry Song <song.bao.hua(a)hisilicon.com>
---
Documentation/ABI/stable/sysfs-devices-node | 5 ++++-
Documentation/admin-guide/cputopology.rst | 15 +++++++++++++++
drivers/base/node.c | 3 ---
include/linux/cpumask.h | 6 ++++++
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-devices-node b/Documentation/ABI/stable/sysfs-devices-node
index 484fc04bcc25..9832a17b2b15 100644
--- a/Documentation/ABI/stable/sysfs-devices-node
+++ b/Documentation/ABI/stable/sysfs-devices-node
@@ -47,7 +47,10 @@ What: /sys/devices/system/node/nodeX/cpulist
Date: October 2002
Contact: Linux Memory Management list <linux-mm(a)kvack.org>
Description:
- The CPUs associated to the node.
+ The CPUs associated to the node. The format is like 0-3,
+ 8-11, 12-13. The maximum size is PAGE_SIZE, so the tail
+ of the string will be trimmed while its size is larger
+ than PAGE_SIZE.
What: /sys/devices/system/node/nodeX/meminfo
Date: October 2002
diff --git a/Documentation/admin-guide/cputopology.rst b/Documentation/admin-guide/cputopology.rst
index b90dafcc8237..8fac776a5ffa 100644
--- a/Documentation/admin-guide/cputopology.rst
+++ b/Documentation/admin-guide/cputopology.rst
@@ -44,6 +44,9 @@ core_cpus:
core_cpus_list:
human-readable list of CPUs within the same core.
+ The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE,
+ so the tail of the string will be trimmed while its size is larger
+ than PAGE_SIZE.
(deprecated name: "thread_siblings_list");
package_cpus:
@@ -54,6 +57,9 @@ package_cpus:
package_cpus_list:
human-readable list of CPUs sharing the same physical_package_id.
+ The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE,
+ so the tail of the string will be trimmed while its size is larger
+ than PAGE_SIZE.
(deprecated name: "core_siblings_list")
die_cpus:
@@ -63,6 +69,9 @@ die_cpus:
die_cpus_list:
human-readable list of CPUs within the same die.
+ The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE,
+ so the tail of the string will be trimmed while its size is larger
+ than PAGE_SIZE.
book_siblings:
@@ -73,6 +82,9 @@ book_siblings_list:
human-readable list of cpuX's hardware threads within the same
book_id.
+ The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE,
+ so the tail of the string will be trimmed while its size is larger
+ than PAGE_SIZE.
drawer_siblings:
@@ -83,6 +95,9 @@ drawer_siblings_list:
human-readable list of cpuX's hardware threads within the same
drawer_id.
+ The format is like 0-3, 8-11, 12-13. The maximum size is PAGE_SIZE,
+ so the tail of the string will be trimmed while its size is larger
+ than PAGE_SIZE.
Architecture-neutral, drivers/base/topology.c, exports these attributes.
However, the book and drawer related sysfs files will only be created if
diff --git a/drivers/base/node.c b/drivers/base/node.c
index f449dbb2c746..50324d06bcd5 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -33,9 +33,6 @@ static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
cpumask_var_t mask;
struct node *node_dev = to_node(dev);
- /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
- BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
-
if (!alloc_cpumask_var(&mask, GFP_KERNEL))
return 0;
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 383684e30f12..81f145e0c742 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -12,6 +12,7 @@
#include <linux/bitmap.h>
#include <linux/atomic.h>
#include <linux/bug.h>
+#include <asm/page.h>
/* Don't assign or return these: may not be this big! */
typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
@@ -924,6 +925,11 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
static inline ssize_t
cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
{
+ /*
+ * 32bits requires 9bytes: "ff,ffffffff", thus, too many CPUs will
+ * cause the overflow of sysfs pagebuf
+ */
+ BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
nr_cpu_ids);
}
--
2.25.1
Thanks
Barry