[PATCH v9 0/5] use bin_attribute to break the size limitation of cpumap ABI

[PATCH v9 0/5] use bin_attribute to break the size limitation of cpumap ABI

[Barry Song]

V9:
  - Split bitmask and list APIs and removed bool parameter with respect to
    Greg's comment
  - Removed duplication in code doc

v8:
  - add Reviewed-by tags of Jonathan Cameron which I missed in v7;
  - add Reviewed-by tags of Andy Shevchenko
  - add Yury's patch extending comment in this series

v7:
  - update doc in code for new APIs according to the comments of
    Andy Shevchenko;
  - other minor cleanup and commit log fix according to the comments
    of Andy Shevchenko

v6:
  -minor cleanup according to Andy Shevchenko's comment;
  -take bitmap_print_to_buf back according to Yury Norov's comment and
   fix the documents; and also take the bitmap testcase back.
  -Sorry, Yury, I don't think it is doable to move memory allocation
   to drivers.
   Considering a driver like topology.c, we have M CPUs and each CPU
   has N various nodes like core_siblings, package_cpus, die_cpus etc,
   we can't know the size of each node of each CPU in advance. The best
   time to get the size of each node is really when users read the sysfs
   node. otherwise, we have to scan M*N nodes in drivers in advance to
   figure out the exact size of buffers we need.
   On the other hand, it is crazily tricky to ask a bundle of drivers to
   find a proper place to save the pointer of allocated buffers so that
   they can be re-used in second read of the same bin_attribute node.
   And I doubt it is really useful to save the address of buffers
   somewhere. Immediately freeing it seems to be a correct option to
   avoid runtime waste of memory. We can't predict when users will read
   topology ABI and which node users will read.
   Finally, reading topology things wouldn't be the really cpu-bound
   things in user applications, hardly this kind of ABI things can be
   a performance bottleneck. Users use numactl and lstopo commands to
   read ABIs but nobody will do it again and again. And a normal
   application won't do topology repeatly. So the overhead caused by
   malloc/free in the new bitmap API doesn't really matter.
   if we really want a place to re-used the buffer and avoid malloc/free,
   it seems this should be done in some common place rather than each
   driver. still it is hard to find the best place.

   Thanks for the comments of Yury and Andy in v5.

v5:
  -remove the bitmap API bitmap_print_to_buf, alternatively, only provide
   cpumap_print_to_buf API as what we really care about is cpumask for
   this moment. we can freely take bitmap_print_to_buf back once we find
   the second user.
   hopefully this can alleviate Yury's worries on possible abuse of a new
   bitmap API.
  -correct the document of cpumap_print_to_buf;
  -In patch1, clearly explain why we need this new API in commit log;
  -Also refine the commit log of patch 2 and 3;
  -As the modification is narrowed to the scope of cpumask, the kunit
   test of bitmap_print_to_buf doesn't apply in the new patchset. so
   test case patch4/4 is removed.

   Thanks for the comments of Greg, Yury, Andy. Thanks for Jonathan's
   review.

v4:
  -add test cases for bitmap_print_to_buf API;
  -add Reviewed-by of Jonathan Cameron for patches 1-3, thanks!

v3:
  -fixed the strlen issue and patch #1,#2,#3 minor formatting issues, thanks
   to Andy Shevchenko and Jonathan Cameron.

v2:
  -split the original patch #1 into two patches and use kasprintf() in
  -patch #1 to simplify the code. do some minor formatting adjustments.

Background:

the whole story began from this thread when Jonatah and me tried to add a
new topology level-cluster which exists on kunpeng920 and X86 Jacobsville:
https://lore.kernel.org/lkml/YFRGIedW1fUlnmi+@kroah.com/
https://lore.kernel.org/lkml/YFR2kwakbcGiI37w@kroah.com/

in the discussion, Greg had some concern about the potential one page size
limitation of sysfs ABI for topology. Greg's comment is reasonable
and I think we should address the problem.

For this moment, numa node, cpu topology and some other drivers are using
cpu bitmap and list to expose hardware topology. When cpu number is large,
the page buffer of sysfs won't be able to hold the whole bitmask or list.
This doesn't really happen nowadays for bitmask as the maximum NR_CPUS
is 8196 for X86_64 and 4096 for ARM64 since
8196 * 9 / 32 = 2305
is still smaller than 4KB page size.

So the existing BUILD_BUG_ON() in drivers/base/node.c is pretty much
preventing future problems when hardware gets more and more CPUs:
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));
}

But those ABIs exposing cpu lists are much more tricky as a list could be
like: 0, 3, 5, 7, 9, 11... etc. so nobody knows the size till the last
moment. Comparing to bitmask, list is easier to exceed one page.

In the previous discussion, Greg and Dave Hansen preferred to remove this
kind of limitation totally and remove the BUILD_BUG_ON() in
drivers/base/node.c together:
https://lore.kernel.org/lkml/1619679819-45256-2-git-send-email-tiantao6@hisilicon.com/
https://lore.kernel.org/lkml/YIueOR4fOYa1dSAb@kroah.com/

Todo:

right now, only topology and node are addressed. there are many other
drivers are calling cpumap_print_to_pagebuf() and have the similar
problems. we are going to address them one by one after this patchset
settles down.

Barry Song (1):
  lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

Tian Tao (3):
  cpumask: introduce cpumap_print_list/bitmask_to_buf to support large
    bitmask and list
  topology: use bin_attribute to break the size limitation of cpumap ABI
  drivers/base/node.c: use bin_attribute to break the size limitation of
    cpumap ABI

Yury Norov (1):
  bitmap: extend comment to bitmap_print_bitmask/list_to_buf

 drivers/base/node.c     |  63 +++++++++++------
 drivers/base/topology.c | 115 ++++++++++++++++--------------
 include/linux/bitmap.h  |   6 ++
 include/linux/cpumask.h |  38 ++++++++++
 lib/bitmap.c            | 121 ++++++++++++++++++++++++++++++++
 lib/test_bitmap.c       | 150 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 418 insertions(+), 75 deletions(-)

-- 
2.25.1

[PATCH v9 1/5] cpumask: introduce cpumap_print_list/bitmask_to_buf to support large bitmask and list

[Barry Song]

From: Tian Tao <tiantao6@hisilicon.com>

The existing cpumap_print_to_pagebuf() is used by cpu topology and other
drivers to export hexadecimal bitmask and decimal list to userspace by
sysfs ABI.

Right now, those drivers are using a normal attribute for this kind of
ABIs. A normal attribute typically has show entry as below:

static ssize_t example_dev_show(struct device *dev,
                struct device_attribute *attr, char *buf)
{
	...
	return cpumap_print_to_pagebuf(true, buf, &pmu_mmdc->cpu);
}
show entry of attribute has no offset and count parameters and this
means the file is limited to one page only.

cpumap_print_to_pagebuf() API works terribly well for this kind of
normal attribute with buf parameter and without offset, count:

static inline ssize_t
cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
{
	return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
				       nr_cpu_ids);
}

The problem is once we have many cpus, we have a chance to make bitmask
or list more than one page. Especially for list, it could be as complex
as 0,3,5,7,9,...... We have no simple way to know it exact size.

It turns out bin_attribute is a way to break this limit. bin_attribute
has show entry as below:
static ssize_t
example_bin_attribute_show(struct file *filp, struct kobject *kobj,
             struct bin_attribute *attr, char *buf,
             loff_t offset, size_t count)
{
	...
}

With the new offset and count parameters, this makes sysfs ABI be able
to support file size more than one page. For example, offset could be
>= 4096.

This patch introduces cpumap_print_bitmask/list_to_buf() and their bitmap
infrastructure bitmap_print_bitmask/list_to_buf() so that those drivers
can move to bin_attribute to support large bitmask and list. At the same
time, we have to pass those corresponding parameters such as offset, count
from bin_attribute to this new API.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Stefano Brivio <sbrivio@redhat.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: "Ma, Jianpeng" <jianpeng.ma@intel.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 include/linux/bitmap.h  |   6 +++
 include/linux/cpumask.h |  38 +++++++++++++++
 lib/bitmap.c            | 103 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 147 insertions(+)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index a36cfcec4e77..37f36dad18bd 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -227,6 +227,12 @@ unsigned int bitmap_ord_to_pos(const unsigned long *bitmap, unsigned int ord, un
 int bitmap_print_to_pagebuf(bool list, char *buf,
 				   const unsigned long *maskp, int nmaskbits);
 
+extern int bitmap_print_bitmask_to_buf(char *buf, const unsigned long *maskp,
+				      int nmaskbits, loff_t off, size_t count);
+
+extern int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp,
+				      int nmaskbits, loff_t off, size_t count);
+
 #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
 #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
 
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index f3689a52bfd0..5d4d07a9e1ed 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -983,6 +983,44 @@ cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
 				      nr_cpu_ids);
 }
 
