linux-kernel - Re: [PATCH v2 1/6] blk-mq: introduce blk_mq_hctx_map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241112044736.GA8883@lst.de>
Date: Tue, 12 Nov 2024 05:47:36 +0100
From: Christoph Hellwig <hch@....de>
To: Daniel Wagner <wagi@...nel.org>
Cc: Jens Axboe <axboe@...nel.dk>, Bjorn Helgaas <bhelgaas@...gle.com>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Jason Wang <jasowang@...hat.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Keith Busch <kbusch@...nel.org>, Christoph Hellwig <hch@....de>,
	Sagi Grimberg <sagi@...mberg.me>, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	virtualization@...ts.linux.dev, linux-scsi@...r.kernel.org,
	megaraidlinux.pdl@...adcom.com, mpi3mr-linuxdrv.pdl@...adcom.com,
	MPT-FusionLinux.pdl@...adcom.com, storagedev@...rochip.com,
	linux-nvme@...ts.infradead.org, Daniel Wagner <dwagner@...e.de>
Subject: Re: [PATCH v2 1/6] blk-mq: introduce blk_mq_hctx_map_queues

On Mon, Nov 11, 2024 at 07:02:09PM +0100, Daniel Wagner wrote:
> blk_mq_pci_map_queues and blk_mq_virtio_map_queues will create a CPU to
> hardware queue mapping based on affinity information. These two function
> share common code and only differ on how the affinity information is
> retrieved. Also, those functions are located in the block subsystem
> where it doesn't really fit in. They are virtio and pci subsystem
> specific.
> 
> Introduce a new callback in struct bus_type to get the affinity mask.
> The callbacks can then be populated by the subsystem directly.
> 
> All but one driver use the subsystem default affinity masks. hisi_sas v2
> depends on a driver specific mapping, thus use the optional argument
> get_queue_affinity to retrieve the mapping.

This seems to mix up a few different things:

 1) adding a new bus operation
 2) implementations of that operation for PCI and virtio
 3) a block layer consumer of the operation

all these really should be separate per-subsystem patches.

You'll also need to Cc the driver model maintainers.

> +void blk_mq_hctx_map_queues(struct blk_mq_queue_map *qmap,
> +			    struct device *dev, unsigned int offset,
> +			    get_queue_affinity_fn *get_irq_affinity)
> +{
> +	const struct cpumask *mask = NULL;
> +	unsigned int queue, cpu;
> +
> +	for (queue = 0; queue < qmap->nr_queues; queue++) {
> +		if (get_irq_affinity)
> +			mask = get_irq_affinity(dev, queue + offset);
> +		else if (dev->bus->irq_get_affinity)
> +			mask = dev->bus->irq_get_affinity(dev, queue + offset);
> +
> +		if (!mask)
> +			goto fallback;
> +
> +		for_each_cpu(cpu, mask)
> +			qmap->mq_map[cpu] = qmap->queue_offset + queue;
> +	}

This does different things with a NULL argument vs not.  Please split it
into two separate helpers.  The non-NULL case is only uses in hisi_sas,
so it might be worth just open coding it there as well.

> +static const struct cpumask *pci_device_irq_get_affinity(struct device *dev,
> +					unsigned int irq_vec)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	return pci_irq_get_affinity(pdev, irq_vec);

Nit: this could be shortened to:

	return pci_irq_get_affinity(to_pci_dev(dev), irq_vec);