* Re: [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes
@ 2022-05-11 8:54 kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-05-11 8:54 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 20820 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <1652181274-136198-3-git-send-email-john.garry@huawei.com>
References: <1652181274-136198-3-git-send-email-john.garry@huawei.com>
TO: John Garry <john.garry@huawei.com>
Hi John,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linux/master linus/master v5.18-rc6 next-20220510]
[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/intel-lab-lkp/linux/commits/John-Garry/sbitmap-NUMA-node-spreading/20220510-192122
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
:::::: branch date: 21 hours ago
:::::: commit date: 21 hours ago
config: i386-randconfig-c001-20220509 (https://download.01.org/0day-ci/archive/20220511/202205111616.bVssFWeh-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 18dd123c56754edf62c7042dcf23185c3727610f)
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/intel-lab-lkp/linux/commit/9cb4cd7060790dcb3d7f9405c11a0da884a0c616
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review John-Garry/sbitmap-NUMA-node-spreading/20220510-192122
git checkout 9cb4cd7060790dcb3d7f9405c11a0da884a0c616
# save the config file
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
^
fs/ntfs/mft.c:1798:2: note: Taking false branch
if (IS_ERR(rl)) {
^
fs/ntfs/mft.c:1811:2: note: Taking false branch
ntfs_debug("Allocated %lli clusters.", (long long)nr);
^
fs/ntfs/debug.h:39:2: note: expanded from macro 'ntfs_debug'
if (0) \
^
fs/ntfs/mft.c:1811:2: note: Loop condition is false. Exiting loop
ntfs_debug("Allocated %lli clusters.", (long long)nr);
^
fs/ntfs/debug.h:37:35: note: expanded from macro 'ntfs_debug'
#define ntfs_debug(fmt, ...) \
^
fs/ntfs/mft.c:1813:2: note: Loop condition is false. Execution continues on line 1816
for (; rl[1].length; rl++)
^
fs/ntfs/mft.c:1817:2: note: Taking true branch
if (IS_ERR(mrec)) {
^
fs/ntfs/mft.c:1820:3: note: Control jumps to line 1953
goto undo_alloc;
^
fs/ntfs/mft.c:1953:6: note: Assuming the condition is false
if (ntfs_cluster_free(mft_ni, old_last_vcn, -1, ctx) < 0) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/ntfs/mft.c:1953:2: note: Taking false branch
if (ntfs_cluster_free(mft_ni, old_last_vcn, -1, ctx) < 0) {
^
fs/ntfs/mft.c:1958:6: note: Access to field 'attr' results in a dereference of a null pointer (loaded from variable 'ctx')
a = ctx->attr;
^~~
fs/ntfs/mft.c:2019:2: warning: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
memset(m, 0, vol->mft_record_size);
^
arch/x86/include/asm/string_32.h:195:29: note: expanded from macro 'memset'
#define memset(s, c, count) __builtin_memset(s, c, count)
^~~~~~~~~~~~~~~~
fs/ntfs/mft.c:2019:2: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
memset(m, 0, vol->mft_record_size);
^
arch/x86/include/asm/string_32.h:195:29: note: expanded from macro 'memset'
#define memset(s, c, count) __builtin_memset(s, c, count)
^~~~~~~~~~~~~~~~
fs/ntfs/mft.c:2813:3: warning: Call to function 'memmove' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memmove_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
memmove(extent_nis, extent_nis + 1, (base_ni->nr_extents - i) *
^~~~~~~
fs/ntfs/mft.c:2813:3: note: Call to function 'memmove' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memmove_s' in case of C11
memmove(extent_nis, extent_nis + 1, (base_ni->nr_extents - i) *
^~~~~~~
fs/ntfs/mft.c:2893:4: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
memcpy(extent_nis, base_ni->ext.extent_ntfs_inos,
^
arch/x86/include/asm/string_32.h:150:25: note: expanded from macro 'memcpy'
#define memcpy(t, f, n) __builtin_memcpy(t, f, n)
^~~~~~~~~~~~~~~~
fs/ntfs/mft.c:2893:4: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
memcpy(extent_nis, base_ni->ext.extent_ntfs_inos,
^
arch/x86/include/asm/string_32.h:150:25: note: expanded from macro 'memcpy'
#define memcpy(t, f, n) __builtin_memcpy(t, f, n)
^~~~~~~~~~~~~~~~
Suppressed 43 warnings (43 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
20 warnings generated.
lib/oid_registry.c:142:16: warning: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
ret = count = snprintf(buffer, bufsize, "%u.%u", n / 40, n % 40);
^~~~~~~~
lib/oid_registry.c:142:16: note: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11
ret = count = snprintf(buffer, bufsize, "%u.%u", n / 40, n % 40);
^~~~~~~~
lib/oid_registry.c:149:3: warning: Value stored to 'num' is never read [clang-analyzer-deadcode.DeadStores]
num = 0;
^ ~
lib/oid_registry.c:149:3: note: Value stored to 'num' is never read
num = 0;
^ ~
lib/oid_registry.c:163:18: warning: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
ret += count = snprintf(buffer, bufsize, ".%lu", num);
^~~~~~~~
lib/oid_registry.c:163:18: note: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11
ret += count = snprintf(buffer, bufsize, ".%lu", num);
^~~~~~~~
lib/oid_registry.c:173:2: warning: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
snprintf(buffer, bufsize, "(bad)");
^~~~~~~~
lib/oid_registry.c:173:2: note: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11
snprintf(buffer, bufsize, "(bad)");
^~~~~~~~
Suppressed 16 warnings (16 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
16 warnings generated.
Suppressed 16 warnings (16 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
19 warnings generated.
Suppressed 19 warnings (12 in non-user code, 7 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
31 warnings generated.
>> lib/sbitmap.c:138:63: warning: The expression is an uninitialized value. The computed value will also be garbage [clang-analyzer-core.uninitialized.Assign]
for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
^
lib/sbitmap.c:481:8: note: Calling 'sbitmap_init_node'
ret = sbitmap_init_node(&sbq->sb, depth, shift, flags, node,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lib/sbitmap.c:103:11: note: 'map_nr_cnt' declared without an initial value
int nid, map_nr_cnt;
^~~~~~~~~~
lib/sbitmap.c:105:6: note: Assuming 'shift' is >= 0
if (shift < 0)
^~~~~~~~~
lib/sbitmap.c:105:2: note: Taking false branch
if (shift < 0)
^
lib/sbitmap.c:109:6: note: Assuming 'bits_per_word' is <= BITS_PER_LONG
if (bits_per_word > BITS_PER_LONG)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lib/sbitmap.c:109:2: note: Taking false branch
if (bits_per_word > BITS_PER_LONG)
^
lib/sbitmap.c:117:6: note: Assuming 'depth' is not equal to 0
if (depth == 0) {
^~~~~~~~~~
lib/sbitmap.c:117:2: note: Taking false branch
if (depth == 0) {
^
lib/sbitmap.c:122:6: note: Assuming 'num_nodes' is <= field 'map_nr'
if (sb->map_nr < num_nodes) {
^~~~~~~~~~~~~~~~~~~~~~
lib/sbitmap.c:122:2: note: Taking false branch
if (sb->map_nr < num_nodes) {
^
lib/sbitmap.c:127:6: note: 'alloc_hint' is true
if (alloc_hint) {
^~~~~~~~~~
lib/sbitmap.c:127:2: note: Taking true branch
if (alloc_hint) {
^
lib/sbitmap.c:128:7: note: Calling 'init_alloc_hint'
if (init_alloc_hint(sb, flags))
^~~~~~~~~~~~~~~~~~~~~~~~~~
lib/sbitmap.c:28:6: note: Assuming field 'alloc_hint' is non-null
if (!sb->alloc_hint)
^~~~~~~~~~~~~~~
lib/sbitmap.c:28:2: note: Taking false branch
if (!sb->alloc_hint)
^
lib/sbitmap.c:31:6: note: 'depth' is not equal to 0
if (depth && !sb->round_robin) {
^~~~~
lib/sbitmap.c:31:6: note: Left side of '&&' is true
lib/sbitmap.c:31:15: note: Assuming field 'round_robin' is true
if (depth && !sb->round_robin) {
^~~~~~~~~~~~~~~~
lib/sbitmap.c:31:2: note: Taking false branch
if (depth && !sb->round_robin) {
^
lib/sbitmap.c:39:2: note: Returning without writing to 'sb->map', which participates in a condition later
return 0;
^
lib/sbitmap.c:128:7: note: Returning from 'init_alloc_hint'
if (init_alloc_hint(sb, flags))
^~~~~~~~~~~~~~~~~~~~~~~~~~
lib/sbitmap.c:128:3: note: Taking false branch
if (init_alloc_hint(sb, flags))
^
lib/sbitmap.c:135:6: note: Assuming field 'map' is non-null
if (!sb->map)
^~~~~~~~
lib/sbitmap.c:135:2: note: Taking false branch
if (!sb->map)
^
lib/sbitmap.c:138:27: note: 'index' is < field 'map_nr'
for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
^~~~~
lib/sbitmap.c:138:2: note: Loop condition is true. Entering loop body
for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
^
lib/sbitmap.c:141:3: note: Taking true branch
if ((index % sb->map_nr_per_node) == 0) {
^
lib/sbitmap.c:144:8: note: 'index' is equal to 0
if (index == 0) {
^~~~~
lib/sbitmap.c:144:4: note: Taking true branch
if (index == 0) {
^
lib/sbitmap.c:152:8: note: Assuming 'map' is non-null
if (!map)
^~~~
lib/sbitmap.c:152:4: note: Taking false branch
if (!map)
^
lib/sbitmap.c:138:63: note: The expression is an uninitialized value. The computed value will also be garbage
for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
^~~~~~~~~~
Suppressed 30 warnings (29 in non-user code, 1 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
36 warnings generated.
Suppressed 36 warnings (36 in non-user code).
vim +138 lib/sbitmap.c
b2dbff1bb893d5 Jens Axboe 2018-12-11 95
88459642cba452 Omar Sandoval 2016-09-17 96 int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
c548e62bcf6adc Ming Lei 2021-01-22 97 gfp_t flags, int node, bool round_robin,
c548e62bcf6adc Ming Lei 2021-01-22 98 bool alloc_hint)
88459642cba452 Omar Sandoval 2016-09-17 99 {
88459642cba452 Omar Sandoval 2016-09-17 100 unsigned int bits_per_word;
829ed3bea2e697 John Garry 2022-05-10 101 struct sbitmap_word *map;
9cb4cd7060790d John Garry 2022-05-10 102 int index, num_nodes = num_online_nodes();
9cb4cd7060790d John Garry 2022-05-10 103 int nid, map_nr_cnt;
88459642cba452 Omar Sandoval 2016-09-17 104
2d13b1ea9f4aff Ming Lei 2021-01-22 105 if (shift < 0)
2d13b1ea9f4aff Ming Lei 2021-01-22 106 shift = sbitmap_calculate_shift(depth);
2d13b1ea9f4aff Ming Lei 2021-01-22 107
88459642cba452 Omar Sandoval 2016-09-17 108 bits_per_word = 1U << shift;
88459642cba452 Omar Sandoval 2016-09-17 109 if (bits_per_word > BITS_PER_LONG)
88459642cba452 Omar Sandoval 2016-09-17 110 return -EINVAL;
88459642cba452 Omar Sandoval 2016-09-17 111
88459642cba452 Omar Sandoval 2016-09-17 112 sb->shift = shift;
88459642cba452 Omar Sandoval 2016-09-17 113 sb->depth = depth;
88459642cba452 Omar Sandoval 2016-09-17 114 sb->map_nr = DIV_ROUND_UP(sb->depth, bits_per_word);
efe1f3a1d5833c Ming Lei 2021-01-22 115 sb->round_robin = round_robin;
88459642cba452 Omar Sandoval 2016-09-17 116
88459642cba452 Omar Sandoval 2016-09-17 117 if (depth == 0) {
88459642cba452 Omar Sandoval 2016-09-17 118 sb->map = NULL;
88459642cba452 Omar Sandoval 2016-09-17 119 return 0;
88459642cba452 Omar Sandoval 2016-09-17 120 }
88459642cba452 Omar Sandoval 2016-09-17 121
9cb4cd7060790d John Garry 2022-05-10 122 if (sb->map_nr < num_nodes) {
9cb4cd7060790d John Garry 2022-05-10 123 sb->map_nr_per_node = 1;
9cb4cd7060790d John Garry 2022-05-10 124 } else {
9cb4cd7060790d John Garry 2022-05-10 125 sb->map_nr_per_node = sb->map_nr / num_nodes;
9cb4cd7060790d John Garry 2022-05-10 126 }
c548e62bcf6adc Ming Lei 2021-01-22 127 if (alloc_hint) {
c548e62bcf6adc Ming Lei 2021-01-22 128 if (init_alloc_hint(sb, flags))
c548e62bcf6adc Ming Lei 2021-01-22 129 return -ENOMEM;
c548e62bcf6adc Ming Lei 2021-01-22 130 } else {
c548e62bcf6adc Ming Lei 2021-01-22 131 sb->alloc_hint = NULL;
c548e62bcf6adc Ming Lei 2021-01-22 132 }
c548e62bcf6adc Ming Lei 2021-01-22 133
863a66cdb4df25 Ming Lei 2022-03-16 134 sb->map = kvzalloc_node(sb->map_nr * sizeof(*sb->map), flags, node);
9cb4cd7060790d John Garry 2022-05-10 135 if (!sb->map)
9cb4cd7060790d John Garry 2022-05-10 136 goto err_map;
9cb4cd7060790d John Garry 2022-05-10 137
9cb4cd7060790d John Garry 2022-05-10 @138 for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
9cb4cd7060790d John Garry 2022-05-10 139 struct sbitmap_word **_map;
9cb4cd7060790d John Garry 2022-05-10 140
9cb4cd7060790d John Garry 2022-05-10 141 if ((index % sb->map_nr_per_node) == 0) {
9cb4cd7060790d John Garry 2022-05-10 142 int cnt;
9cb4cd7060790d John Garry 2022-05-10 143
9cb4cd7060790d John Garry 2022-05-10 144 if (index == 0) {
9cb4cd7060790d John Garry 2022-05-10 145 cnt = sb->map_nr_per_node +
9cb4cd7060790d John Garry 2022-05-10 146 (sb->map_nr % sb->map_nr_per_node);
9cb4cd7060790d John Garry 2022-05-10 147 } else {
9cb4cd7060790d John Garry 2022-05-10 148 cnt = sb->map_nr_per_node;
c548e62bcf6adc Ming Lei 2021-01-22 149 }
88459642cba452 Omar Sandoval 2016-09-17 150
9cb4cd7060790d John Garry 2022-05-10 151 map = kvzalloc_node(cnt * sizeof(**sb->map), flags, nid);
829ed3bea2e697 John Garry 2022-05-10 152 if (!map)
9cb4cd7060790d John Garry 2022-05-10 153 goto err_map_numa;
9cb4cd7060790d John Garry 2022-05-10 154 nid++;
9cb4cd7060790d John Garry 2022-05-10 155 }
829ed3bea2e697 John Garry 2022-05-10 156
829ed3bea2e697 John Garry 2022-05-10 157 _map = &sb->map[index];
829ed3bea2e697 John Garry 2022-05-10 158 *_map = map;
829ed3bea2e697 John Garry 2022-05-10 159 }
829ed3bea2e697 John Garry 2022-05-10 160
88459642cba452 Omar Sandoval 2016-09-17 161 return 0;
9cb4cd7060790d John Garry 2022-05-10 162 err_map_numa:
9cb4cd7060790d John Garry 2022-05-10 163 for (index = 0; index < sb->map_nr; index++, map++) {
9cb4cd7060790d John Garry 2022-05-10 164 if ((index % sb->map_nr_per_node) == 0) {
9cb4cd7060790d John Garry 2022-05-10 165 kfree(map);
9cb4cd7060790d John Garry 2022-05-10 166 }
9cb4cd7060790d John Garry 2022-05-10 167 }
9cb4cd7060790d John Garry 2022-05-10 168 err_map:
9cb4cd7060790d John Garry 2022-05-10 169 free_percpu(sb->alloc_hint);
9cb4cd7060790d John Garry 2022-05-10 170
9cb4cd7060790d John Garry 2022-05-10 171 return -ENOMEM;
88459642cba452 Omar Sandoval 2016-09-17 172 }
88459642cba452 Omar Sandoval 2016-09-17 173 EXPORT_SYMBOL_GPL(sbitmap_init_node);
88459642cba452 Omar Sandoval 2016-09-17 174
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 3+ messages in thread* [RFC PATCH 0/2] sbitmap: NUMA node spreading
@ 2022-05-10 11:14 John Garry
2022-05-10 11:14 ` [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes John Garry
0 siblings, 1 reply; 3+ messages in thread
From: John Garry @ 2022-05-10 11:14 UTC (permalink / raw)
To: axboe, linux-block; +Cc: linux-kernel, linux-scsi, John Garry
Hi Jens, guys,
I am sending this as an RFC to see if there is any future in it or ideas
on how to make better. I also need to improve some items (as mentioned in
2/2 commit message) and test a lot more.
The general idea is that we change from allocating a single array of
sbitmap words to allocating an sub-array per NUMA node. And then each CPU
in that node is hinted to use that sub-array
Initial performance looks decent.
Some figures:
System: 4-nodes (with memory on all nodes), 128 CPUs
null blk config block:
20 devs, submit_queues=NR_CPUS, shared_tags, shared_tag_bitmap,
hw_queue_depth=256
fio config:
bs=4096, iodepth=128, numjobs=10, cpus_allowed_policy=split, rw=read,
ioscheduler=none
Before:
7130K
After:
7630K
So a +7% IOPS gain.
Any comments welcome, thanks!.
Based on v5.18-rc6.
John Garry (2):
sbitmap: Make sbitmap.map a double pointer
sbitmap: Spread sbitmap word allocation over NUMA nodes
include/linux/sbitmap.h | 16 +++++---
lib/sbitmap.c | 83 +++++++++++++++++++++++++++++++++--------
2 files changed, 79 insertions(+), 20 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes
2022-05-10 11:14 [RFC PATCH 0/2] sbitmap: NUMA node spreading John Garry
@ 2022-05-10 11:14 ` John Garry
2022-05-10 19:31 ` kernel test robot
0 siblings, 1 reply; 3+ messages in thread
From: John Garry @ 2022-05-10 11:14 UTC (permalink / raw)
To: axboe, linux-block; +Cc: linux-kernel, linux-scsi, John Garry
Currently sbitmap words are allocated in a single array. That array is
flagged for allocating at the NUMA node passed in sbitmap_init_node().
However often the sbitmap will be accessed by all the CPUs in the system -
for example, when BLK_MQ_F_TAG_HCTX_SHARED is set for the blk-mq tagset.
This can lead to performance issues where all CPUs in the system are doing
cross-NUMA node accesses to read/set/unset sbitmap tags.
Improve this by spreading the word allocations across all NUMA nodes as
evenly as possible. We set the per-CPU hint to fall within range of words
allocated for the NUMA node to which that CPU belongs.
Known issues:
- sbitmap resize does not work well for this scheme
- Improve updating hint for sbitmap_get() failure and sbitmap_put() when
hint is outside range of CPU's NUMA node words.
- Add intelligence for sub-arrays to be allocated at a single node, e.g.
when numa != NUMA_NO_NODE in sbitmap_init_node()
Signed-off-by: John Garry <john.garry@huawei.com>
---
include/linux/sbitmap.h | 5 ++++
lib/sbitmap.c | 63 +++++++++++++++++++++++++++++++++--------
2 files changed, 56 insertions(+), 12 deletions(-)
diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h
index 46268f391e32..6d897032dbc6 100644
--- a/include/linux/sbitmap.h
+++ b/include/linux/sbitmap.h
@@ -60,6 +60,11 @@ struct sbitmap {
*/
unsigned int map_nr;
+ /**
+ * @map_nr_per_node: Number of words being used per NUMA node.
+ */
+ unsigned int map_nr_per_node;
+
/**
* @round_robin: Allocate bits in strict round-robin order.
*/
diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index 64fb9800ed8c..99c87fbfa1a1 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -8,6 +8,17 @@
#include <linux/random.h>
#include <linux/sbitmap.h>
#include <linux/seq_file.h>
+#include <linux/mm.h>
+
+static unsigned int sbitmap_get_new_hint(struct sbitmap *sb, int cpu)
+{
+ unsigned int shift = sb->shift;
+ unsigned int map_nr_per_node = sb->map_nr_per_node;
+ unsigned int bit_per_node = map_nr_per_node << shift;
+ unsigned int hint_base = bit_per_node * cpu_to_node(cpu);
+
+ return hint_base + (prandom_u32() % bit_per_node);
+}
static int init_alloc_hint(struct sbitmap *sb, gfp_t flags)
{
@@ -20,8 +31,10 @@ static int init_alloc_hint(struct sbitmap *sb, gfp_t flags)
if (depth && !sb->round_robin) {
int i;
- for_each_possible_cpu(i)
- *per_cpu_ptr(sb->alloc_hint, i) = prandom_u32() % depth;
+ for_each_possible_cpu(i) {
+ *per_cpu_ptr(sb->alloc_hint, i) =
+ sbitmap_get_new_hint(sb, i);
+ }
}
return 0;
}
@@ -86,7 +99,8 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
{
unsigned int bits_per_word;
struct sbitmap_word *map;
- int index;
+ int index, num_nodes = num_online_nodes();
+ int nid, map_nr_cnt;
if (shift < 0)
shift = sbitmap_calculate_shift(depth);
@@ -105,6 +119,11 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
return 0;
}
+ if (sb->map_nr < num_nodes) {
+ sb->map_nr_per_node = 1;
+ } else {
+ sb->map_nr_per_node = sb->map_nr / num_nodes;
+ }
if (alloc_hint) {
if (init_alloc_hint(sb, flags))
return -ENOMEM;
@@ -113,23 +132,43 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
}
sb->map = kvzalloc_node(sb->map_nr * sizeof(*sb->map), flags, node);
- if (!sb->map) {
- free_percpu(sb->alloc_hint);
- return -ENOMEM;
- }
-
- map = kvzalloc_node(sb->map_nr * sizeof(**sb->map), flags, node);
- if (!map)
- return -ENOMEM;
+ if (!sb->map)
+ goto err_map;
- for (index = 0; index < sb->map_nr; index++, map++) {
+ for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
struct sbitmap_word **_map;
+ if ((index % sb->map_nr_per_node) == 0) {
+ int cnt;
+
+ if (index == 0) {
+ cnt = sb->map_nr_per_node +
+ (sb->map_nr % sb->map_nr_per_node);
+ } else {
+ cnt = sb->map_nr_per_node;
+ }
+
+ map = kvzalloc_node(cnt * sizeof(**sb->map), flags, nid);
+ if (!map)
+ goto err_map_numa;
+ nid++;
+ }
+
_map = &sb->map[index];
*_map = map;
}
return 0;
+err_map_numa:
+ for (index = 0; index < sb->map_nr; index++, map++) {
+ if ((index % sb->map_nr_per_node) == 0) {
+ kfree(map);
+ }
+ }
+err_map:
+ free_percpu(sb->alloc_hint);
+
+ return -ENOMEM;
}
EXPORT_SYMBOL_GPL(sbitmap_init_node);
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes
2022-05-10 11:14 ` [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes John Garry
@ 2022-05-10 19:31 ` kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-05-10 19:31 UTC (permalink / raw)
To: John Garry; +Cc: llvm, kbuild-all
Hi John,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linux/master linus/master v5.18-rc6 next-20220510]
[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/intel-lab-lkp/linux/commits/John-Garry/sbitmap-NUMA-node-spreading/20220510-192122
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-randconfig-a003-20220509 (https://download.01.org/0day-ci/archive/20220511/202205110332.EbWRHZ10-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 18dd123c56754edf62c7042dcf23185c3727610f)
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/intel-lab-lkp/linux/commit/9cb4cd7060790dcb3d7f9405c11a0da884a0c616
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review John-Garry/sbitmap-NUMA-node-spreading/20220510-192122
git checkout 9cb4cd7060790dcb3d7f9405c11a0da884a0c616
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 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 >>):
>> lib/sbitmap.c:138:63: warning: variable 'map_nr_cnt' is uninitialized when used here [-Wuninitialized]
for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
^~~~~~~~~~
lib/sbitmap.c:103:21: note: initialize the variable 'map_nr_cnt' to silence this warning
int nid, map_nr_cnt;
^
= 0
1 warning generated.
vim +/map_nr_cnt +138 lib/sbitmap.c
95
96 int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
97 gfp_t flags, int node, bool round_robin,
98 bool alloc_hint)
99 {
100 unsigned int bits_per_word;
101 struct sbitmap_word *map;
102 int index, num_nodes = num_online_nodes();
103 int nid, map_nr_cnt;
104
105 if (shift < 0)
106 shift = sbitmap_calculate_shift(depth);
107
108 bits_per_word = 1U << shift;
109 if (bits_per_word > BITS_PER_LONG)
110 return -EINVAL;
111
112 sb->shift = shift;
113 sb->depth = depth;
114 sb->map_nr = DIV_ROUND_UP(sb->depth, bits_per_word);
115 sb->round_robin = round_robin;
116
117 if (depth == 0) {
118 sb->map = NULL;
119 return 0;
120 }
121
122 if (sb->map_nr < num_nodes) {
123 sb->map_nr_per_node = 1;
124 } else {
125 sb->map_nr_per_node = sb->map_nr / num_nodes;
126 }
127 if (alloc_hint) {
128 if (init_alloc_hint(sb, flags))
129 return -ENOMEM;
130 } else {
131 sb->alloc_hint = NULL;
132 }
133
134 sb->map = kvzalloc_node(sb->map_nr * sizeof(*sb->map), flags, node);
135 if (!sb->map)
136 goto err_map;
137
> 138 for (index = 0, nid = 0; index < sb->map_nr; index++, map++, map_nr_cnt++) {
139 struct sbitmap_word **_map;
140
141 if ((index % sb->map_nr_per_node) == 0) {
142 int cnt;
143
144 if (index == 0) {
145 cnt = sb->map_nr_per_node +
146 (sb->map_nr % sb->map_nr_per_node);
147 } else {
148 cnt = sb->map_nr_per_node;
149 }
150
151 map = kvzalloc_node(cnt * sizeof(**sb->map), flags, nid);
152 if (!map)
153 goto err_map_numa;
154 nid++;
155 }
156
157 _map = &sb->map[index];
158 *_map = map;
159 }
160
161 return 0;
162 err_map_numa:
163 for (index = 0; index < sb->map_nr; index++, map++) {
164 if ((index % sb->map_nr_per_node) == 0) {
165 kfree(map);
166 }
167 }
168 err_map:
169 free_percpu(sb->alloc_hint);
170
171 return -ENOMEM;
172 }
173 EXPORT_SYMBOL_GPL(sbitmap_init_node);
174
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-11 8:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-11 8:54 [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2022-05-10 11:14 [RFC PATCH 0/2] sbitmap: NUMA node spreading John Garry
2022-05-10 11:14 ` [RFC PATCH 2/2] sbitmap: Spread sbitmap word allocation over NUMA nodes John Garry
2022-05-10 19:31 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.