+/**
+ * cpumap_print_bitmask_to_buf  - copies the cpumask into the buffer as
+ *	hex values of cpumask
+ *
+ * @buf: the buffer to copy into
+ * @mask: the cpumask to copy
+ * @off: in the string from which we are copying, we copy to @buf
+ * @count: the maximum number of bytes to print
+ *
+ * The function prints the cpumask into the buffer as hex values of
+ * cpumask; Typically used by bin_attribute to export cpumask bitmask
+ * ABI.
+ *
+ * Returns the length of how many bytes have been copied.
+ */
+static inline ssize_t
+cpumap_print_bitmask_to_buf(char *buf, const struct cpumask *mask,
+		loff_t off, size_t count)
+{
+	return bitmap_print_bitmask_to_buf(buf, cpumask_bits(mask),
+				   nr_cpu_ids, off, count);
+}
+
+/**
+ * cpumap_print_list_to_buf  - copies the cpumask into the buffer as
+ *	comma-separated list of cpus
+ *
+ * Everything is same with the above cpumap_print_bitmask_to_buf()
+ * except the print format.
+ */
+static inline ssize_t
+cpumap_print_list_to_buf(char *buf, const struct cpumask *mask,
+		loff_t off, size_t count)
+{
+	return bitmap_print_list_to_buf(buf, cpumask_bits(mask),
+				   nr_cpu_ids, off, count);
+}
+
 #if NR_CPUS <= BITS_PER_LONG
 #define CPU_MASK_ALL							\
 (cpumask_t) { {								\
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 9401d39e4722..73746d96af81 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -487,6 +487,109 @@ int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp,
 }
 EXPORT_SYMBOL(bitmap_print_to_pagebuf);
 
+/**
+ * bitmap_print_to_buf  - convert bitmap to list or hex format ASCII string
+ * @list: indicates whether the bitmap must be list
+ *      true:  print in decimal list format
+ *      false: print in hexadecimal bitmask format
+ */
+static int bitmap_print_to_buf(bool list, char *buf, const unsigned long *maskp,
+		int nmaskbits, loff_t off, size_t count)
+{
+	const char *fmt = list ? "%*pbl\n" : "%*pb\n";
+	ssize_t size;
+	void *data;
+
+	data = kasprintf(GFP_KERNEL, fmt, nmaskbits, maskp);
+	if (!data)
+		return -ENOMEM;
+
+	size = memory_read_from_buffer(buf, count, &off, data, strlen(data) + 1);
+	kfree(data);
+
+	return size;
+}
+
+/**
+ * bitmap_print_bitmask_to_buf  - convert bitmap to hex bitmask format ASCII string
+ *
+ * The bitmap_print_to_pagebuf() is used indirectly via its cpumap wrapper
+ * cpumap_print_to_pagebuf() or directly by drivers to export hexadecimal
+ * bitmask and decimal list to userspace by sysfs ABI.
+ * Drivers might be using a normal attribute for this kind of ABIs. A
+ * normal attribute typically has show entry as below:
+ * static ssize_t example_attribute_show(struct device *dev,
+ * 		struct device_attribute *attr, char *buf)
+ * {
+ * 	...
+ * 	return bitmap_print_to_pagebuf(true, buf, &mask, nr_trig_max);
+ * }
+ * show entry of attribute has no offset and count parameters and this
+ * means the file is limited to one page only.
+ * bitmap_print_to_pagebuf() API works terribly well for this kind of
+ * normal attribute with buf parameter and without offset, count:
+ * bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp,
+ * 			   int nmaskbits)
+ * {
+ * }
+ * The problem is once we have a large bitmap, we have a chance to get a
+ * bitmask or list more than one page. Especially for list, it could be
+ * as complex as 0,3,5,7,9,... We have no simple way to know it exact size.
+ * It turns out bin_attribute is a way to break this limit. bin_attribute
+ * has show entry as below:
+ * static ssize_t
+ * example_bin_attribute_show(struct file *filp, struct kobject *kobj,
+ * 		struct bin_attribute *attr, char *buf,
+ * 		loff_t offset, size_t count)
+ * {
+ * 	...
+ * }
+ * With the new offset and count parameters, this makes sysfs ABI be able
+ * to support file size more than one page. For example, offset could be
+ * >= 4096.
+ * bitmap_print_bitmask_to_buf(), bitmap_print_list_to_buf() wit their
+ * cpumap wrapper cpumap_print_bitmask_to_buf(), cpumap_print_list_to_buf()
+ * make those drivers be able to support large bitmask and list after they
+ * move to use bin_attribute. In result, we have to pass the corresponding
+ * parameters such as off, count from bin_attribute show entry to this API.
+ *
+ * @buf: buffer into which string is placed
+ * @maskp: pointer to bitmap to convert
+ * @nmaskbits: size of bitmap, in bits
+ * @off: in the string from which we are copying, We copy to @buf
+ * @count: the maximum number of bytes to print
+ *
+ * The role of cpumap_print_bitmask_to_buf() and cpumap_print_list_to_buf()
+ * is similar with cpumap_print_to_pagebuf(),  the difference is that
+ * bitmap_print_to_pagebuf() mainly serves sysfs attribute with the assumption
+ * the destination buffer is exactly one page and won't be more than one page.
+ * cpumap_print_bitmask_to_buf() and cpumap_print_list_to_buf(), on the other
+ * hand, mainly serves bin_attribute which doesn't work with exact one page,
+ * and it can break the size limit of converted decimal list and hexadecimal
+ * bitmask.
+ *
+ * Returns the number of characters actually printed to @buf
+ */
+int bitmap_print_bitmask_to_buf(char *buf, const unsigned long *maskp,
+				int nmaskbits, loff_t off, size_t count)
+{
+	return bitmap_print_to_buf(false, buf, maskp, nmaskbits, off, count);
+}
+EXPORT_SYMBOL(bitmap_print_bitmask_to_buf);
+
+/**
+ * bitmap_print_list_to_buf  - convert bitmap to decimal list format ASCII string
+ *
+ * Everything is same with the above bitmap_print_bitmask_to_buf() except
+ * the print format.
+ */
+int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp,
+			     int nmaskbits, loff_t off, size_t count)
+{
+	return bitmap_print_to_buf(true, buf, maskp, nmaskbits, off, count);
+}
+EXPORT_SYMBOL(bitmap_print_list_to_buf);
+
 /*
  * Region 9-38:4/10 describes the following bitmap structure:
  * 0	   9  12    18			38	     N
-- 
2.25.1

[PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[Barry Song]

The added test items cover both cases where bitmap buf of the printed
result is greater than and less than 4KB.
And it also covers the case where offset for printing is non-zero
which will happen when printed buf is larger than one page in
sysfs bin_attribute.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 lib/test_bitmap.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)

diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 4ea73f5aed41..d33fa5a61b95 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -19,6 +19,7 @@
 KSTM_MODULE_GLOBALS();
 
 static char pbl_buffer[PAGE_SIZE] __initdata;
+static char print_buf[PAGE_SIZE * 2] __initdata;
 
 static const unsigned long exp1[] __initconst = {
 	BITMAP_FROM_U64(1),
@@ -156,6 +157,20 @@ static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
 	return true;
 }
 
+static bool __init
+__check_eq_str(const char *srcfile, unsigned int line,
+		const char *exp_str, const char *str,
+		unsigned int len)
+{
+	bool eq;
+
+	eq = strncmp(exp_str, str, len) == 0;
+	if (!eq)
+		pr_err("[%s:%u] expected %s, got %s\n", srcfile, line, exp_str, str);
+
+	return eq;
+}
+
 #define __expect_eq(suffix, ...)					\
 	({								\
 		int result = 0;						\
@@ -173,6 +188,7 @@ static bool __init __check_eq_clump8(const char *srcfile, unsigned int line,
 #define expect_eq_pbl(...)		__expect_eq(pbl, ##__VA_ARGS__)
 #define expect_eq_u32_array(...)	__expect_eq(u32_array, ##__VA_ARGS__)
 #define expect_eq_clump8(...)		__expect_eq(clump8, ##__VA_ARGS__)
+#define expect_eq_str(...)		__expect_eq(str, ##__VA_ARGS__)
 
 static void __init test_zero_clear(void)
 {
@@ -660,6 +676,139 @@ static void __init test_bitmap_cut(void)
 	}
 }
 
+struct test_bitmap_print {
+	const unsigned long *bitmap;
+	unsigned long nbits;
+	const char *mask;
+	const char *list;
+};
+
+static const unsigned long small_bitmap[] __initconst = {
+	BITMAP_FROM_U64(0x3333333311111111ULL),
+};
+
+static const char small_mask[] __initconst = "33333333,11111111\n";
+static const char small_list[] __initconst = "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61\n";
+
+static const unsigned long large_bitmap[] __initconst = {
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+	BITMAP_FROM_U64(0x3333333311111111ULL), BITMAP_FROM_U64(0x3333333311111111ULL),
+};
+
+static const char large_mask[] __initconst = "33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111,"
+					"33333333,11111111,33333333,11111111\n";
+
+static const char large_list[] __initconst = /* more than 4KB */
+	"0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61,64,68,72,76,80,84,88,92,96-97,100-101,104-1"
+	"05,108-109,112-113,116-117,120-121,124-125,128,132,136,140,144,148,152,156,160-161,164-165,168-169,172-173,176-1"
+	"77,180-181,184-185,188-189,192,196,200,204,208,212,216,220,224-225,228-229,232-233,236-237,240-241,244-245,248-2"
+	"49,252-253,256,260,264,268,272,276,280,284,288-289,292-293,296-297,300-301,304-305,308-309,312-313,316-317,320,3"
+	"24,328,332,336,340,344,348,352-353,356-357,360-361,364-365,368-369,372-373,376-377,380-381,384,388,392,396,400,4"
+	"04,408,412,416-417,420-421,424-425,428-429,432-433,436-437,440-441,444-445,448,452,456,460,464,468,472,476,480-4"
+	"81,484-485,488-489,492-493,496-497,500-501,504-505,508-509,512,516,520,524,528,532,536,540,544-545,548-549,552-5"
+	"53,556-557,560-561,564-565,568-569,572-573,576,580,584,588,592,596,600,604,608-609,612-613,616-617,620-621,624-6"
+	"25,628-629,632-633,636-637,640,644,648,652,656,660,664,668,672-673,676-677,680-681,684-685,688-689,692-693,696-6"
+	"97,700-701,704,708,712,716,720,724,728,732,736-737,740-741,744-745,748-749,752-753,756-757,760-761,764-765,768,7"
+	"72,776,780,784,788,792,796,800-801,804-805,808-809,812-813,816-817,820-821,824-825,828-829,832,836,840,844,848,8"
+	"52,856,860,864-865,868-869,872-873,876-877,880-881,884-885,888-889,892-893,896,900,904,908,912,916,920,924,928-9"
+	"29,932-933,936-937,940-941,944-945,948-949,952-953,956-957,960,964,968,972,976,980,984,988,992-993,996-997,1000-"
+	"1001,1004-1005,1008-1009,1012-1013,1016-1017,1020-1021,1024,1028,1032,1036,1040,1044,1048,1052,1056-1057,1060-10"
+	"61,1064-1065,1068-1069,1072-1073,1076-1077,1080-1081,1084-1085,1088,1092,1096,1100,1104,1108,1112,1116,1120-1121"
+	",1124-1125,1128-1129,1132-1133,1136-1137,1140-1141,1144-1145,1148-1149,1152,1156,1160,1164,1168,1172,1176,1180,1"
+	"184-1185,1188-1189,1192-1193,1196-1197,1200-1201,1204-1205,1208-1209,1212-1213,1216,1220,1224,1228,1232,1236,124"
+	"0,1244,1248-1249,1252-1253,1256-1257,1260-1261,1264-1265,1268-1269,1272-1273,1276-1277,1280,1284,1288,1292,1296,"
+	"1300,1304,1308,1312-1313,1316-1317,1320-1321,1324-1325,1328-1329,1332-1333,1336-1337,1340-1341,1344,1348,1352,13"
+	"56,1360,1364,1368,1372,1376-1377,1380-1381,1384-1385,1388-1389,1392-1393,1396-1397,1400-1401,1404-1405,1408,1412"
+	",1416,1420,1424,1428,1432,1436,1440-1441,1444-1445,1448-1449,1452-1453,1456-1457,1460-1461,1464-1465,1468-1469,1"
+	"472,1476,1480,1484,1488,1492,1496,1500,1504-1505,1508-1509,1512-1513,1516-1517,1520-1521,1524-1525,1528-1529,153"
+	"2-1533,1536,1540,1544,1548,1552,1556,1560,1564,1568-1569,1572-1573,1576-1577,1580-1581,1584-1585,1588-1589,1592-"
+	"1593,1596-1597,1600,1604,1608,1612,1616,1620,1624,1628,1632-1633,1636-1637,1640-1641,1644-1645,1648-1649,1652-16"
+	"53,1656-1657,1660-1661,1664,1668,1672,1676,1680,1684,1688,1692,1696-1697,1700-1701,1704-1705,1708-1709,1712-1713"
+	",1716-1717,1720-1721,1724-1725,1728,1732,1736,1740,1744,1748,1752,1756,1760-1761,1764-1765,1768-1769,1772-1773,1"
+	"776-1777,1780-1781,1784-1785,1788-1789,1792,1796,1800,1804,1808,1812,1816,1820,1824-1825,1828-1829,1832-1833,183"
+	"6-1837,1840-1841,1844-1845,1848-1849,1852-1853,1856,1860,1864,1868,1872,1876,1880,1884,1888-1889,1892-1893,1896-"
+	"1897,1900-1901,1904-1905,1908-1909,1912-1913,1916-1917,1920,1924,1928,1932,1936,1940,1944,1948,1952-1953,1956-19"
+	"57,1960-1961,1964-1965,1968-1969,1972-1973,1976-1977,1980-1981,1984,1988,1992,1996,2000,2004,2008,2012,2016-2017"
+	",2020-2021,2024-2025,2028-2029,2032-2033,2036-2037,2040-2041,2044-2045,2048,2052,2056,2060,2064,2068,2072,2076,2"
+	"080-2081,2084-2085,2088-2089,2092-2093,2096-2097,2100-2101,2104-2105,2108-2109,2112,2116,2120,2124,2128,2132,213"
+	"6,2140,2144-2145,2148-2149,2152-2153,2156-2157,2160-2161,2164-2165,2168-2169,2172-2173,2176,2180,2184,2188,2192,"
+	"2196,2200,2204,2208-2209,2212-2213,2216-2217,2220-2221,2224-2225,2228-2229,2232-2233,2236-2237,2240,2244,2248,22"
+	"52,2256,2260,2264,2268,2272-2273,2276-2277,2280-2281,2284-2285,2288-2289,2292-2293,2296-2297,2300-2301,2304,2308"
+	",2312,2316,2320,2324,2328,2332,2336-2337,2340-2341,2344-2345,2348-2349,2352-2353,2356-2357,2360-2361,2364-2365,2"
+	"368,2372,2376,2380,2384,2388,2392,2396,2400-2401,2404-2405,2408-2409,2412-2413,2416-2417,2420-2421,2424-2425,242"
+	"8-2429,2432,2436,2440,2444,2448,2452,2456,2460,2464-2465,2468-2469,2472-2473,2476-2477,2480-2481,2484-2485,2488-"
+	"2489,2492-2493,2496,2500,2504,2508,2512,2516,2520,2524,2528-2529,2532-2533,2536-2537,2540-2541,2544-2545,2548-25"
+	"49,2552-2553,2556-2557\n";
+
+static const struct test_bitmap_print test_print[] __initconst = {
+	{ small_bitmap, sizeof(small_bitmap) * BITS_PER_BYTE, small_mask, small_list },
+	{ large_bitmap, sizeof(large_bitmap) * BITS_PER_BYTE, large_mask, large_list },
+};
+
+static void __init test_bitmap_print_buf(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(test_print); i++) {
+		const struct test_bitmap_print *t = &test_print[i];
+		int n;
+
+		n = bitmap_print_bitmask_to_buf(print_buf, t->bitmap, t->nbits,
+						0, 2 * PAGE_SIZE);
+		expect_eq_uint(strlen(t->mask) + 1, n);
+		expect_eq_str(t->mask, print_buf, n);
+
+		n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
+					     0, 2 * PAGE_SIZE);
+		expect_eq_uint(strlen(t->list) + 1, n);
+		expect_eq_str(t->list, print_buf, n);
+
+		/* test by non-zero offset */
+		if (strlen(t->list) > PAGE_SIZE) {
+			n = bitmap_print_list_to_buf(print_buf, t->bitmap, t->nbits,
+						     PAGE_SIZE, PAGE_SIZE);
+			expect_eq_uint(strlen(t->list) + 1 - PAGE_SIZE, n);
+			expect_eq_str(t->list + PAGE_SIZE, print_buf, n);
+		}
+	}
+}
+
 static void __init selftest(void)
 {
 	test_zero_clear();
@@ -672,6 +821,7 @@ static void __init selftest(void)
 	test_mem_optimisations();
 	test_for_each_set_clump8();
 	test_bitmap_cut();
+	test_bitmap_print_buf();
 }
 
 KSTM_MODULE_LOADERS(test_bitmap);
-- 
2.25.1

[PATCH v9 3/5] topology: use bin_attribute to break the size limitation of cpumap ABI

[Barry Song]

From: Tian Tao <tiantao6@hisilicon.com>

Reading /sys/devices/system/cpu/cpuX/topology/ returns cpu topology.
However, the size of this file is limited to PAGE_SIZE because of
the limitation for sysfs attribute.
This patch moves to use bin_attribute to extend the ABI to be more
than one page so that cpumap bitmask and list won't be potentially
trimmed.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 drivers/base/topology.c | 115 ++++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 52 deletions(-)

diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 4d254fcc93d1..43c0940643f5 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -21,25 +21,27 @@ 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);                                 \
+										\
+	return cpumap_print_bitmask_to_buf(buf, topology_##mask(dev->id),	\
+					   off, count);                         \
+}										\
+										\
+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);					\
+										\
+	return cpumap_print_list_to_buf(buf, topology_##mask(dev->id),		\
+					off, count);				\
 }
 
-#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 +51,80 @@ 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_show_func(core_cpus, sibling_cpumask);
-static DEVICE_ATTR_RO(core_cpus);
-static DEVICE_ATTR_RO(core_cpus_list);
+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_siblings, core_cpumask);
-static DEVICE_ATTR_RO(core_siblings);
-static DEVICE_ATTR_RO(core_siblings_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(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_show_func(package_cpus, core_cpumask);
-static DEVICE_ATTR_RO(package_cpus);
-static DEVICE_ATTR_RO(package_cpus_list);
+define_siblings_read_func(package_cpus, core_cpumask);
+static BIN_ATTR_RO(package_cpus, 0);
+static BIN_ATTR_RO(package_cpus_list, 0);
 
 #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.25.1

[PATCH v9 4/5] drivers/base/node.c: use bin_attribute to break the size limitation of cpumap ABI

[Barry Song]

From: Tian Tao <tiantao6@hisilicon.com>

Reading /sys/devices/system/cpu/cpuX/nodeX/ returns cpumap and cpulist.
However, the size of this file is limited to PAGE_SIZE because of the
limitation for sysfs attribute.

This patch moves to use bin_attribute to extend the ABI to be more
than one page so that cpumap bitmask and list won't be potentially
trimmed.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 drivers/base/node.c | 63 ++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 4a4ae868ad9f..70a84e85ad3f 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -26,43 +26,47 @@ static struct bus_type node_subsys = {
 	.dev_name = "node",
 };
 
-
-static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
+static inline ssize_t cpumap_read(struct file *file, struct kobject *kobj,
+				  struct bin_attribute *attr, char *buf,
+				  loff_t off, size_t count)
 {
-	ssize_t n;
-	cpumask_var_t mask;
+	struct device *dev = kobj_to_dev(kobj);
 	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));
+	cpumask_var_t mask;
+	ssize_t n;
 
 	if (!alloc_cpumask_var(&mask, GFP_KERNEL))
 		return 0;
 
 	cpumask_and(mask, cpumask_of_node(node_dev->dev.id), cpu_online_mask);
-	n = cpumap_print_to_pagebuf(list, buf, mask);
+	n = cpumap_print_bitmask_to_buf(buf, mask, off, count);
 	free_cpumask_var(mask);
 
 	return n;
 }
 
