* [PATCH 8/9] virtio_blk: use virtio IRQ affinity
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
Use automatic IRQ affinity assignment in the virtio layer if available,
and build the blk-mq queues based on it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/block/virtio_blk.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c54118bdc67d..1028dfeb5a7f 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -5,6 +5,7 @@
#include <linux/hdreg.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/interrupt.h>
#include <linux/virtio.h>
#include <linux/virtio_blk.h>
#include <linux/scatterlist.h>
@@ -12,6 +13,7 @@
#include <scsi/scsi_cmnd.h>
#include <linux/idr.h>
#include <linux/blk-mq.h>
+#include <linux/blk-mq-virtio.h>
#include <linux/numa.h>
#define PART_BITS 4
@@ -385,6 +387,7 @@ static int init_vq(struct virtio_blk *vblk)
struct virtqueue **vqs;
unsigned short num_vqs;
struct virtio_device *vdev = vblk->vdev;
+ struct irq_affinity desc = { 0, };
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_MQ,
struct virtio_blk_config, num_queues,
@@ -412,7 +415,7 @@ static int init_vq(struct virtio_blk *vblk)
/* Discover virtqueues and write information to configuration. */
err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names,
- NULL);
+ &desc);
if (err)
goto out;
@@ -543,10 +546,18 @@ static int virtblk_init_request(void *data, struct request *rq,
return 0;
}
+static int virtblk_map_queues(struct blk_mq_tag_set *set)
+{
+ struct virtio_blk *vblk = set->driver_data;
+
+ return blk_mq_virtio_map_queues(set, vblk->vdev, 0);
+}
+
static struct blk_mq_ops virtio_mq_ops = {
.queue_rq = virtio_queue_rq,
.complete = virtblk_request_done,
.init_request = virtblk_init_request,
+ .map_queues = virtblk_map_queues,
};
static unsigned int virtblk_queue_depth;
--
2.11.0
^ permalink raw reply related
* [PATCH 7/9] blk-mq: provide a default queue mapping for virtio device
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
Similar to the PCI version, just calling into virtio instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/Kconfig | 5 ++++
block/Makefile | 1 +
block/blk-mq-virtio.c | 54 +++++++++++++++++++++++++++++++++++++++++++
include/linux/blk-mq-virtio.h | 10 ++++++++
4 files changed, 70 insertions(+)
create mode 100644 block/blk-mq-virtio.c
create mode 100644 include/linux/blk-mq-virtio.h
diff --git a/block/Kconfig b/block/Kconfig
index 8bf114a3858a..3523b4f0cd8b 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -165,4 +165,9 @@ config BLK_MQ_PCI
depends on BLOCK && PCI
default y
+config BLK_MQ_VIRTIO
+ bool
+ depends on BLOCK && VIRTIO
+ default y
+
source block/Kconfig.iosched
diff --git a/block/Makefile b/block/Makefile
index a827f988c4e6..60691949d28d 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -23,5 +23,6 @@ obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o
obj-$(CONFIG_BLK_CMDLINE_PARSER) += cmdline-parser.o
obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o t10-pi.o
obj-$(CONFIG_BLK_MQ_PCI) += blk-mq-pci.o
+obj-$(CONFIG_BLK_MQ_VIRTIO) += blk-mq-virtio.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
new file mode 100644
index 000000000000..c3afbca11299
--- /dev/null
+++ b/block/blk-mq-virtio.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2016 Christoph Hellwig.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+#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
+ * @set: tagset to provide the mapping for
+ * @vdev: virtio device associated with @set.
+ * @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_tag_set *set,
+ 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 < set->nr_hw_queues; queue++) {
+ mask = vdev->config->get_vq_affinity(vdev, first_vec + queue);
+ if (!mask)
+ goto fallback;
+
+ for_each_cpu(cpu, mask)
+ set->mq_map[cpu] = queue;
+ }
+
+ return 0;
+fallback:
+ return blk_mq_map_queues(set);
+}
+EXPORT_SYMBOL_GPL(blk_mq_virtio_map_queues);
diff --git a/include/linux/blk-mq-virtio.h b/include/linux/blk-mq-virtio.h
new file mode 100644
index 000000000000..b1ef6e14744f
--- /dev/null
+++ b/include/linux/blk-mq-virtio.h
@@ -0,0 +1,10 @@
+#ifndef _LINUX_BLK_MQ_VIRTIO_H
+#define _LINUX_BLK_MQ_VIRTIO_H
+
+struct blk_mq_tag_set;
+struct virtio_device;
+
+int blk_mq_virtio_map_queues(struct blk_mq_tag_set *set,
+ struct virtio_device *vdev, int first_vec);
+
+#endif /* _LINUX_BLK_MQ_VIRTIO_H */
--
2.11.0
^ permalink raw reply related
* [PATCH 6/9] virtio: provide a method to get the IRQ affinity mask for a virtqueue
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
This basically passed up the pci_irq_get_affinity information through
virtio through an optional get_vq_affinity method. It is only implemented
by the PCI backend for now, and only when we use per-virtqueue IRQs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 11 +++++++++++
drivers/virtio/virtio_pci_common.h | 2 ++
drivers/virtio/virtio_pci_legacy.c | 1 +
drivers/virtio/virtio_pci_modern.c | 2 ++
include/linux/virtio_config.h | 3 +++
5 files changed, 19 insertions(+)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 7902e920fc73..df548a6fb844 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -361,6 +361,17 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
return 0;
}
+const struct cpumask *vp_get_vq_affinity(struct virtio_device *vdev, int index)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ unsigned int *map = vp_dev->msix_vector_map;
+
+ if (!map || map[index] == VIRTIO_MSI_NO_VECTOR)
+ return NULL;
+
+ return pci_irq_get_affinity(vp_dev->pci_dev, map[index]);
+}
+
#ifdef CONFIG_PM_SLEEP
static int virtio_pci_freeze(struct device *dev)
{
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index a6ad9ec6baef..ac8c9d788964 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -108,6 +108,8 @@ const char *vp_bus_name(struct virtio_device *vdev);
*/
int vp_set_vq_affinity(struct virtqueue *vq, int cpu);
+const struct cpumask *vp_get_vq_affinity(struct virtio_device *vdev, int index);
+
#if IS_ENABLED(CONFIG_VIRTIO_PCI_LEGACY)
int virtio_pci_legacy_probe(struct virtio_pci_device *);
void virtio_pci_legacy_remove(struct virtio_pci_device *);
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 2ab6aee51bf6..f7362c5fe18a 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -190,6 +190,7 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
.finalize_features = vp_finalize_features,
.bus_name = vp_bus_name,
.set_vq_affinity = vp_set_vq_affinity,
+ .get_vq_affinity = vp_get_vq_affinity,
};
/* the PCI probing function */
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index a7a0981e441c..7bc3004b840e 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -437,6 +437,7 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
.finalize_features = vp_finalize_features,
.bus_name = vp_bus_name,
.set_vq_affinity = vp_set_vq_affinity,
+ .get_vq_affinity = vp_get_vq_affinity,
};
static const struct virtio_config_ops virtio_pci_config_ops = {
@@ -452,6 +453,7 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
.finalize_features = vp_finalize_features,
.bus_name = vp_bus_name,
.set_vq_affinity = vp_set_vq_affinity,
+ .get_vq_affinity = vp_get_vq_affinity,
};
/**
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 2ebe506fe41a..8355bab175e1 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -58,6 +58,7 @@ struct irq_affinity;
* This returns a pointer to the bus name a la pci_name from which
* the caller can then copy.
* @set_vq_affinity: set the affinity for a virtqueue.
+ * @get_vq_affinity: get the affinity for a virtqueue (optional).
*/
typedef void vq_callback_t(struct virtqueue *);
struct virtio_config_ops {
@@ -77,6 +78,8 @@ struct virtio_config_ops {
int (*finalize_features)(struct virtio_device *vdev);
const char *(*bus_name)(struct virtio_device *vdev);
int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
+ const struct cpumask *(*get_vq_affinity)(struct virtio_device *vdev,
+ int index);
};
/* If driver didn't advertise the feature, it will never appear. */
--
2.11.0
^ permalink raw reply related
* [PATCH 5/9] virtio: allow drivers to request IRQ affinity when creating VQs
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
Add a struct irq_affinity pointer to the find_vqs methods, which if set
is used to tell the PCI layer to create the MSI-X vectors for our I/O
virtqueues with the proper affinity from the start. Compared to after
the fact affinity hints this gives us an instantly working setup and
allows to allocate the irq descritors node-local and avoid interconnect
traffic. Last but not least this will allow blk-mq queues are created
based on the interrupt affinity for storage drivers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Wang <jasowang@redhat.com>
---
drivers/block/virtio_blk.c | 3 ++-
drivers/char/virtio_console.c | 2 +-
drivers/crypto/virtio/virtio_crypto_core.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_kms.c | 2 +-
drivers/misc/mic/vop/vop_main.c | 2 +-
drivers/net/caif/caif_virtio.c | 3 ++-
drivers/net/virtio_net.c | 2 +-
drivers/remoteproc/remoteproc_virtio.c | 3 ++-
drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
drivers/s390/virtio/kvm_virtio.c | 3 ++-
drivers/s390/virtio/virtio_ccw.c | 3 ++-
drivers/scsi/virtio_scsi.c | 3 ++-
drivers/virtio/virtio_balloon.c | 3 ++-
drivers/virtio/virtio_input.c | 3 ++-
drivers/virtio/virtio_mmio.c | 3 ++-
drivers/virtio/virtio_pci_common.c | 19 ++++++++++++-------
drivers/virtio/virtio_pci_common.h | 5 ++---
drivers/virtio/virtio_pci_modern.c | 7 +++----
include/linux/virtio_config.h | 9 +++++----
net/vmw_vsock/virtio_transport.c | 3 ++-
20 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 10332c24f961..c54118bdc67d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -411,7 +411,8 @@ static int init_vq(struct virtio_blk *vblk)
}
/* Discover virtqueues and write information to configuration. */
- err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names);
+ err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names,
+ NULL);
if (err)
goto out;
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 17857beb4892..6266c0568e1d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1939,7 +1939,7 @@ static int init_vqs(struct ports_device *portdev)
/* Find the queues. */
err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
io_callbacks,
- (const char **)io_names);
+ (const char **)io_names, NULL);
if (err)
goto free;
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index fe70ec823b27..0aa2f045543b 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -119,7 +119,7 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
}
ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
- names);
+ names, NULL);
if (ret)
goto err_find;
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 1235519853f4..e975fa5b0a32 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -172,7 +172,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
vgdev->has_virgl_3d ? "enabled" : "not available");
ret = vgdev->vdev->config->find_vqs(vgdev->vdev, 2, vqs,
- callbacks, names);
+ callbacks, names, NULL);
if (ret) {
DRM_ERROR("failed to find virt queues\n");
goto err_vqs;
diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
index 1a2b67f3183d..c2e29d7f0de8 100644
--- a/drivers/misc/mic/vop/vop_main.c
+++ b/drivers/misc/mic/vop/vop_main.c
@@ -374,7 +374,7 @@ static struct virtqueue *vop_find_vq(struct virtio_device *dev,
static int vop_find_vqs(struct virtio_device *dev, unsigned nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
- const char * const names[])
+ const char * const names[], struct irq_affinity *desc)
{
struct _vop_vdev *vdev = to_vopvdev(dev);
struct vop_device *vpdev = vdev->vpdev;
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index b306210b02b7..bc0eb47eccee 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -679,7 +679,8 @@ static int cfv_probe(struct virtio_device *vdev)
goto err;
/* Get the TX virtio ring. This is a "guest side vring". */
- err = vdev->config->find_vqs(vdev, 1, &cfv->vq_tx, &vq_cbs, &names);
+ err = vdev->config->find_vqs(vdev, 1, &cfv->vq_tx, &vq_cbs, &names,
+ NULL);
if (err)
goto err;
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 765c2d6358da..9be74c2dfb22 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2003,7 +2003,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
}
ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
- names);
+ names, NULL);
if (ret)
goto err_find;
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index 364411fb7734..0142cc3f0c91 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -137,7 +137,8 @@ static void rproc_virtio_del_vqs(struct virtio_device *vdev)
static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
- const char * const names[])
+ const char * const names[],
+ struct irq_affinity *desc)
{
int i, ret;
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 3090b0d3072f..5e66e081027e 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -869,7 +869,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
init_waitqueue_head(&vrp->sendq);
/* We expect two virtqueues, rx and tx (and in this order) */
- err = vdev->config->find_vqs(vdev, 2, vqs, vq_cbs, names);
+ err = vdev->config->find_vqs(vdev, 2, vqs, vq_cbs, names, NULL);
if (err)
goto free_vrp;
diff --git a/drivers/s390/virtio/kvm_virtio.c b/drivers/s390/virtio/kvm_virtio.c
index 5e5c11f37b24..2ce0b3eb2efe 100644
--- a/drivers/s390/virtio/kvm_virtio.c
+++ b/drivers/s390/virtio/kvm_virtio.c
@@ -255,7 +255,8 @@ static void kvm_del_vqs(struct virtio_device *vdev)
static int kvm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
- const char * const names[])
+ const char * const names[],
+ struct irq_affinity *desc)
{
struct kvm_device *kdev = to_kvmdev(vdev);
int i;
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 070c4da95f48..304d3b3cbfd3 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -628,7 +628,8 @@ static int virtio_ccw_register_adapter_ind(struct virtio_ccw_device *vcdev,
static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
- const char * const names[])
+ const char * const names[],
+ struct irq_affinity *desc)
{
struct virtio_ccw_device *vcdev = to_vc_device(vdev);
unsigned long *indicatorp = NULL;
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index ec91bd07f00a..32a16293436b 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -932,7 +932,8 @@ static int virtscsi_init(struct virtio_device *vdev,
}
/* Discover virtqueues and write information to configuration. */
- err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names);
+ err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names,
+ NULL);
if (err)
goto out;
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 181793f07852..36c9c8fcb7f8 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -413,7 +413,8 @@ static int init_vqs(struct virtio_balloon *vb)
* optionally stat.
*/
nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
- err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names);
+ err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
+ NULL);
if (err)
return err;
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index 350a2a5a49db..79f1293cda93 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -173,7 +173,8 @@ static int virtinput_init_vqs(struct virtio_input *vi)
static const char * const names[] = { "events", "status" };
int err;
- err = vi->vdev->config->find_vqs(vi->vdev, 2, vqs, cbs, names);
+ err = vi->vdev->config->find_vqs(vi->vdev, 2, vqs, cbs, names,
+ NULL);
if (err)
return err;
vi->evt = vqs[0];
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index c71fde5fe835..ec8ccb21e9b7 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -446,7 +446,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index,
static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
- const char * const names[])
+ const char * const names[],
+ struct irq_affinity *desc)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
unsigned int irq = platform_get_irq(vm_dev->pdev, 0);
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 822f8e5dcee4..7902e920fc73 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -143,22 +143,28 @@ void vp_del_vqs(struct virtio_device *vdev)
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
- const char * const names[])
+ const char * const names[], struct irq_affinity *desc)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
const char *name = dev_name(&vp_dev->vdev.dev);
int i, err = -ENOMEM, allocated_vectors, nvectors;
+ unsigned flags = PCI_IRQ_MSIX;
bool shared = false;
u16 msix_vec;
+ if (desc) {
+ flags |= PCI_IRQ_AFFINITY;
+ desc->pre_vectors++; /* virtio config vector */
+ }
+
nvectors = 1;
for (i = 0; i < nvqs; i++)
if (callbacks[i])
nvectors++;
/* Try one vector per queue first. */
- err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
- PCI_IRQ_MSIX);
+ err = pci_alloc_irq_vectors_affinity(vp_dev->pci_dev, nvectors,
+ nvectors, flags, desc);
if (err < 0) {
/* Fallback to one vector for config, one shared for queues. */
shared = true;
@@ -308,13 +314,12 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
/* the config->find_vqs() implementation */
int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[])
+ struct virtqueue *vqs[], vq_callback_t *callbacks[],
+ const char * const names[], struct irq_affinity *desc)
{
int err;
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, desc);
if (!err)
return 0;
return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 217ca876eed7..a6ad9ec6baef 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -97,9 +97,8 @@ bool vp_notify(struct virtqueue *vq);
void vp_del_vqs(struct virtio_device *vdev);
/* the config->find_vqs() implementation */
int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[]);
+ struct virtqueue *vqs[], vq_callback_t *callbacks[],
+ const char * const names[], struct irq_affinity *desc);
const char *vp_bus_name(struct virtio_device *vdev);
/* Setup the affinity for a virtqueue:
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index e5ce31091953..a7a0981e441c 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -384,13 +384,12 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
}
static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[])
+ struct virtqueue *vqs[], vq_callback_t *callbacks[],
+ const char * const names[], struct irq_affinity *desc)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtqueue *vq;
- int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names);
+ int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, desc);
if (rc)
return rc;
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 26c155bb639b..2ebe506fe41a 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -7,6 +7,8 @@
#include <linux/virtio_byteorder.h>
#include <uapi/linux/virtio_config.h>
+struct irq_affinity;
+
/**
* virtio_config_ops - operations for configuring a virtio device
* @get: read the value of a configuration field
@@ -68,9 +70,8 @@ struct virtio_config_ops {
void (*set_status)(struct virtio_device *vdev, u8 status);
void (*reset)(struct virtio_device *vdev);
int (*find_vqs)(struct virtio_device *, unsigned nvqs,
- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[]);
+ struct virtqueue *vqs[], vq_callback_t *callbacks[],
+ const char * const names[], struct irq_affinity *desc);
void (*del_vqs)(struct virtio_device *);
u64 (*get_features)(struct virtio_device *vdev);
int (*finalize_features)(struct virtio_device *vdev);
@@ -169,7 +170,7 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev,
vq_callback_t *callbacks[] = { c };
const char *names[] = { n };
struct virtqueue *vq;
- int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names);
+ int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names, NULL);
if (err < 0)
return ERR_PTR(err);
return vq;
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 6788264acc63..9d24c0e958b1 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -532,7 +532,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
vsock->vdev = vdev;
ret = vsock->vdev->config->find_vqs(vsock->vdev, VSOCK_VQ_MAX,
- vsock->vqs, callbacks, names);
+ vsock->vqs, callbacks, names,
+ NULL);
if (ret < 0)
goto out;
--
2.11.0
^ permalink raw reply related
* [PATCH 4/9] virtio_pci: simplify MSI-X setup
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
Try to grab the MSI-X vectors early and fall back to the shared one
before doing lots of allocations.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index b83053082875..822f8e5dcee4 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -142,14 +142,13 @@ void vp_del_vqs(struct virtio_device *vdev)
}
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[],
- bool per_vq_vectors)
+ struct virtqueue *vqs[], vq_callback_t *callbacks[],
+ const char * const names[])
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
const char *name = dev_name(&vp_dev->vdev.dev);
int i, err = -ENOMEM, allocated_vectors, nvectors;
+ bool shared = false;
u16 msix_vec;
nvectors = 1;
@@ -157,12 +156,16 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (callbacks[i])
nvectors++;
- if (per_vq_vectors) {
- err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
- PCI_IRQ_MSIX);
- } else {
+ /* Try one vector per queue first. */
+ err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
+ PCI_IRQ_MSIX);
+ if (err < 0) {
+ /* Fallback to one vector for config, one shared for queues. */
+ shared = true;
err = pci_alloc_irq_vectors(vp_dev->pci_dev, 2, 2,
PCI_IRQ_MSIX);
+ if (err < 0)
+ return err;
}
if (err < 0)
return err;
@@ -190,7 +193,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
0, vp_dev->msix_names[0], vp_dev);
if (err)
- goto out_free_irq_vectors;
+ goto out_free_msix_affinity_masks;
/* Verify we had enough resources to assign the vector */
if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
@@ -240,7 +243,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
}
vp_dev->msix_vector_map[i] = msix_vec;
- if (per_vq_vectors)
+ /*
+ * Use a different vector for each queue if they are available,
+ * else share the same vector for all VQs.
+ */
+ if (!shared)
allocated_vectors++;
}
@@ -307,15 +314,9 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
{
int err;
- /* Try MSI-X with one vector per queue. */
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true);
- if (!err)
- return 0;
- /* Fallback: MSI-X with one vector for config, one shared for queues. */
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names);
if (!err)
return 0;
- /* Finally fall back to regular interrupts. */
return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 3/9] virtio_pci: don't duplicate the msix_enable flag in struct pci_dev
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_pci_common.c | 5 ++---
drivers/virtio/virtio_pci_common.h | 2 --
drivers/virtio/virtio_pci_legacy.c | 2 +-
drivers/virtio/virtio_pci_modern.c | 2 +-
include/uapi/linux/virtio_pci.h | 2 +-
5 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 274dc1ff09c0..b83053082875 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -125,7 +125,7 @@ void vp_del_vqs(struct virtio_device *vdev)
vp_remove_vqs(vdev);
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
for (i = 0; i < vp_dev->msix_vectors; i++)
free_cpumask_var(vp_dev->msix_affinity_masks[i]);
@@ -244,7 +244,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
allocated_vectors++;
}
- vp_dev->msix_enabled = 1;
return 0;
out_remove_vqs:
@@ -340,7 +339,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
if (!vq->callback)
return -EINVAL;
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
int vec = vp_dev->msix_vector_map[vq->index];
struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 85593867e712..217ca876eed7 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -64,8 +64,6 @@ struct virtio_pci_device {
/* the IO mapping for the PCI config space */
void __iomem *ioaddr;
- /* MSI-X support */
- int msix_enabled;
cpumask_var_t *msix_affinity_masks;
/* Name strings for interrupts. This size should be enough,
* and I'm too lazy to allocate each name separately. */
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 47292dad0ff9..2ab6aee51bf6 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -165,7 +165,7 @@ static void del_vq(struct virtqueue *vq)
iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
iowrite16(VIRTIO_MSI_NO_VECTOR,
vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
/* Flush the write out to device */
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 00e6fc1df407..e5ce31091953 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -412,7 +412,7 @@ static void del_vq(struct virtqueue *vq)
vp_iowrite16(vq->index, &vp_dev->common->queue_select);
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
&vp_dev->common->queue_msix_vector);
/* Flush the write out to device */
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 90007a1abcab..15b4385a2be1 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -79,7 +79,7 @@
* configuration space */
#define VIRTIO_PCI_CONFIG_OFF(msix_enabled) ((msix_enabled) ? 24 : 20)
/* Deprecated: please use VIRTIO_PCI_CONFIG_OFF instead */
-#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->msix_enabled)
+#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->pci_dev->msix_enabled)
/* Virtio ABI version, this must match exactly */
#define VIRTIO_PCI_ABI_VERSION 0
--
2.11.0
^ permalink raw reply related
* [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
This lets IRQ layer handle dispatching IRQs to separate handlers for the
case where we don't have per-VQ MSI-X vectors, and allows us to greatly
simplify the code based on the assumption that we always have interrupt
vector 0 (legacy INTx or config interrupt for MSI-X) available, and
any other interrupt is request/freed throught the VQ, even if the
actual interrupt line might be shared in some cases.
This allows removing a great deal of variables keeping track of the
interrupt state in struct virtio_pci_device, as we can now simply walk the
list of VQs and deal with per-VQ interrupt handlers there, and only treat
vector 0 special.
Additionally clean up the VQ allocation code to properly unwind on error
instead of having a single global cleanup label, which is error prone,
and in this case also leads to more code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 235 ++++++++++++++++---------------------
drivers/virtio/virtio_pci_common.h | 16 +--
2 files changed, 106 insertions(+), 145 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index a33767318cbf..274dc1ff09c0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -33,10 +33,8 @@ void vp_synchronize_vectors(struct virtio_device *vdev)
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
int i;
- if (vp_dev->intx_enabled)
- synchronize_irq(vp_dev->pci_dev->irq);
-
- for (i = 0; i < vp_dev->msix_vectors; ++i)
+ synchronize_irq(pci_irq_vector(vp_dev->pci_dev, 0));
+ for (i = 1; i < vp_dev->msix_vectors; i++)
synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i));
}
@@ -99,77 +97,10 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
return vp_vring_interrupt(irq, opaque);
}
-static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
- bool per_vq_vectors)
-{
- struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- const char *name = dev_name(&vp_dev->vdev.dev);
- unsigned i, v;
- int err = -ENOMEM;
-
- vp_dev->msix_vectors = nvectors;
-
- vp_dev->msix_names = kmalloc(nvectors * sizeof *vp_dev->msix_names,
- GFP_KERNEL);
- if (!vp_dev->msix_names)
- goto error;
- vp_dev->msix_affinity_masks
- = kzalloc(nvectors * sizeof *vp_dev->msix_affinity_masks,
- GFP_KERNEL);
- if (!vp_dev->msix_affinity_masks)
- goto error;
- for (i = 0; i < nvectors; ++i)
- if (!alloc_cpumask_var(&vp_dev->msix_affinity_masks[i],
- GFP_KERNEL))
- goto error;
-
- err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
- PCI_IRQ_MSIX);
- if (err < 0)
- goto error;
- vp_dev->msix_enabled = 1;
-
- /* Set the vector used for configuration */
- v = vp_dev->msix_used_vectors;
- snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
- "%s-config", name);
- err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
- vp_config_changed, 0, vp_dev->msix_names[v],
- vp_dev);
- if (err)
- goto error;
- ++vp_dev->msix_used_vectors;
-
- v = vp_dev->config_vector(vp_dev, v);
- /* Verify we had enough resources to assign the vector */
- if (v == VIRTIO_MSI_NO_VECTOR) {
- err = -EBUSY;
- goto error;
- }
-
- if (!per_vq_vectors) {
- /* Shared vector for all VQs */
- v = vp_dev->msix_used_vectors;
- snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
- "%s-virtqueues", name);
- err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
- vp_vring_interrupt, 0, vp_dev->msix_names[v],
- vp_dev);
- if (err)
- goto error;
- ++vp_dev->msix_used_vectors;
- }
- return 0;
-error:
- return err;
-}
-
-/* the config->del_vqs() implementation */
-void vp_del_vqs(struct virtio_device *vdev)
+static void vp_remove_vqs(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtqueue *vq, *n;
- int i;
list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
if (vp_dev->msix_vector_map) {
@@ -181,35 +112,33 @@ void vp_del_vqs(struct virtio_device *vdev)
}
vp_dev->del_vq(vq);
}
+}
- if (vp_dev->intx_enabled) {
- free_irq(vp_dev->pci_dev->irq, vp_dev);
- vp_dev->intx_enabled = 0;
- }
+/* the config->del_vqs() implementation */
+void vp_del_vqs(struct virtio_device *vdev)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ int i;
- for (i = 0; i < vp_dev->msix_used_vectors; ++i)
- free_irq(pci_irq_vector(vp_dev->pci_dev, i), vp_dev);
+ if (WARN_ON_ONCE(list_empty_careful(&vdev->vqs)))
+ return;
- for (i = 0; i < vp_dev->msix_vectors; i++)
- if (vp_dev->msix_affinity_masks[i])
- free_cpumask_var(vp_dev->msix_affinity_masks[i]);
+ vp_remove_vqs(vdev);
if (vp_dev->msix_enabled) {
+ for (i = 0; i < vp_dev->msix_vectors; i++)
+ free_cpumask_var(vp_dev->msix_affinity_masks[i]);
+
/* Disable the vector used for configuration */
vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
- pci_free_irq_vectors(vp_dev->pci_dev);
- vp_dev->msix_enabled = 0;
+ kfree(vp_dev->msix_affinity_masks);
+ kfree(vp_dev->msix_names);
+ kfree(vp_dev->msix_vector_map);
}
- vp_dev->msix_vectors = 0;
- vp_dev->msix_used_vectors = 0;
- kfree(vp_dev->msix_names);
- vp_dev->msix_names = NULL;
- kfree(vp_dev->msix_affinity_masks);
- vp_dev->msix_affinity_masks = NULL;
- kfree(vp_dev->msix_vector_map);
- vp_dev->msix_vector_map = NULL;
+ free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
+ pci_free_irq_vectors(vp_dev->pci_dev);
}
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
@@ -219,79 +148,122 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
bool per_vq_vectors)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ const char *name = dev_name(&vp_dev->vdev.dev);
+ int i, err = -ENOMEM, allocated_vectors, nvectors;
u16 msix_vec;
- int i, err, nvectors, allocated_vectors;
+
+ nvectors = 1;
+ for (i = 0; i < nvqs; i++)
+ if (callbacks[i])
+ nvectors++;
if (per_vq_vectors) {
- /* Best option: one for change interrupt, one per vq. */
- nvectors = 1;
- for (i = 0; i < nvqs; ++i)
- if (callbacks[i])
- ++nvectors;
+ err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
+ PCI_IRQ_MSIX);
} else {
- /* Second best: one for change, shared for all vqs. */
- nvectors = 2;
+ err = pci_alloc_irq_vectors(vp_dev->pci_dev, 2, 2,
+ PCI_IRQ_MSIX);
}
+ if (err < 0)
+ return err;
- err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors);
+ vp_dev->msix_vectors = nvectors;
+ vp_dev->msix_names = kmalloc_array(nvectors,
+ sizeof(*vp_dev->msix_names), GFP_KERNEL);
+ if (!vp_dev->msix_names)
+ goto out_free_irq_vectors;
+
+ vp_dev->msix_affinity_masks = kcalloc(nvectors,
+ sizeof(*vp_dev->msix_affinity_masks), GFP_KERNEL);
+ if (!vp_dev->msix_affinity_masks)
+ goto out_free_msix_names;
+
+ for (i = 0; i < nvectors; ++i) {
+ if (!alloc_cpumask_var(&vp_dev->msix_affinity_masks[i],
+ GFP_KERNEL))
+ goto out_free_msix_affinity_masks;
+ }
+
+ /* Set the vector used for configuration */
+ snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
+ "%s-config", name);
+ err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
+ 0, vp_dev->msix_names[0], vp_dev);
if (err)
- goto error_find;
+ goto out_free_irq_vectors;
- if (per_vq_vectors) {
- vp_dev->msix_vector_map = kmalloc_array(nvqs,
- sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
- if (!vp_dev->msix_vector_map)
- goto error_find;
+ /* Verify we had enough resources to assign the vector */
+ if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
+ err = -EBUSY;
+ goto out_free_config_irq;
}
- allocated_vectors = vp_dev->msix_used_vectors;
+ vp_dev->msix_vector_map = kmalloc_array(nvqs,
+ sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
+ if (!vp_dev->msix_vector_map)
+ goto out_disable_config_irq;
+
+ allocated_vectors = 1; /* vector 0 is the config interrupt */
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
vqs[i] = NULL;
continue;
}
- if (!callbacks[i])
- msix_vec = VIRTIO_MSI_NO_VECTOR;
- else if (per_vq_vectors)
- msix_vec = allocated_vectors++;
+ if (callbacks[i])
+ msix_vec = allocated_vectors;
else
- msix_vec = VP_MSIX_VQ_VECTOR;
+ msix_vec = VIRTIO_MSI_NO_VECTOR;
+
vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
msix_vec);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
- goto error_find;
+ goto out_remove_vqs;
}
- if (!per_vq_vectors)
- continue;
-
if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
continue;
}
- /* allocate per-vq irq if available and necessary */
- snprintf(vp_dev->msix_names[msix_vec],
- sizeof *vp_dev->msix_names,
- "%s-%s",
+ snprintf(vp_dev->msix_names[i + 1],
+ sizeof(*vp_dev->msix_names), "%s-%s",
dev_name(&vp_dev->vdev.dev), names[i]);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
- vring_interrupt, 0,
- vp_dev->msix_names[msix_vec],
- vqs[i]);
+ vring_interrupt, IRQF_SHARED,
+ vp_dev->msix_names[i + 1], vqs[i]);
if (err) {
/* don't free this irq on error */
vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
- goto error_find;
+ goto out_remove_vqs;
}
vp_dev->msix_vector_map[i] = msix_vec;
+
+ if (per_vq_vectors)
+ allocated_vectors++;
}
+
+ vp_dev->msix_enabled = 1;
return 0;
-error_find:
- vp_del_vqs(vdev);
+out_remove_vqs:
+ vp_remove_vqs(vdev);
+ kfree(vp_dev->msix_vector_map);
+out_disable_config_irq:
+ vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
+out_free_config_irq:
+ free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
+out_free_msix_affinity_masks:
+ for (i = 0; i < nvectors; i++) {
+ if (vp_dev->msix_affinity_masks[i])
+ free_cpumask_var(vp_dev->msix_affinity_masks[i]);
+ }
+ kfree(vp_dev->msix_affinity_masks);
+out_free_msix_names:
+ kfree(vp_dev->msix_names);
+out_free_irq_vectors:
+ pci_free_irq_vectors(vp_dev->pci_dev);
return err;
}
@@ -305,9 +277,8 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED,
dev_name(&vdev->dev), vp_dev);
if (err)
- goto out_del_vqs;
+ return err;
- vp_dev->intx_enabled = 1;
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
vqs[i] = NULL;
@@ -317,13 +288,15 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
VIRTIO_MSI_NO_VECTOR);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
- goto out_del_vqs;
+ goto out_remove_vqs;
}
}
return 0;
-out_del_vqs:
- vp_del_vqs(vdev);
+
+out_remove_vqs:
+ vp_remove_vqs(vdev);
+ free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
return err;
}
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 2038887bdf23..85593867e712 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -66,16 +66,12 @@ struct virtio_pci_device {
/* MSI-X support */
int msix_enabled;
- int intx_enabled;
cpumask_var_t *msix_affinity_masks;
/* Name strings for interrupts. This size should be enough,
* and I'm too lazy to allocate each name separately. */
char (*msix_names)[256];
- /* Number of available vectors */
- unsigned msix_vectors;
- /* Vectors allocated, excluding per-vq vectors if any */
- unsigned msix_used_vectors;
-
+ /* Total Number of MSI-X vectors (including per-VQ ones). */
+ int msix_vectors;
/* Map of per-VQ MSI-X vectors, may be NULL */
unsigned *msix_vector_map;
@@ -89,14 +85,6 @@ struct virtio_pci_device {
u16 (*config_vector)(struct virtio_pci_device *vp_dev, u16 vector);
};
-/* Constants for MSI-X */
-/* Use first vector for configuration changes, second and the rest for
- * virtqueues Thus, we need at least 2 vectors for MSI. */
-enum {
- VP_MSIX_CONFIG_VECTOR = 0,
- VP_MSIX_VQ_VECTOR = 1,
-};
-
/* Convert a generic virtio device to our structure */
static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
{
--
2.11.0
^ permalink raw reply related
* [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
In-Reply-To: <20170205171526.6224-1-hch@lst.de>
We don't really need struct virtio_pci_vq_info, as most field in there
are redundant:
- the vq backpointer is not strictly neede to start with
- the entry in the vqs list is not needed - the generic virtqueue already
has list, we only need to check if it has a callback to get the same
semantics
- we can use a simple array to look up the MSI-X vec if needed.
- That simple array now also duoble serves to replace the per_vq_vectors
flag
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 117 +++++++++++--------------------------
drivers/virtio/virtio_pci_common.h | 25 +-------
drivers/virtio/virtio_pci_legacy.c | 6 +-
drivers/virtio/virtio_pci_modern.c | 6 +-
4 files changed, 39 insertions(+), 115 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 186cbab327b8..a33767318cbf 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -62,16 +62,13 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
{
struct virtio_pci_device *vp_dev = opaque;
- struct virtio_pci_vq_info *info;
irqreturn_t ret = IRQ_NONE;
- unsigned long flags;
+ struct virtqueue *vq;
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_for_each_entry(info, &vp_dev->virtqueues, node) {
- if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+ list_for_each_entry(vq, &vp_dev->vdev.vqs, list) {
+ if (vq->callback && vring_interrupt(irq, vq) == IRQ_HANDLED)
ret = IRQ_HANDLED;
}
- spin_unlock_irqrestore(&vp_dev->lock, flags);
return ret;
}
@@ -167,55 +164,6 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
return err;
}
-static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned index,
- void (*callback)(struct virtqueue *vq),
- const char *name,
- u16 msix_vec)
-{
- struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_vq_info *info = kmalloc(sizeof *info, GFP_KERNEL);
- struct virtqueue *vq;
- unsigned long flags;
-
- /* fill out our structure that represents an active queue */
- if (!info)
- return ERR_PTR(-ENOMEM);
-
- vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, msix_vec);
- if (IS_ERR(vq))
- goto out_info;
-
- info->vq = vq;
- if (callback) {
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_add(&info->node, &vp_dev->virtqueues);
- spin_unlock_irqrestore(&vp_dev->lock, flags);
- } else {
- INIT_LIST_HEAD(&info->node);
- }
-
- vp_dev->vqs[index] = info;
- return vq;
-
-out_info:
- kfree(info);
- return vq;
-}
-
-static void vp_del_vq(struct virtqueue *vq)
-{
- struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
- struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
- unsigned long flags;
-
- spin_lock_irqsave(&vp_dev->lock, flags);
- list_del(&info->node);
- spin_unlock_irqrestore(&vp_dev->lock, flags);
-
- vp_dev->del_vq(info);
- kfree(info);
-}
-
/* the config->del_vqs() implementation */
void vp_del_vqs(struct virtio_device *vdev)
{
@@ -224,16 +172,15 @@ void vp_del_vqs(struct virtio_device *vdev)
int i;
list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
- if (vp_dev->per_vq_vectors) {
- int v = vp_dev->vqs[vq->index]->msix_vector;
+ if (vp_dev->msix_vector_map) {
+ int v = vp_dev->msix_vector_map[vq->index];
if (v != VIRTIO_MSI_NO_VECTOR)
free_irq(pci_irq_vector(vp_dev->pci_dev, v),
vq);
}
- vp_del_vq(vq);
+ vp_dev->del_vq(vq);
}
- vp_dev->per_vq_vectors = false;
if (vp_dev->intx_enabled) {
free_irq(vp_dev->pci_dev->irq, vp_dev);
@@ -261,8 +208,8 @@ void vp_del_vqs(struct virtio_device *vdev)
vp_dev->msix_names = NULL;
kfree(vp_dev->msix_affinity_masks);
vp_dev->msix_affinity_masks = NULL;
- kfree(vp_dev->vqs);
- vp_dev->vqs = NULL;
+ kfree(vp_dev->msix_vector_map);
+ vp_dev->msix_vector_map = NULL;
}
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
@@ -275,10 +222,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
u16 msix_vec;
int i, err, nvectors, allocated_vectors;
- vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
- if (!vp_dev->vqs)
- return -ENOMEM;
-
if (per_vq_vectors) {
/* Best option: one for change interrupt, one per vq. */
nvectors = 1;
@@ -294,7 +237,13 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (err)
goto error_find;
- vp_dev->per_vq_vectors = per_vq_vectors;
+ if (per_vq_vectors) {
+ vp_dev->msix_vector_map = kmalloc_array(nvqs,
+ sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
+ if (!vp_dev->msix_vector_map)
+ goto error_find;
+ }
+
allocated_vectors = vp_dev->msix_used_vectors;
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
@@ -304,19 +253,25 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (!callbacks[i])
msix_vec = VIRTIO_MSI_NO_VECTOR;
- else if (vp_dev->per_vq_vectors)
+ else if (per_vq_vectors)
msix_vec = allocated_vectors++;
else
msix_vec = VP_MSIX_VQ_VECTOR;
- vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
+ vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
+ msix_vec);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
goto error_find;
}
- if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
+ if (!per_vq_vectors)
continue;
+ if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
+ vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
+ continue;
+ }
+
/* allocate per-vq irq if available and necessary */
snprintf(vp_dev->msix_names[msix_vec],
sizeof *vp_dev->msix_names,
@@ -326,8 +281,12 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
vring_interrupt, 0,
vp_dev->msix_names[msix_vec],
vqs[i]);
- if (err)
+ if (err) {
+ /* don't free this irq on error */
+ vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
goto error_find;
+ }
+ vp_dev->msix_vector_map[i] = msix_vec;
}
return 0;
@@ -343,23 +302,18 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
int i, err;
- vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
- if (!vp_dev->vqs)
- return -ENOMEM;
-
err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED,
dev_name(&vdev->dev), vp_dev);
if (err)
goto out_del_vqs;
vp_dev->intx_enabled = 1;
- vp_dev->per_vq_vectors = false;
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
vqs[i] = NULL;
continue;
}
- vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i],
+ vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
VIRTIO_MSI_NO_VECTOR);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
@@ -409,16 +363,15 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
{
struct virtio_device *vdev = vq->vdev;
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
- struct cpumask *mask;
- unsigned int irq;
if (!vq->callback)
return -EINVAL;
if (vp_dev->msix_enabled) {
- mask = vp_dev->msix_affinity_masks[info->msix_vector];
- irq = pci_irq_vector(vp_dev->pci_dev, info->msix_vector);
+ int vec = vp_dev->msix_vector_map[vq->index];
+ struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
+ unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
+
if (cpu == -1)
irq_set_affinity_hint(irq, NULL);
else {
@@ -498,8 +451,6 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
vp_dev->vdev.dev.parent = &pci_dev->dev;
vp_dev->vdev.dev.release = virtio_pci_release_dev;
vp_dev->pci_dev = pci_dev;
- INIT_LIST_HEAD(&vp_dev->virtqueues);
- spin_lock_init(&vp_dev->lock);
/* enable the device */
rc = pci_enable_device(pci_dev);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index b2f666250ae0..2038887bdf23 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -31,17 +31,6 @@
#include <linux/highmem.h>
#include <linux/spinlock.h>
-struct virtio_pci_vq_info {
- /* the actual virtqueue */
- struct virtqueue *vq;
-
- /* the list node for the virtqueues list */
- struct list_head node;
-
- /* MSI-X vector (or none) */
- unsigned msix_vector;
-};
-
/* Our device structure */
struct virtio_pci_device {
struct virtio_device vdev;
@@ -75,13 +64,6 @@ struct virtio_pci_device {
/* the IO mapping for the PCI config space */
void __iomem *ioaddr;
- /* a list of queues so we can dispatch IRQs */
- spinlock_t lock;
- struct list_head virtqueues;
-
- /* array of all queues for house-keeping */
- struct virtio_pci_vq_info **vqs;
-
/* MSI-X support */
int msix_enabled;
int intx_enabled;
@@ -94,16 +76,15 @@ struct virtio_pci_device {
/* Vectors allocated, excluding per-vq vectors if any */
unsigned msix_used_vectors;
- /* Whether we have vector per vq */
- bool per_vq_vectors;
+ /* Map of per-VQ MSI-X vectors, may be NULL */
+ unsigned *msix_vector_map;
struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev,
- struct virtio_pci_vq_info *info,
unsigned idx,
void (*callback)(struct virtqueue *vq),
const char *name,
u16 msix_vec);
- void (*del_vq)(struct virtio_pci_vq_info *info);
+ void (*del_vq)(struct virtqueue *vq);
u16 (*config_vector)(struct virtio_pci_device *vp_dev, u16 vector);
};
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 6d9e5173d5fa..47292dad0ff9 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -112,7 +112,6 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
}
static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
- struct virtio_pci_vq_info *info,
unsigned index,
void (*callback)(struct virtqueue *vq),
const char *name,
@@ -130,8 +129,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
if (!num || ioread32(vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN))
return ERR_PTR(-ENOENT);
- info->msix_vector = msix_vec;
-
/* create the vring */
vq = vring_create_virtqueue(index, num,
VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev,
@@ -162,9 +159,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
return ERR_PTR(err);
}
-static void del_vq(struct virtio_pci_vq_info *info)
+static void del_vq(struct virtqueue *vq)
{
- struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 4bf7ab375894..00e6fc1df407 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -293,7 +293,6 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
}
static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
- struct virtio_pci_vq_info *info,
unsigned index,
void (*callback)(struct virtqueue *vq),
const char *name,
@@ -323,8 +322,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
/* get offset of notification word for this vq */
off = vp_ioread16(&cfg->queue_notify_off);
- info->msix_vector = msix_vec;
-
/* create the vring */
vq = vring_create_virtqueue(index, num,
SMP_CACHE_BYTES, &vp_dev->vdev,
@@ -409,9 +406,8 @@ static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
return 0;
}
-static void del_vq(struct virtio_pci_vq_info *info)
+static void del_vq(struct virtqueue *vq)
{
- struct virtqueue *vq = info->vq;
struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
vp_iowrite16(vq->index, &vp_dev->common->queue_select);
--
2.11.0
^ permalink raw reply related
* automatic IRQ affinity for virtio V3
From: Christoph Hellwig @ 2017-02-05 17:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: axboe, pbonzini, virtualization, linux-block, linux-kernel
Hi Michael, hi Jason,
This patches applies a few cleanups to the virtio PCI interrupt handling
code, and then converts the virtio PCI code to use the automatic MSI-X
vectors spreading, as well as using the information in virtio-blk
and virtio-scsi to automatically align the blk-mq queues to the MSI-X
vectors.
Changes since V2:
- remove a redundant callback check
- calculate ->msix_vectors correctly
- add a few Reviewed-by: tags for Jason
Changes since V1:
- dropped the patches already merged for 4.10-rc
- new patch to remove struct virtio_pci_vq_info
- improve probe error handling by unwinding step by step
- new patch to convert virtio-scsi
^ permalink raw reply
* [PATCH] block: don't try Write Same from __blkdev_issue_zeroout
From: Christoph Hellwig @ 2017-02-05 17:10 UTC (permalink / raw)
To: axboe; +Cc: j-nomura, chaitanya.kulkarni, linux-block
Write Same can return an error asynchronously if it turns out the
underlying SCSI device does not support Write Same, which makes a
proper fallback to other methods in __blkdev_issue_zeroout impossible.
Thus only issue a Write Same from blkdev_issue_zeroout an don't try it
at all from __blkdev_issue_zeroout as a non-invasive workaround.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Junichi Nomura <j-nomura@ce.jp.nec.com>
Fixes: e73c23ff ("block: add async variant of blkdev_issue_zeroout")
---
block/blk-lib.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index f8c82a9b4012..ed1e78e24db0 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -306,11 +306,6 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
if (ret == 0 || (ret && ret != -EOPNOTSUPP))
goto out;
- ret = __blkdev_issue_write_same(bdev, sector, nr_sects, gfp_mask,
- ZERO_PAGE(0), biop);
- if (ret == 0 || (ret && ret != -EOPNOTSUPP))
- goto out;
-
ret = 0;
while (nr_sects != 0) {
bio = next_bio(bio, min(nr_sects, (sector_t)BIO_MAX_PAGES),
@@ -369,6 +364,10 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
return 0;
}
+ if (!blkdev_issue_write_same(bdev, sector, nr_sects, gfp_mask,
+ ZERO_PAGE(0)))
+ return 0;
+
blk_start_plug(&plug);
ret = __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask,
&bio, discard);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 1/6] genirq: allow assigning affinity to present but not online CPUs
From: Christoph Hellwig @ 2017-02-05 16:40 UTC (permalink / raw)
To: Joe Korty
Cc: Christoph Hellwig, Keith Busch, linux-nvme, linux-block,
linux-kernel
In-Reply-To: <20170204015809.GA16203@zipoli.ccur.kvm>
Hi Joe,
On Fri, Feb 03, 2017 at 08:58:09PM -0500, Joe Korty wrote:
> IIRC, some years ago I ran across a customer system where
> the #cpus_present was twice as big as #cpus_possible.
>
> Hyperthreading was turned off in the BIOS so it was not
> entirely out of line for the extra cpus to be declared
> present, even though none of them would ever be available
> for use.
This sounds like a system we should quirk around instead of optimizing
for it. Unless I totally misunderstand the idea behind cpu_possible
and cpu_present.
^ permalink raw reply
* [PATCH 4/4] md: fast clone bio in bio_clone_mddev()
From: Ming Lei @ 2017-02-05 6:22 UTC (permalink / raw)
To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
Christoph Hellwig, NeilBrown
Cc: Ming Lei
In-Reply-To: <1486275733-7268-1-git-send-email-tom.leiming@gmail.com>
Firstly bio_clone_mddev() is used in raid normal I/O and isn't
in resync I/O path.
Secondly all the direct access to bvec table in raid happens on
resync I/O except for write behind of raid1, in which we still
use bio_clone() for allocating new bvec table.
So this patch replaces bio_clone() with bio_clone_fast()
in bio_clone_mddev().
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/md/md.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 704be11355a9..7d176f025add 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -193,10 +193,14 @@ EXPORT_SYMBOL_GPL(bio_alloc_mddev);
struct bio *bio_clone_mddev(struct bio *bio, gfp_t gfp_mask,
struct mddev *mddev)
{
+ struct bio_set *bs;
+
if (!mddev || !mddev->bio_set)
- return bio_clone(bio, gfp_mask);
+ bs = fs_bio_set;
+ else
+ bs = mddev->bio_set;
- return bio_clone_bioset(bio, gfp_mask, mddev->bio_set);
+ return bio_clone_fast(bio, gfp_mask, bs);
}
EXPORT_SYMBOL_GPL(bio_clone_mddev);
--
2.7.4
^ permalink raw reply related
* [PATCH 3/4] md/raid1: use bio_clone_slow_mddev_partial in case of write behind
From: Ming Lei @ 2017-02-05 6:22 UTC (permalink / raw)
To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
Christoph Hellwig, NeilBrown
Cc: Ming Lei
In-Reply-To: <1486275733-7268-1-git-send-email-tom.leiming@gmail.com>
Write behind need to replace pages in bio's bvecs, and we have
to clone a fresh bio with new bvec table, so use the introduced
bio_clone_slow_mddev_partial() for it.
For other bio_clone_mddev() cases, we will use fast clone since
they don't need to touch bvec table.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/md/raid1.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 830ff2b20346..e1c5639febdb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1341,13 +1341,12 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
first_clone = 1;
for (i = 0; i < disks; i++) {
- struct bio *mbio;
+ struct bio *mbio = NULL;
+ int offset;
if (!r1_bio->bios[i])
continue;
- mbio = bio_clone_mddev(bio, GFP_NOIO, mddev);
- bio_trim(mbio, r1_bio->sector - bio->bi_iter.bi_sector,
- max_sectors);
+ offset = r1_bio->sector - bio->bi_iter.bi_sector;
if (first_clone) {
/* do behind I/O ?
@@ -1357,8 +1356,14 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
if (bitmap &&
(atomic_read(&bitmap->behind_writes)
< mddev->bitmap_info.max_write_behind) &&
- !waitqueue_active(&bitmap->behind_wait))
+ !waitqueue_active(&bitmap->behind_wait)) {
+ mbio = bio_clone_slow_mddev_partial(bio,
+ GFP_NOIO,
+ mddev,
+ offset,
+ max_sectors);
alloc_behind_pages(mbio, r1_bio);
+ }
bitmap_startwrite(bitmap, r1_bio->sector,
r1_bio->sectors,
@@ -1366,6 +1371,12 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
&r1_bio->state));
first_clone = 0;
}
+
+ if (!mbio) {
+ mbio = bio_clone_mddev(bio, GFP_NOIO, mddev);
+ bio_trim(mbio, offset, max_sectors);
+ }
+
if (r1_bio->behind_bvecs) {
struct bio_vec *bvec;
int j;
--
2.7.4
^ permalink raw reply related
* [PATCH 2/4] md: introduce bio_clone_slow_mddev_partial()
From: Ming Lei @ 2017-02-05 6:22 UTC (permalink / raw)
To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
Christoph Hellwig, NeilBrown
Cc: Ming Lei
In-Reply-To: <1486275733-7268-1-git-send-email-tom.leiming@gmail.com>
In raid1/raid10, most users of bio_clone_mddev() need bio_trim() too,
that means only part of the bio is required to be cloned, and not
necessary to clone the whole bio each time, and it is just enough
to clone the specified bvecs range.
So this patch introduces bio_clone_slow_mddev_partial() to improve
the situation, and it is named as slow because the following patch
will switch to bio_clone_fast() in bio_clone_mddev().
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/md/md.c | 16 ++++++++++++++++
drivers/md/md.h | 3 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 4c1b82defa78..704be11355a9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -200,6 +200,22 @@ struct bio *bio_clone_mddev(struct bio *bio, gfp_t gfp_mask,
}
EXPORT_SYMBOL_GPL(bio_clone_mddev);
+struct bio *bio_clone_slow_mddev_partial(struct bio *bio, gfp_t gfp_mask,
+ struct mddev *mddev, int offset,
+ int size)
+{
+ struct bio_set *bs;
+
+ if (!mddev || !mddev->bio_set)
+ bs = fs_bio_set;
+ else
+ bs = mddev->bio_set;
+
+ return bio_clone_bioset_partial(bio, gfp_mask, bs, offset << 9,
+ size << 9);
+}
+EXPORT_SYMBOL_GPL(bio_clone_slow_mddev_partial);
+
/*
* We have a system wide 'event count' that is incremented
* on any 'interesting' event, and readers of /proc/mdstat
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 968bbe72b237..4f4e6ded59e5 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -675,6 +675,9 @@ extern void mddev_suspend(struct mddev *mddev);
extern void mddev_resume(struct mddev *mddev);
extern struct bio *bio_clone_mddev(struct bio *bio, gfp_t gfp_mask,
struct mddev *mddev);
+extern struct bio *bio_clone_slow_mddev_partial(struct bio *bio, gfp_t gfp_mask,
+ struct mddev *mddev, int offset,
+ int size);
extern struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
struct mddev *mddev);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/4] block: introduce bio_clone_bioset_partial()
From: Ming Lei @ 2017-02-05 6:22 UTC (permalink / raw)
To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
Christoph Hellwig, NeilBrown
Cc: Ming Lei
In-Reply-To: <1486275733-7268-1-git-send-email-tom.leiming@gmail.com>
md still need bio clone(not the fast version) for behind write,
and it is more efficient to use bio_clone_bioset_partial().
The idea is simple and just copy the bvecs range specified from
parameters.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
block/bio.c | 61 +++++++++++++++++++++++++++++++++++++++++------------
include/linux/bio.h | 11 ++++++++--
2 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 4b564d0c3e29..5eec5e08417f 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -625,21 +625,20 @@ struct bio *bio_clone_fast(struct bio *bio, gfp_t gfp_mask, struct bio_set *bs)
}
EXPORT_SYMBOL(bio_clone_fast);
-/**
- * bio_clone_bioset - clone a bio
- * @bio_src: bio to clone
- * @gfp_mask: allocation priority
- * @bs: bio_set to allocate from
- *
- * Clone bio. Caller will own the returned bio, but not the actual data it
- * points to. Reference count of returned bio will be one.
- */
-struct bio *bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
- struct bio_set *bs)
+static struct bio *__bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
+ struct bio_set *bs, int offset,
+ int size)
{
struct bvec_iter iter;
struct bio_vec bv;
struct bio *bio;
+ struct bvec_iter iter_src = bio_src->bi_iter;
+
+ /* for supporting partial clone */
+ if (offset || size != bio_src->bi_iter.bi_size) {
+ bio_advance_iter(bio_src, &iter_src, offset);
+ iter_src.bi_size = size;
+ }
/*
* Pre immutable biovecs, __bio_clone() used to just do a memcpy from
@@ -663,7 +662,8 @@ struct bio *bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
* __bio_clone_fast() anyways.
*/
- bio = bio_alloc_bioset(gfp_mask, bio_segments(bio_src), bs);
+ bio = bio_alloc_bioset(gfp_mask, __bio_segments(bio_src,
+ &iter_src), bs);
if (!bio)
return NULL;
bio->bi_bdev = bio_src->bi_bdev;
@@ -680,7 +680,7 @@ struct bio *bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
bio->bi_io_vec[bio->bi_vcnt++] = bio_src->bi_io_vec[0];
break;
default:
- bio_for_each_segment(bv, bio_src, iter)
+ __bio_for_each_segment(bv, bio_src, iter, iter_src)
bio->bi_io_vec[bio->bi_vcnt++] = bv;
break;
}
@@ -699,9 +699,44 @@ struct bio *bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
return bio;
}
+
+/**
+ * bio_clone_bioset - clone a bio
+ * @bio_src: bio to clone
+ * @gfp_mask: allocation priority
+ * @bs: bio_set to allocate from
+ *
+ * Clone bio. Caller will own the returned bio, but not the actual data it
+ * points to. Reference count of returned bio will be one.
+ */
+struct bio *bio_clone_bioset(struct bio *bio_src, gfp_t gfp_mask,
+ struct bio_set *bs)
+{
+ return __bio_clone_bioset(bio_src, gfp_mask, bs, 0,
+ bio_src->bi_iter.bi_size);
+}
EXPORT_SYMBOL(bio_clone_bioset);
/**
+ * bio_clone_bioset_partial - clone a partial bio
+ * @bio_src: bio to clone
+ * @gfp_mask: allocation priority
+ * @bs: bio_set to allocate from
+ * @offset: cloned starting from the offset
+ * @size: size for the cloned bio
+ *
+ * Clone bio. Caller will own the returned bio, but not the actual data it
+ * points to. Reference count of returned bio will be one.
+ */
+struct bio *bio_clone_bioset_partial(struct bio *bio_src, gfp_t gfp_mask,
+ struct bio_set *bs, int offset,
+ int size)
+{
+ return __bio_clone_bioset(bio_src, gfp_mask, bs, offset, size);
+}
+EXPORT_SYMBOL(bio_clone_bioset_partial);
+
+/**
* bio_add_pc_page - attempt to add page to bio
* @q: the target queue
* @bio: destination bio
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 7cf8a6c70a3f..8e521194f6fc 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -183,7 +183,7 @@ static inline void bio_advance_iter(struct bio *bio, struct bvec_iter *iter,
#define bio_iter_last(bvec, iter) ((iter).bi_size == (bvec).bv_len)
-static inline unsigned bio_segments(struct bio *bio)
+static inline unsigned __bio_segments(struct bio *bio, struct bvec_iter *bvec)
{
unsigned segs = 0;
struct bio_vec bv;
@@ -205,12 +205,17 @@ static inline unsigned bio_segments(struct bio *bio)
break;
}
- bio_for_each_segment(bv, bio, iter)
+ __bio_for_each_segment(bv, bio, iter, *bvec)
segs++;
return segs;
}
+static inline unsigned bio_segments(struct bio *bio)
+{
+ return __bio_segments(bio, &bio->bi_iter);
+}
+
/*
* get a reference to a bio, so it won't disappear. the intended use is
* something like:
@@ -384,6 +389,8 @@ extern void bio_put(struct bio *);
extern void __bio_clone_fast(struct bio *, struct bio *);
extern struct bio *bio_clone_fast(struct bio *, gfp_t, struct bio_set *);
extern struct bio *bio_clone_bioset(struct bio *, gfp_t, struct bio_set *bs);
+extern struct bio *bio_clone_bioset_partial(struct bio *, gfp_t,
+ struct bio_set *, int, int);
extern struct bio_set *fs_bio_set;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/4] md: use bio_clone_fast()
From: Ming Lei @ 2017-02-05 6:22 UTC (permalink / raw)
To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
Christoph Hellwig, NeilBrown
Cc: Ming Lei
Hi,
This patches replaces bio_clone() with bio_fast_clone() in
bio_clone_mddev() because:
1) bio_clone_mddev() is used in raid normal I/O and isn't in
resync I/O path, and all the direct access to bvec table in
raid happens on resync I/O only except for write behind of raid1.
Write behind is treated specially, so the replacement is safe.
2) for write behind, bio_clone() is kept, but this patchset
introduces bio_clone_bioset_partial() to just clone one specific
bvecs range instead of whole table. Then write behind is improved
too.
Thanks,
Ming
Ming Lei (4):
block: introduce bio_clone_bioset_partial()
md: introduce bio_clone_slow_mddev_partial()
md/raid1: use bio_clone_slow_mddev_partial in case of write behind
md: fast clone bio in bio_clone_mddev()
block/bio.c | 61 +++++++++++++++++++++++++++++++++++++++++------------
drivers/md/md.c | 24 +++++++++++++++++++--
drivers/md/md.h | 3 +++
drivers/md/raid1.c | 21 +++++++++++++-----
include/linux/bio.h | 11 ++++++++--
5 files changed, 98 insertions(+), 22 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v4] dm: don't allow ioctls to targets that don't map to whole devices
From: Jens Axboe @ 2017-02-04 17:20 UTC (permalink / raw)
To: Christoph Hellwig, snitzer; +Cc: agk, pbonzini, dm-devel, linux-block
In-Reply-To: <20170204094503.19322-1-hch@lst.de>
On 02/04/2017 02:45 AM, Christoph Hellwig wrote:
> .. at least for unprivileged users. Before we called into the SCSI
> ioctl code to allow excemptions for a few SCSI passthrough ioctls,
> but this is pretty unsafe and except for this call dm knows nothing
> about SCSI ioctls.
>
> As the SCSI ioctl code is now optional, we really don't want to
> drag it in for DM, and the exception is not very useful anyway.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Mike Snitzer <snitzer@redhat.com>
Added, with the acks/reviews from Johannes and Pablo added.
--
Jens Axboe
^ permalink raw reply
* [PATCH v4] dm: don't allow ioctls to targets that don't map to whole devices
From: Christoph Hellwig @ 2017-02-04 9:45 UTC (permalink / raw)
To: snitzer, axboe; +Cc: agk, pbonzini, dm-devel, linux-block
.. at least for unprivileged users. Before we called into the SCSI
ioctl code to allow excemptions for a few SCSI passthrough ioctls,
but this is pretty unsafe and except for this call dm knows nothing
about SCSI ioctls.
As the SCSI ioctl code is now optional, we really don't want to
drag it in for DM, and the exception is not very useful anyway.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 9e958bc94fed..5bd9ab06a562 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -465,13 +465,16 @@ static int dm_blk_ioctl(struct block_device *bdev, fmode_t mode,
if (r > 0) {
/*
- * Target determined this ioctl is being issued against
- * a logical partition of the parent bdev; so extra
- * validation is needed.
+ * Target determined this ioctl is being issued against a
+ * subset of the parent bdev; require extra privileges.
*/
- r = scsi_verify_blk_ioctl(NULL, cmd);
- if (r)
+ if (!capable(CAP_SYS_RAWIO)) {
+ DMWARN_LIMIT(
+ "%s: sending ioctl %x to DM device without required privilege.",
+ current->comm, cmd);
+ r = -ENOIOCTLCMD;
goto out;
+ }
}
r = __blkdev_driver_ioctl(bdev, mode, cmd, arg);
--
2.11.0
^ permalink raw reply related
* Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
From: Christoph Hellwig @ 2017-02-04 8:48 UTC (permalink / raw)
To: Jens Axboe
Cc: Martin K. Petersen, Christoph Hellwig, Junichi Nomura,
linux-block@vger.kernel.org, linux-scsi, Chaitanya Kulkarni
In-Reply-To: <34e7f917-8fff-d8f9-c498-3431a27113ca@kernel.dk>
On Fri, Feb 03, 2017 at 08:17:10PM -0700, Jens Axboe wrote:
> We're at (almost) -rc7 time, we have to do more than hand wave about
> this. What's the plan for 4.10 final?
I'll send your a fix to revert the async write same changes for now,
as suggested in the last mail from me.
^ permalink raw reply
* Re: Doubt related to immutable biovecs
From: Ming Lei @ 2017-02-04 7:59 UTC (permalink / raw)
To: Suraj Choudhari; +Cc: linux-block
In-Reply-To: <CAGvB2rNtGnbFcconCbG8THjFKCHBj-cjKE9tX61pPLFEYxK0yg@mail.gmail.com>
On Thu, Feb 2, 2017 at 5:35 PM, Suraj Choudhari
<surajschoudhari@gmail.com> wrote:
> Thanks very much Ming and all !
You are welcome!
>
> I referred to multipage bvec patches link and it was indeed good & insightful.
>
> I have few more query related to immutable bvecs & multipage bvecs, in
> particular related to 4.4 kernel --
>
> 1) My understanding is bi_vcnt can't be used for a cloned BIO because
> 'bio_clone_fast' does NOT update the bi_vcnt field in the cloned BIO.
> Is my understanding correct ?
Yes and no.
A cloned BIO may just map to a range of vectors from the source bio via
helpers.
>
> 2) For a block device driver, is it be safe to use the 'bi_vcnt' of a
> BIO that's received from the upper layers, in order to access the
> bvecs array, particularly in the '4.4' kernel ?
No, as we mentioned, it isn't safe to do that, especially after v4.3 introduces
bio splitting, which may cause a block driver to see a cloned bio easily.
Also dm may pass a cloned a bio to block driver too.
So please use bio helpers to iterate the bvec instead of direct access.
> - I am thinking it may be safe to access bi_vcnt as long as the
> block driver has NOT received a CLONE bio from upper layer. The
> reason I think can be bvecs are not multipage in 4.4. Is my
> understanding right ?
Yes, but why not use use the helpers transparently for both cloned bio
and non-cloned?
Thanks,
Ming
>
> Thanks & Regards,
> Suraj
>
>
>
>
> On Fri, Jan 20, 2017 at 9:27 PM, Ming Lei <tom.leiming@gmail.com> wrote:
>> Hi,
>>
>> On Fri, Jan 20, 2017 at 9:41 PM, Suraj Choudhari
>> <surajschoudhari@gmail.com> wrote:
>>> Hello,
>>>
>>> I've some queries related to accessing the 'bio_vec' and 'bi_vcnt'
>>> members in the BIO structure after the kernel changes implemented for
>>> the 'immutable biovecs' -
>>>
>>> Background -
>>> - From the changes done for the immutable biovecs, I understand that
>>> the driver code now no longer needs to reference the 'bi_vcnt' and
>>> 'bi_io_vec' fields directly. Instead we can use the 'bvec_iter'
>>> iterator in order to to access the bio_vec.
>>> - We've iterator functions bio_iter_iovec() and bio_advance_iter() as
>>> well, which return us literal 'struct biovecs' taking into account the
>>> bi_bvec_done and bi_size values.
>>>
>>> Doubts -
>>> 1) Few functions in the block layer and some drivers still directly
>>> refer to the 'bi_vcnt' and 'bio_vec' members of the BIO structure
>>> [instead of accessing bio_vecs using the bvec_iter iterator].
>>> Would there be some changes in the functions to compulsorily use the
>>> 'bvec_iter' in order to access bio_vecs? If yes, are such changes
>>
>> Generally speaking, yes, it is always better to use iterator helpers.
>>
>>> related to the 'multipage biovec' feature ?
>>
>> Yes, it is a must for supporting multipage bvec.
>>
>>>
>>> 2) Can there be some problem if a driver directly accesses the
>>> bi_io_vec [bio_vec] in the bio structure based on the values of bi_idx
>>> and bi_vcnt ? [instead of using the bvec_iter iterator].
>>
>> The direct access can't work any more once switching to multipage bvec,
>> and you can find some hint in the following link:
>>
>> https://lkml.org/lkml/2017/1/15/266
>>
>> Also .bi_vcnt can't be used for BIO_CLONED bio, and it is very easy
>> to make mistakes to access the table directly if someone don't understand
>> the iterator details enough.
>>
>>
>> Thanks,
>> Ming Lei
--
Ming Lei
^ permalink raw reply
* Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
From: Jens Axboe @ 2017-02-04 3:17 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Junichi Nomura, linux-block@vger.kernel.org,
linux-scsi, Chaitanya Kulkarni
In-Reply-To: <yq1wpd6vqpm.fsf@oracle.com>
On 02/03/2017 03:45 PM, Martin K. Petersen wrote:
>>>>>> "Jens" == Jens Axboe <axboe@kernel.dk> writes:
>
>>> I think we should fix sd.c to only send WRITE SAME if either of the
>>> variants are explicitly listed as supported through REPORT SUPPORTED
>>> OPERATION CODES, or maybe through a whitelist if there are important
>>> enough devices.
>
> Jens> Yep
>
> I hate it too. But the reason it's assumed on is that there is
> essentially no heuristic that works. Just like we assume that READ
> always works.
>
> Out of the ~200 devices I have access to in the lab:
>
> - 100% of the SAS/FC disk drives and SSDs support WRITE SAME
> - Only 2 out of about 50 different drive models support RSOC
> - About half of the arrays support WRITE SAME(10/16)
> - None of the arrays I have support RSOC
>
> So even if we were to entertain using RSOC for "enterprise" transport
> classes (which I concur would be nice for other reasons), it wouldn't
> solve the WRITE SAME problem...
We're at (almost) -rc7 time, we have to do more than hand wave about
this. What's the plan for 4.10 final?
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH] blk-mq-sched: separate mark hctx and queue restart operations
From: Omar Sandoval @ 2017-02-03 23:54 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: kernel-team
In-Reply-To: <0e041bc5f968fd15d86f1a89789c30ea82dd96ab.1486150202.git.osandov@fb.com>
On Fri, Feb 03, 2017 at 11:35:58AM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> In blk_mq_sched_dispatch_requests(), we call blk_mq_sched_mark_restart()
> after we dispatch requests left over on our hardware queue dispatch
> list. This is so we'll go back and dispatch requests from the scheduler.
> In this case, it's only necessary to restart the hardware queue that we
> are running; there's no reason to run other hardware queues just because
> we are using shared tags.
>
> So, split out blk_mq_sched_mark_restart() into two operations: the "all"
> variant is for when we really need to mark the request queue as needing
> a restart, and we'll keep the original for just the hardware queue.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> Based on block/for-next. There might be a better name for this.
>
> block/blk-mq-sched.h | 15 +++++++++++++++
> block/blk-mq.c | 2 +-
> 2 files changed, 16 insertions(+), 1 deletion(-)
Actually, I think I want to do this slightly differently, ignore this
for now.
^ permalink raw reply
* Re: [PATCH 11/24] ecryptfs: Convert to separately allocated bdi
From: Tyler Hicks @ 2017-02-03 23:54 UTC (permalink / raw)
To: Jan Kara, linux-fsdevel; +Cc: Christoph Hellwig, linux-block, ecryptfs
In-Reply-To: <20170202173422.3240-12-jack@suse.cz>
[-- Attachment #1.1: Type: text/plain, Size: 1831 bytes --]
On 02/02/2017 11:34 AM, Jan Kara wrote:
> Allocate struct backing_dev_info separately instead of embedding it
> inside the superblock. This unifies handling of bdi among users.
>
> CC: Tyler Hicks <tyhicks@canonical.com>
Looks fine to me.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
> CC: ecryptfs@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/ecryptfs/ecryptfs_kernel.h | 1 -
> fs/ecryptfs/main.c | 4 +---
> 2 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index 599a29237cfe..e93444a4c4b1 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -349,7 +349,6 @@ struct ecryptfs_mount_crypt_stat {
> struct ecryptfs_sb_info {
> struct super_block *wsi_sb;
> struct ecryptfs_mount_crypt_stat mount_crypt_stat;
> - struct backing_dev_info bdi;
> };
>
> /* file private data. */
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index 151872dcc1f4..9014479d0160 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -519,12 +519,11 @@ static struct dentry *ecryptfs_mount(struct file_system_type *fs_type, int flags
> goto out;
> }
>
> - rc = bdi_setup_and_register(&sbi->bdi, "ecryptfs");
> + rc = super_setup_bdi(s);
> if (rc)
> goto out1;
>
> ecryptfs_set_superblock_private(s, sbi);
> - s->s_bdi = &sbi->bdi;
>
> /* ->kill_sb() will take care of sbi after that point */
> sbi = NULL;
> @@ -633,7 +632,6 @@ static void ecryptfs_kill_block_super(struct super_block *sb)
> if (!sb_info)
> return;
> ecryptfs_destroy_mount_crypt_stat(&sb_info->mount_crypt_stat);
> - bdi_destroy(&sb_info->bdi);
> kmem_cache_free(ecryptfs_sb_info_cache, sb_info);
> }
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [REGRESSION v4.10-rc1] blkdev_issue_zeroout() returns -EREMOTEIO on the first call for SCSI device that doesn't support WRITE SAME
From: Martin K. Petersen @ 2017-02-03 22:45 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, Junichi Nomura, linux-block@vger.kernel.org,
linux-scsi, Chaitanya Kulkarni, Martin K. Petersen
In-Reply-To: <1e819f0d-ecdc-e54a-bd3d-17de2f71c8a7@kernel.dk>
>>>>> "Jens" == Jens Axboe <axboe@kernel.dk> writes:
>> I think we should fix sd.c to only send WRITE SAME if either of the
>> variants are explicitly listed as supported through REPORT SUPPORTED
>> OPERATION CODES, or maybe through a whitelist if there are important
>> enough devices.
Jens> Yep
I hate it too. But the reason it's assumed on is that there is
essentially no heuristic that works. Just like we assume that READ
always works.
Out of the ~200 devices I have access to in the lab:
- 100% of the SAS/FC disk drives and SSDs support WRITE SAME
- Only 2 out of about 50 different drive models support RSOC
- About half of the arrays support WRITE SAME(10/16)
- None of the arrays I have support RSOC
So even if we were to entertain using RSOC for "enterprise" transport
classes (which I concur would be nice for other reasons), it wouldn't
solve the WRITE SAME problem...
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v3] dm: don't allow ioctls to targets that don't map to whole devices
From: Bart Van Assche @ 2017-02-03 21:41 UTC (permalink / raw)
To: hch@lst.de, jthumshirn@suse.de, snitzer@redhat.com
Cc: agk@redhat.com, dm-devel@redhat.com, linux-block@vger.kernel.org,
pbonzini@redhat.com, axboe@kernel.dk
In-Reply-To: <4435311b-80e1-8764-fa12-140203e9231f@suse.de>
On Fri, 2017-02-03 at 19:17 +0100, Johannes Thumshirn wrote:
> Forgotten git add? git commit --amend without git add is such a classic=20
> mistake on my side as well :-/
Are you familiar with the -a option of git commit? Just run
git commit -a --amend
Bart.=
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox