Re: Another (ESP?) scsi blk-mq problem on sparc64

[Jens Axboe <axboe@kernel.dk>]

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

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


[-- Attachment #2: cpu-pos-count.patch --]
[-- Type: text/x-patch, Size: 634 bytes --]

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;