-static inline ssize_t cpumap_show(struct device *dev,
-				  struct device_attribute *attr,
-				  char *buf)
+static BIN_ATTR_RO(cpumap, 0);
+
+static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
+				   struct bin_attribute *attr, char *buf,
+				   loff_t off, size_t count)
 {
-	return node_read_cpumap(dev, false, buf);
-}
+	struct device *dev = kobj_to_dev(kobj);
+	struct node *node_dev = to_node(dev);
+	cpumask_var_t mask;
+	ssize_t n;
+
+	if (!alloc_cpumask_var(&mask, GFP_KERNEL))
+		return 0;
 
-static DEVICE_ATTR_RO(cpumap);
+	cpumask_and(mask, cpumask_of_node(node_dev->dev.id), cpu_online_mask);
+	n = cpumap_print_list_to_buf(buf, mask, off, count);
+	free_cpumask_var(mask);
 
-static inline ssize_t cpulist_show(struct device *dev,
-				   struct device_attribute *attr,
-				   char *buf)
-{
-	return node_read_cpumap(dev, true, buf);
+	return n;
 }
 
-static DEVICE_ATTR_RO(cpulist);
+static BIN_ATTR_RO(cpulist, 0);
 
 /**
  * struct node_access_nodes - Access class device to hold user visible
@@ -557,15 +561,28 @@ static ssize_t node_read_distance(struct device *dev,
 static DEVICE_ATTR(distance, 0444, node_read_distance, NULL);
 
 static struct attribute *node_dev_attrs[] = {
-	&dev_attr_cpumap.attr,
-	&dev_attr_cpulist.attr,
 	&dev_attr_meminfo.attr,
 	&dev_attr_numastat.attr,
 	&dev_attr_distance.attr,
 	&dev_attr_vmstat.attr,
 	NULL
 };
-ATTRIBUTE_GROUPS(node_dev);
+
+static struct bin_attribute *node_dev_bin_attrs[] = {
+	&bin_attr_cpumap,
+	&bin_attr_cpulist,
+	NULL
+};
+
+static const struct attribute_group node_dev_group = {
+	.attrs = node_dev_attrs,
+	.bin_attrs = node_dev_bin_attrs
+};
+
+static const struct attribute_group *node_dev_groups[] = {
+	&node_dev_group,
+	NULL
+};
 
 #ifdef CONFIG_HUGETLBFS
 /*
-- 
2.25.1

[PATCH v9 5/5] bitmap: extend comment to bitmap_print_bitmask/list_to_buf

[Barry Song]

From: Yury Norov <yury.norov@gmail.com>

Extend comment to new function to warn potential users about caveats.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
 lib/bitmap.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index 73746d96af81..663dd81967d4 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -568,6 +568,24 @@ static int bitmap_print_to_buf(bool list, char *buf, const unsigned long *maskp,
  * and it can break the size limit of converted decimal list and hexadecimal
  * bitmask.
  *
+ * WARNING!
+ *
+ * This function is not a replacement for sprintf() or bitmap_print_to_pagebuf().
+ * It is intended to workaround sysfs limitations discussed above and should be
+ * used carefully in general case for the following reasons:
+ *  - Time complexity is O(nbits^2/count), comparing to O(nbits) for snprintf().
+ *  - Memory complexity is O(nbits), comparing to O(1) for snprintf().
+ *  - @off and @count are NOT offset and number of bits to print.
+ *  - If printing part of bitmap as list, the resulting string is not a correct
+ *    list representation of bitmap. Particularly, some bits within or out of
+ *    related interval may be erroneously set or unset. The format of the string
+ *    may be broken, so bitmap_parselist-like parser may fail parsing it.
+ *  - If printing the whole bitmap as list by parts, user must ensure the order
+ *    of calls of the function such that the offset is incremented linearly.
+ *  - If printing the whole bitmap as list by parts, user must keep bitmap
+ *    unchanged between the very first and very last call. Otherwise concatenated
+ *    result may be incorrect, and format may be broken.
+ *
  * Returns the number of characters actually printed to @buf
  */
 int bitmap_print_bitmask_to_buf(char *buf, const unsigned long *maskp,
-- 
2.25.1

Re: [PATCH v9 1/5] cpumask: introduce cpumap_print_list/bitmask_to_buf to support large bitmask and list

[Greg KH]

On Fri, Aug 06, 2021 at 11:02:47PM +1200, Barry Song wrote:
> From: Tian Tao <tiantao6@hisilicon.com>
> 
> The existing cpumap_print_to_pagebuf() is used by cpu topology and other
> drivers to export hexadecimal bitmask and decimal list to userspace by
> sysfs ABI.
> 
> Right now, those drivers are using a normal attribute for this kind of
> ABIs. A normal attribute typically has show entry as below:
> 
> static ssize_t example_dev_show(struct device *dev,
>                 struct device_attribute *attr, char *buf)
> {
> 	...
> 	return cpumap_print_to_pagebuf(true, buf, &pmu_mmdc->cpu);
> }
> show entry of attribute has no offset and count parameters and this
> means the file is limited to one page only.
> 
> cpumap_print_to_pagebuf() API works terribly well for this kind of
> normal attribute with buf parameter and without offset, count:
> 
> static inline ssize_t
> cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
> {
> 	return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
> 				       nr_cpu_ids);
> }
> 
> The problem is once we have many cpus, we have a chance to make bitmask
> or list more than one page. Especially for list, it could be as complex
> as 0,3,5,7,9,...... We have no simple way to know it exact size.
> 
> It turns out bin_attribute is a way to break this limit. bin_attribute
> has show entry as below:
> static ssize_t
> example_bin_attribute_show(struct file *filp, struct kobject *kobj,
>              struct bin_attribute *attr, char *buf,
>              loff_t offset, size_t count)
> {
> 	...
> }
> 
> With the new offset and count parameters, this makes sysfs ABI be able
> to support file size more than one page. For example, offset could be
> >= 4096.
> 
> This patch introduces cpumap_print_bitmask/list_to_buf() and their bitmap
> infrastructure bitmap_print_bitmask/list_to_buf() so that those drivers
> can move to bin_attribute to support large bitmask and list. At the same
> time, we have to pass those corresponding parameters such as offset, count
> from bin_attribute to this new API.
> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Stefano Brivio <sbrivio@redhat.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: "Ma, Jianpeng" <jianpeng.ma@intel.com>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
>  include/linux/bitmap.h  |   6 +++
>  include/linux/cpumask.h |  38 +++++++++++++++
>  lib/bitmap.c            | 103 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 147 insertions(+)
> 
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index a36cfcec4e77..37f36dad18bd 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -227,6 +227,12 @@ unsigned int bitmap_ord_to_pos(const unsigned long *bitmap, unsigned int ord, un
>  int bitmap_print_to_pagebuf(bool list, char *buf,
>  				   const unsigned long *maskp, int nmaskbits);
>  
> +extern int bitmap_print_bitmask_to_buf(char *buf, const unsigned long *maskp,
> +				      int nmaskbits, loff_t off, size_t count);
> +
> +extern int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp,
> +				      int nmaskbits, loff_t off, size_t count);
> +

