LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Inline contents of BLK_MQ_VIRTIO config
@ 2020-03-11 23:56 Ram Muthiah
  2020-03-12  8:24 ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Ram Muthiah @ 2020-03-11 23:56 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, linux-kernel, kernel-team, 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Inline contents of BLK_MQ_VIRTIO config
  2020-03-11 23:56 [PATCH] Inline contents of BLK_MQ_VIRTIO config Ram Muthiah
@ 2020-03-12  8:24 ` Christoph Hellwig
  2020-03-13 20:50   ` Ram Muthiah
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2020-03-12  8:24 UTC (permalink / raw)
  To: Ram Muthiah; +Cc: Jens Axboe, linux-block, linux-kernel, kernel-team

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?


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Inline contents of BLK_MQ_VIRTIO config
  2020-03-12  8:24 ` Christoph Hellwig
@ 2020-03-13 20:50   ` Ram Muthiah
  2020-03-19 10:31     ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Ram Muthiah @ 2020-03-13 20:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, linux-kernel, kernel-team

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Inline contents of BLK_MQ_VIRTIO config
  2020-03-13 20:50   ` Ram Muthiah
@ 2020-03-19 10:31     ` Christoph Hellwig
  0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2020-03-19 10:31 UTC (permalink / raw)
  To: Ram Muthiah
  Cc: Christoph Hellwig, Jens Axboe, linux-block, linux-kernel, kernel-team

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-19 10:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 23:56 [PATCH] Inline contents of BLK_MQ_VIRTIO config Ram Muthiah
2020-03-12  8:24 ` Christoph Hellwig
2020-03-13 20:50   ` Ram Muthiah
2020-03-19 10:31     ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).