On 11/24/2014 01:21 AM, Christoph Hellwig wrote:
On Fri, Nov 21, 2014 at 02:56:00PM -0500, David Miller wrote:
I would suggest looking into the possibility that we allocate the memory
using the count of valid cpus, rather than the largest cpu number.
That's a common error that runs into problems with discontiguous
cpu numbering like Sparc sometimes has.
Yes, that does look like the case. Do you have a good trick on how
to allocate a map for the highest possible cpu number without first
iterating the cpu map? I couldn't find something that looks like a
highest_possible_cpu() helper.
Honestly I think that num_posible_cpus() should return the max of number
of CPUs (weigt), and the highest numbered CPU. It's a pain in the butt
to handle this otherwise.
/* If cpus are offline, map them to first hctx */
map = kzalloc_node(sizeof(*map) * num_possible_cpus(), GFP_KERNEL,
set->numa_node);
is where it goes wrong, assuming Meelis has NR_CPUS set to something <
14, which was the highest numbered CPU, iirc. A construct like:
map = alloc(num_possible_cpus());
for_each_possible_cpu(i)
map[i] = ...
seems like the obvious way to do things, yet it's broken. And not broken
on x86 where we'd find the issue pretty quickly, but on sparc64 where
it'll take a lot longer to run into. It just violates the principle of
least surprise, which is bad for an API.
That said, if there was a helper like Christoph suggested, it'd be a bit
better. And perhaps we can't just modify num_possible_cpus(), we'd need
num_*_cpus() for the other bitmaps, too. That might be breaking other
things...
Meelis, can you try the attached?
--
Jens Axboe
diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 1065d7c65fa1..15da9cc08f64 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -87,10 +87,14 @@ int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues)
unsigned int *blk_mq_make_queue_map(struct blk_mq_tag_set *set)
{
+ unsigned int max_cpus;
unsigned int *map;
+ for_each_possible_cpu(max_cpus)
+ ;
+
/* If cpus are offline, map them to first hctx */
- map = kzalloc_node(sizeof(*map) * num_possible_cpus(), GFP_KERNEL,
+ map = kzalloc_node(sizeof(*map) * max_cpus, GFP_KERNEL,
set->numa_node);
if (!map)
return NULL;