Why are you adding bitmap_print_list_to_buf() when no one uses it in
this patch series?

Did I miss it somewhere?

thanks,

greg k-h

Re: [PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

Hi Barry,

I love your patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on staging/staging-testing linus/master v5.14-rc4 next-20210805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 112cedc8e600b668688eb809bf11817adec58ddc
config: xtensa-randconfig-r011-20210805 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
        git checkout cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> WARNING: modpost: lib/test_bitmap.o(.text.unlikely+0x58): Section mismatch in reference from the function bitmap_equal() to the variable .init.data:print_buf
The function bitmap_equal() references
the variable __initdata print_buf.
This is often because bitmap_equal lacks a __initdata
annotation or the annotation of print_buf is wrong.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33794 bytes --]

Re: [PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2147 bytes --]

Hi Barry,

I love your patch! Perhaps something to improve:

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on staging/staging-testing linus/master v5.14-rc4 next-20210805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 112cedc8e600b668688eb809bf11817adec58ddc
config: xtensa-randconfig-r011-20210805 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
        git checkout cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> WARNING: modpost: lib/test_bitmap.o(.text.unlikely+0x58): Section mismatch in reference from the function bitmap_equal() to the variable .init.data:print_buf
The function bitmap_equal() references
the variable __initdata print_buf.
This is often because bitmap_equal lacks a __initdata
annotation or the annotation of print_buf is wrong.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33794 bytes --]

Re: [PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[Andy Shevchenko]

On Sat, Aug 07, 2021 at 01:51:36AM +0800, kernel test robot wrote:
> Hi Barry,
> 
> I love your patch! Perhaps something to improve:

I recall that I saw it ~1 year ago and informed Max about it.
Don't remember what was the outcome, though.

> [auto build test WARNING on driver-core/driver-core-testing]
> [also build test WARNING on staging/staging-testing linus/master v5.14-rc4 next-20210805]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 112cedc8e600b668688eb809bf11817adec58ddc
> config: xtensa-randconfig-r011-20210805 (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 10.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
>         git checkout cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
>         # save the attached .config to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> WARNING: modpost: lib/test_bitmap.o(.text.unlikely+0x58): Section mismatch in reference from the function bitmap_equal() to the variable .init.data:print_buf
> The function bitmap_equal() references
> the variable __initdata print_buf.
> This is often because bitmap_equal lacks a __initdata
> annotation or the annotation of print_buf is wrong.
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[Andy Shevchenko]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]

On Sat, Aug 07, 2021 at 01:51:36AM +0800, kernel test robot wrote:
> Hi Barry,
> 
> I love your patch! Perhaps something to improve:

I recall that I saw it ~1 year ago and informed Max about it.
Don't remember what was the outcome, though.

> [auto build test WARNING on driver-core/driver-core-testing]
> [also build test WARNING on staging/staging-testing linus/master v5.14-rc4 next-20210805]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 112cedc8e600b668688eb809bf11817adec58ddc
> config: xtensa-randconfig-r011-20210805 (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 10.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Barry-Song/use-bin_attribute-to-break-the-size-limitation-of-cpumap-ABI/20210806-190735
>         git checkout cc67b898a0f6f1a93163c0c296cfa77b1f04f72d
>         # save the attached .config to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> WARNING: modpost: lib/test_bitmap.o(.text.unlikely+0x58): Section mismatch in reference from the function bitmap_equal() to the variable .init.data:print_buf
> The function bitmap_equal() references
> the variable __initdata print_buf.
> This is often because bitmap_equal lacks a __initdata
> annotation or the annotation of print_buf is wrong.
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org



-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v9 1/5] cpumask: introduce cpumap_print_list/bitmask_to_buf to support large bitmask and list

[Song Bao Hua (Barry Song)]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Saturday, August 7, 2021 1:22 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: andriy.shevchenko@linux.intel.com; yury.norov@gmail.com;
> linux-kernel@vger.kernel.org; akpm@linux-foundation.org;
> dave.hansen@intel.com; linux@rasmusvillemoes.dk; rafael@kernel.org;
> rdunlap@infradead.org; agordeev@linux.ibm.com; sbrivio@redhat.com;
> jianpeng.ma@intel.com; valentin.schneider@arm.com; peterz@infradead.org;
> bristot@redhat.com; guodong.xu@linaro.org; tangchengchang
> <tangchengchang@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
> yangyicong <yangyicong@huawei.com>; tim.c.chen@linux.intel.com; Linuxarm
> <linuxarm@huawei.com>; tiantao (H) <tiantao6@hisilicon.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: Re: [PATCH v9 1/5] cpumask: introduce
> cpumap_print_list/bitmask_to_buf to support large bitmask and list
> 
> On Fri, Aug 06, 2021 at 11:02:47PM +1200, Barry Song wrote:
> > From: Tian Tao <tiantao6@hisilicon.com>
> >
> > The existing cpumap_print_to_pagebuf() is used by cpu topology and other
> > drivers to export hexadecimal bitmask and decimal list to userspace by
> > sysfs ABI.
> >
> > Right now, those drivers are using a normal attribute for this kind of
> > ABIs. A normal attribute typically has show entry as below:
> >
> > static ssize_t example_dev_show(struct device *dev,
> >                 struct device_attribute *attr, char *buf)
> > {
> > 	...
> > 	return cpumap_print_to_pagebuf(true, buf, &pmu_mmdc->cpu);
> > }
> > show entry of attribute has no offset and count parameters and this
> > means the file is limited to one page only.
> >
> > cpumap_print_to_pagebuf() API works terribly well for this kind of
> > normal attribute with buf parameter and without offset, count:
> >
> > static inline ssize_t
> > cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
> > {
> > 	return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
> > 				       nr_cpu_ids);
> > }
> >
> > The problem is once we have many cpus, we have a chance to make bitmask
> > or list more than one page. Especially for list, it could be as complex
> > as 0,3,5,7,9,...... We have no simple way to know it exact size.
> >
> > It turns out bin_attribute is a way to break this limit. bin_attribute
> > has show entry as below:
> > static ssize_t
> > example_bin_attribute_show(struct file *filp, struct kobject *kobj,
> >              struct bin_attribute *attr, char *buf,
> >              loff_t offset, size_t count)
> > {
> > 	...
> > }
> >
> > With the new offset and count parameters, this makes sysfs ABI be able
> > to support file size more than one page. For example, offset could be
> > >= 4096.
> >
> > This patch introduces cpumap_print_bitmask/list_to_buf() and their bitmap
> > infrastructure bitmap_print_bitmask/list_to_buf() so that those drivers
> > can move to bin_attribute to support large bitmask and list. At the same
> > time, we have to pass those corresponding parameters such as offset, count
> > from bin_attribute to this new API.
> >
> > Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Randy Dunlap <rdunlap@infradead.org>
> > Cc: Stefano Brivio <sbrivio@redhat.com>
> > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > Cc: "Ma, Jianpeng" <jianpeng.ma@intel.com>
> > Cc: Yury Norov <yury.norov@gmail.com>
> > Cc: Valentin Schneider <valentin.schneider@arm.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> > ---
> >  include/linux/bitmap.h  |   6 +++
> >  include/linux/cpumask.h |  38 +++++++++++++++
> >  lib/bitmap.c            | 103 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 147 insertions(+)
> >
> > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> > index a36cfcec4e77..37f36dad18bd 100644
> > --- a/include/linux/bitmap.h
> > +++ b/include/linux/bitmap.h
> > @@ -227,6 +227,12 @@ unsigned int bitmap_ord_to_pos(const unsigned long
> *bitmap, unsigned int ord, un
> >  int bitmap_print_to_pagebuf(bool list, char *buf,
> >  				   const unsigned long *maskp, int nmaskbits);
> >
> > +extern int bitmap_print_bitmask_to_buf(char *buf, const unsigned long
> *maskp,
> > +				      int nmaskbits, loff_t off, size_t count);
> > +
> > +extern int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp,
> > +				      int nmaskbits, loff_t off, size_t count);
> > +
> 
> Why are you adding bitmap_print_list_to_buf() when no one uses it in
> this patch series?
> 
> Did I miss it somewhere?

