Re: [PATCH v4 1/9] lib/group_cpus: let group_cpu_evenly return number of groups

Hannes Reinecke
2025-01-07T08:24:50Z

On 12/17/24 19:29, Daniel Wagner wrote:
> group_cpu_evenly might allocated less groups then the requested:
>
> group_cpu_evenly
> __group_cpus_evenly
> alloc_nodes_groups
> # allocated total groups may be less than numgrps when
> # active total CPU number is less then numgrps
>
> In this case, the caller will do an out of bound access because the
> caller assumes the masks returned has numgrps.
>
> Return the number of groups created so the caller can limit the access
> range accordingly.
>
> Signed-off-by: Daniel Wagner <wagi@kernel.org>
> ---
> block/blk-mq-cpumap.c | 7 ++++---
> drivers/virtio/virtio_vdpa.c | 2 +-
> fs/fuse/virtio_fs.c | 7 ++++---
> include/linux/group_cpus.h | 2 +-
> kernel/irq/affinity.c | 2 +-
> lib/group_cpus.c | 23 +++++++++++++----------
> 6 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
> index ad8d6a363f24ae11968b42f7bcfd6a719a0499b7..85c0a7073bd8bff5d34aad1729d45d89da4c4bd1 100644
> --- a/block/blk-mq-cpumap.c
> +++ b/block/blk-mq-cpumap.c
> @@ -19,9 +19,10 @@
> void blk_mq_map_queues(struct blk_mq_queue_map *qmap)
> {
> const struct cpumask *masks;
> - unsigned int queue, cpu;
> + unsigned int queue, cpu, nr_masks;
>
> - masks = group_cpus_evenly(qmap->nr_queues);
> + nr_masks = qmap->nr_queues;
> + masks = group_cpus_evenly(&nr_masks);

Hmph. I am a big fan of separating input and output paramenters;
most ABI definitions will be doing that anyway.
Makes it also really hard to track whether the output parameters
had been set at all. Care to split it up?

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich