[PATCH] Inline contents of BLK_MQ_VIRTIO config

[PATCH] Inline contents of BLK_MQ_VIRTIO config

[Ram Muthiah]

The config contains one symbol and is a dep of only two configs.
Inlined this symbol so that it's built in by the two configs
which need it and deleted the config.

Signed-off-by: Ram Muthiah <rammuthiah@google.com>
---
 block/Kconfig                 |  5 ----
 block/Makefile                |  1 -
 block/blk-mq-virtio.c         | 46 -----------------------------------
 include/linux/blk-mq-virtio.h | 43 +++++++++++++++++++++++++++++---
 4 files changed, 39 insertions(+), 56 deletions(-)
 delete mode 100644 block/blk-mq-virtio.c

diff --git a/block/Kconfig b/block/Kconfig
index 3bc76bb113a0..953744daff7c 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -203,11 +203,6 @@ config BLK_MQ_PCI
 	depends on BLOCK && PCI
 	default y
 
-config BLK_MQ_VIRTIO
-	bool
-	depends on BLOCK && VIRTIO
-	default y
-
 config BLK_MQ_RDMA
 	bool
 	depends on BLOCK && INFINIBAND
diff --git a/block/Makefile b/block/Makefile
index 1a43750f4b01..709695f54150 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -29,7 +29,6 @@ obj-$(CONFIG_BLK_CMDLINE_PARSER)	+= cmdline-parser.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o
 obj-$(CONFIG_BLK_DEV_INTEGRITY_T10)	+= t10-pi.o
 obj-$(CONFIG_BLK_MQ_PCI)	+= blk-mq-pci.o
-obj-$(CONFIG_BLK_MQ_VIRTIO)	+= blk-mq-virtio.o
 obj-$(CONFIG_BLK_MQ_RDMA)	+= blk-mq-rdma.o
 obj-$(CONFIG_BLK_DEV_ZONED)	+= blk-zoned.o
 obj-$(CONFIG_BLK_WBT)		+= blk-wbt.o
diff --git a/block/blk-mq-virtio.c b/block/blk-mq-virtio.c
deleted file mode 100644
index 488341628256..000000000000
--- a/block/blk-mq-virtio.c
+++ /dev/null
@@ -1,46 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2016 Christoph Hellwig.
- */
-#include <linux/device.h>
-#include <linux/blk-mq.h>
-#include <linux/blk-mq-virtio.h>
-#include <linux/virtio_config.h>
-#include <linux/module.h>
-#include "blk-mq.h"
-
-/**
- * blk_mq_virtio_map_queues - provide a default queue mapping for virtio device
- * @qmap:	CPU to hardware queue map.
- * @vdev:	virtio device to provide a mapping for.
- * @first_vec:	first interrupt vectors to use for queues (usually 0)
- *
- * This function assumes the virtio device @vdev has at least as many available
- * interrupt vetors as @set has queues.  It will then queuery the vector
- * corresponding to each queue for it's affinity mask and built queue mapping
- * that maps a queue to the CPUs that have irq affinity for the corresponding
- * vector.
- */
-int blk_mq_virtio_map_queues(struct blk_mq_queue_map *qmap,
-		struct virtio_device *vdev, int first_vec)
-{
-	const struct cpumask *mask;
-	unsigned int queue, cpu;
-
-	if (!vdev->config->get_vq_affinity)
-		goto fallback;
-
-	for (queue = 0; queue < qmap->nr_queues; queue++) {
-		mask = vdev->config->get_vq_affinity(vdev, first_vec + queue);
-		if (!mask)
-			goto fallback;
-
-		for_each_cpu(cpu, mask)
-			qmap->mq_map[cpu] = qmap->queue_offset + queue;
-	}
-
-	return 0;
-fallback:
-	return blk_mq_map_queues(qmap);
-}
-EXPORT_SYMBOL_GPL(blk_mq_virtio_map_queues);
diff --git a/include/linux/blk-mq-virtio.h b/include/linux/blk-mq-virtio.h
index 687ae287e1dc..b3ddb8b6da76 100644
--- a/include/linux/blk-mq-virtio.h
+++ b/include/linux/blk-mq-virtio.h
@@ -1,11 +1,46 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2016 Christoph Hellwig.
+ */
 #ifndef _LINUX_BLK_MQ_VIRTIO_H
 #define _LINUX_BLK_MQ_VIRTIO_H
 
-struct blk_mq_queue_map;
-struct virtio_device;
+#include <linux/blk-mq.h>
+#include <linux/virtio_config.h>
 
-int blk_mq_virtio_map_queues(struct blk_mq_queue_map *qmap,
-		struct virtio_device *vdev, int first_vec);
+/**
+ * blk_mq_virtio_map_queues - provide a default queue mapping for virtio device
+ * @qmap:	CPU to hardware queue map.
+ * @vdev:	virtio device to provide a mapping for.
+ * @first_vec:	first interrupt vectors to use for queues (usually 0)
+ *
+ * This function assumes the virtio device @vdev has at least as many available
+ * interrupt vetors as @set has queues.  It will then queuery the vector
+ * corresponding to each queue for it's affinity mask and built queue mapping
+ * that maps a queue to the CPUs that have irq affinity for the corresponding
+ * vector.
+ */
+static inline int blk_mq_virtio_map_queues(struct blk_mq_queue_map *qmap,
+		struct virtio_device *vdev, int first_vec)
+{
+	const struct cpumask *mask;
+	unsigned int queue, cpu;
+
+	if (!vdev->config->get_vq_affinity)
+		goto fallback;
+
+	for (queue = 0; queue < qmap->nr_queues; queue++) {
+		mask = vdev->config->get_vq_affinity(vdev, first_vec + queue);
+		if (!mask)
+			goto fallback;
+
+		for_each_cpu(cpu, mask)
+			qmap->mq_map[cpu] = qmap->queue_offset + queue;
+	}
+
+	return 0;
+fallback:
+	return blk_mq_map_queues(qmap);
+}
 
 #endif /* _LINUX_BLK_MQ_VIRTIO_H */
-- 
2.25.1.481.gfbce0eb801-goog

Re: [PATCH] Inline contents of BLK_MQ_VIRTIO config

[Christoph Hellwig]

On Wed, Mar 11, 2020 at 04:56:53PM -0700, Ram Muthiah wrote:
> The config contains one symbol and is a dep of only two configs.
> Inlined this symbol so that it's built in by the two configs
> which need it and deleted the config.

So now we build the code twice instead of once.  Nevermind that you
have dropped the copyright noticed.  What is the point?

Re: [PATCH] Inline contents of BLK_MQ_VIRTIO config

[Ram Muthiah]

On Thu, Mar 12, 2020 at 1:24 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Mar 11, 2020 at 04:56:53PM -0700, Ram Muthiah wrote:
> > The config contains one symbol and is a dep of only two configs.
> > Inlined this symbol so that it's built in by the two configs
> > which need it and deleted the config.
>
> So now we build the code twice instead of once.  Nevermind that you
> have dropped the copyright noticed.  What is the point?
>

The android kernel team is working towards generating a generic
kernel that can be used across many devices. One of the devices in
question is cuttlefish, a android virtual device that relies on
virtio blk.

The config here, blk_mq_virtio, is needed for virtio-blk but it is
binary. blk-mq-virtio cannot be built into this generic kernel in the
interest of keeping all virtio related configs as modules. (This
compromise is needed to enable all physical devices to run this
generic kernel.)

To fix this problem, there are two paths forward that I see. The
patch proposed fixes the problem by inling the one symbol
blk_mq_virtio has since this symbol is used in just two tristate
virtio configs. Additionally, the symbol is fairly small so this
doesn't seem like a bad solution.

Alternatively, I could modularize blk-mq-virtio. It does look like
this config was meant to be tristate based on the include of the
module header and the export of blk_mq_virtio_map_queues.

If the latter approach is preferred, I'd be happy to resubmit the
patch.

Re: [PATCH] Inline contents of BLK_MQ_VIRTIO config

[Christoph Hellwig]

On Fri, Mar 13, 2020 at 01:50:01PM -0700, Ram Muthiah wrote:
> The config here, blk_mq_virtio, is needed for virtio-blk but it is
> binary. blk-mq-virtio cannot be built into this generic kernel in the
> interest of keeping all virtio related configs as modules. (This
> compromise is needed to enable all physical devices to run this
> generic kernel.)

But that is your personal problem and doesn't otherwise matter.  Stop
having stupid policies based on stupid promises and your will be much
easier.

> To fix this problem,

It is not in any meaningful way a problem.  Stop creating problems where
none actually exist.