Yes. It is used in every patch except the last one from Yury
which is only extending comment.

drivers/base/topology.c:
+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);					\
+										\
+	return cpumap_print_list_to_buf(buf, topology_##mask(dev->id),		\
+					off, count);				\
 }


drivers/base/node.c:
+static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
+				   struct bin_attribute *attr, char *buf,
+				   loff_t off, size_t count)
 {
-	...
+	cpumask_and(mask, cpumask_of_node(node_dev->dev.id), cpu_online_mask);
+	n = cpumap_print_list_to_buf(buf, mask, off, count);
+	free_cpumask_var(mask);

> 
> thanks,
> 
> greg k-h

Thanks
Barry

Re: [PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[Max Filippov]

On Fri, Aug 6, 2021 at 11:04 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Sat, Aug 07, 2021 at 01:51:36AM +0800, kernel test robot wrote:
> > Hi Barry,
> >
> > I love your patch! Perhaps something to improve:
>
> I recall that I saw it ~1 year ago and informed Max about it.
> Don't remember what was the outcome, though.

There's a gcc bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92938
There's an opinion that it's not a gcc bug and there's been no further progress
on that side.

-- 
Thanks.
-- Max

Re: [PATCH v9 2/5] lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases

[Max Filippov]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On Fri, Aug 6, 2021 at 11:04 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Sat, Aug 07, 2021 at 01:51:36AM +0800, kernel test robot wrote:
> > Hi Barry,
> >
> > I love your patch! Perhaps something to improve:
>
> I recall that I saw it ~1 year ago and informed Max about it.
> Don't remember what was the outcome, though.

There's a gcc bug report: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92938
There's an opinion that it's not a gcc bug and there's been no further progress
on that side.

-- 
Thanks.
-- Max

RE: [PATCH v9 1/5] cpumask: introduce cpumap_print_list/bitmask_to_buf to support large bitmask and list

[Song Bao Hua (Barry Song)]

> -----Original Message-----
> From: Song Bao Hua (Barry Song)
> Sent: Saturday, August 7, 2021 7:39 AM
> To: 'Greg KH' <gregkh@linuxfoundation.org>
> Cc: andriy.shevchenko@linux.intel.com; yury.norov@gmail.com;
> linux-kernel@vger.kernel.org; akpm@linux-foundation.org;
> dave.hansen@intel.com; linux@rasmusvillemoes.dk; rafael@kernel.org;
> rdunlap@infradead.org; agordeev@linux.ibm.com; sbrivio@redhat.com;
> jianpeng.ma@intel.com; valentin.schneider@arm.com; peterz@infradead.org;
> bristot@redhat.com; guodong.xu@linaro.org; tangchengchang
> <tangchengchang@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
> yangyicong <yangyicong@huawei.com>; tim.c.chen@linux.intel.com; Linuxarm
> <linuxarm@huawei.com>; tiantao (H) <tiantao6@hisilicon.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: RE: [PATCH v9 1/5] cpumask: introduce
> cpumap_print_list/bitmask_to_buf to support large bitmask and list
> 
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Saturday, August 7, 2021 1:22 AM
> > To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> > Cc: andriy.shevchenko@linux.intel.com; yury.norov@gmail.com;
> > linux-kernel@vger.kernel.org; akpm@linux-foundation.org;
> > dave.hansen@intel.com; linux@rasmusvillemoes.dk; rafael@kernel.org;
> > rdunlap@infradead.org; agordeev@linux.ibm.com; sbrivio@redhat.com;
> > jianpeng.ma@intel.com; valentin.schneider@arm.com; peterz@infradead.org;
> > bristot@redhat.com; guodong.xu@linaro.org; tangchengchang
> > <tangchengchang@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
> > yangyicong <yangyicong@huawei.com>; tim.c.chen@linux.intel.com; Linuxarm
> > <linuxarm@huawei.com>; tiantao (H) <tiantao6@hisilicon.com>; Jonathan
> Cameron
> > <jonathan.cameron@huawei.com>
> > Subject: Re: [PATCH v9 1/5] cpumask: introduce
> > cpumap_print_list/bitmask_to_buf to support large bitmask and list
> >
> > On Fri, Aug 06, 2021 at 11:02:47PM +1200, Barry Song wrote:
> > > From: Tian Tao <tiantao6@hisilicon.com>
> > >
> > > The existing cpumap_print_to_pagebuf() is used by cpu topology and other
> > > drivers to export hexadecimal bitmask and decimal list to userspace by
> > > sysfs ABI.
> > >
> > > Right now, those drivers are using a normal attribute for this kind of
> > > ABIs. A normal attribute typically has show entry as below:
> > >
> > > static ssize_t example_dev_show(struct device *dev,
> > >                 struct device_attribute *attr, char *buf)
> > > {
> > > 	...
> > > 	return cpumap_print_to_pagebuf(true, buf, &pmu_mmdc->cpu);
> > > }
> > > show entry of attribute has no offset and count parameters and this
> > > means the file is limited to one page only.
> > >
> > > cpumap_print_to_pagebuf() API works terribly well for this kind of
> > > normal attribute with buf parameter and without offset, count:
> > >
> > > static inline ssize_t
> > > cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
> > > {
> > > 	return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
> > > 				       nr_cpu_ids);
> > > }
> > >
> > > The problem is once we have many cpus, we have a chance to make bitmask
> > > or list more than one page. Especially for list, it could be as complex
> > > as 0,3,5,7,9,...... We have no simple way to know it exact size.
> > >
> > > It turns out bin_attribute is a way to break this limit. bin_attribute
> > > has show entry as below:
> > > static ssize_t
> > > example_bin_attribute_show(struct file *filp, struct kobject *kobj,
> > >              struct bin_attribute *attr, char *buf,
> > >              loff_t offset, size_t count)
> > > {
> > > 	...
> > > }
> > >
> > > With the new offset and count parameters, this makes sysfs ABI be able
> > > to support file size more than one page. For example, offset could be
> > > >= 4096.
> > >
> > > This patch introduces cpumap_print_bitmask/list_to_buf() and their bitmap
> > > infrastructure bitmap_print_bitmask/list_to_buf() so that those drivers
> > > can move to bin_attribute to support large bitmask and list. At the same
> > > time, we have to pass those corresponding parameters such as offset, count
> > > from bin_attribute to this new API.
> > >
> > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Cc: Randy Dunlap <rdunlap@infradead.org>
> > > Cc: Stefano Brivio <sbrivio@redhat.com>
> > > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > > Cc: "Ma, Jianpeng" <jianpeng.ma@intel.com>
> > > Cc: Yury Norov <yury.norov@gmail.com>
> > > Cc: Valentin Schneider <valentin.schneider@arm.com>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> > > ---
> > >  include/linux/bitmap.h  |   6 +++
> > >  include/linux/cpumask.h |  38 +++++++++++++++
> > >  lib/bitmap.c            | 103 ++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 147 insertions(+)
> > >
> > > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> > > index a36cfcec4e77..37f36dad18bd 100644
> > > --- a/include/linux/bitmap.h
> > > +++ b/include/linux/bitmap.h
> > > @@ -227,6 +227,12 @@ unsigned int bitmap_ord_to_pos(const unsigned long
> > *bitmap, unsigned int ord, un
> > >  int bitmap_print_to_pagebuf(bool list, char *buf,
> > >  				   const unsigned long *maskp, int nmaskbits);
> > >
> > > +extern int bitmap_print_bitmask_to_buf(char *buf, const unsigned long
> > *maskp,
> > > +				      int nmaskbits, loff_t off, size_t count);
> > > +
> > > +extern int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp,
> > > +				      int nmaskbits, loff_t off, size_t count);
> > > +
> >
> > Why are you adding bitmap_print_list_to_buf() when no one uses it in
> > this patch series?
> >
> > Did I miss it somewhere?
> 
> Yes. It is used in every patch except the last one from Yury
> which is only extending comment.
> 
> drivers/base/topology.c:
> +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);					\
> +										\
> +	return cpumap_print_list_to_buf(buf, topology_##mask(dev->id),		\
> +					off, count);				\
>  }
> 
> 
> drivers/base/node.c:
> +static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
> +				   struct bin_attribute *attr, char *buf,
> +				   loff_t off, size_t count)
>  {
> -	...
> +	cpumask_and(mask, cpumask_of_node(node_dev->dev.id), cpu_online_mask);
> +	n = cpumap_print_list_to_buf(buf, mask, off, count);
> +	free_cpumask_var(mask);
> 
> >

And correspondingly, Linux has bitmask and list ABIs for cpus, eg:

root@ubuntu:/sys/devices/system/cpu/cpu0/topology# cat core_siblings
ff
root@ubuntu:/sys/devices/system/cpu/cpu0/topology# cat core_siblings_list
0-7

and for nodes, eg:
root@ubuntu:/sys/devices/system/node/node0# cat cpumap
ff
root@ubuntu:/sys/devices/system/node/node0# cat cpulist
0-7


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

Thanks
Barry

Re: [PATCH v9 0/5] use bin_attribute to break the size limitation of cpumap ABI

[Barry Song]

> V9:
>   - Split bitmask and list APIs and removed bool parameter with respect to
>     Greg's comment
>   - Removed duplication in code doc
>
...
>
> Background:
>
> the whole story began from this thread when Jonatah and me tried to add a
> new topology level-cluster which exists on kunpeng920 and X86 Jacobsville:
> https://lore.kernel.org/lkml/YFRGIedW1fUlnmi+@kroah.com/
> https://lore.kernel.org/lkml/YFR2kwakbcGiI37w@kroah.com/
>

Hi Greg,
Will you take this series so that I can rebase the cluster-scheduler series[1] on top of
this? that cluster series is where this ABI series really get started. I am looking forward
to sending a normal patchset for cluster series after this ABI series settles down.

[1] scheduler: expose the topology of clusters and add cluster scheduler
https://lore.kernel.org/lkml/20210420001844.9116-1-song.bao.hua@hisilicon.com/

>
> Barry Song (1):
>   lib: test_bitmap: add bitmap_print_bitmask/list_to_buf test cases
>
> Tian Tao (3):
>   cpumask: introduce cpumap_print_list/bitmask_to_buf to support large
>     bitmask and list
>   topology: use bin_attribute to break the size limitation of cpumap ABI
>   drivers/base/node.c: use bin_attribute to break the size limitation of
>     cpumap ABI
>
> Yury Norov (1):
>   bitmap: extend comment to bitmap_print_bitmask/list_to_buf
>
>  drivers/base/node.c     |  63 +++++++++++------
>  drivers/base/topology.c | 115 ++++++++++++++++--------------
>  include/linux/bitmap.h  |   6 ++
>  include/linux/cpumask.h |  38 ++++++++++
>  lib/bitmap.c            | 121 ++++++++++++++++++++++++++++++++
>  lib/test_bitmap.c       | 150 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 418 insertions(+), 75 deletions(-)
>
> --
> 2.25.1

Thanks
Barry

Re: [PATCH v9 0/5] use bin_attribute to break the size limitation of cpumap ABI

[Greg KH]

On Thu, Aug 12, 2021 at 04:44:26PM +1200, Barry Song wrote:
> > V9:
> >   - Split bitmask and list APIs and removed bool parameter with respect to
> >     Greg's comment
> >   - Removed duplication in code doc
> >
> ...
> >
> > Background:
> >
> > the whole story began from this thread when Jonatah and me tried to add a
> > new topology level-cluster which exists on kunpeng920 and X86 Jacobsville:
> > https://lore.kernel.org/lkml/YFRGIedW1fUlnmi+@kroah.com/
> > https://lore.kernel.org/lkml/YFR2kwakbcGiI37w@kroah.com/
> >
> 
> Hi Greg,
> Will you take this series so that I can rebase the cluster-scheduler series[1] on top of
> this? that cluster series is where this ABI series really get started. I am looking forward
> to sending a normal patchset for cluster series after this ABI series settles down.
> 
> [1] scheduler: expose the topology of clusters and add cluster scheduler
> https://lore.kernel.org/lkml/20210420001844.9116-1-song.bao.hua@hisilicon.com/

Now applied to my testing tree.

thanks,

greg k-h