vhost-nvme: new qemu nvme backend using nvme target
Subsystems (4)
cover vhost-nvme: new qemu nvme backend using nvme target
Hi,
This is the first attempt to add a new qemu nvme backend using in-kernel nvme target.
Most code are ported from qemu-nvme and also borrow code from Hannes Reinecke's rts-megasas.
It's similar as vhost-scsi, but doesn't use virtio. The advantage is guest can run unmodified NVMe driver. So guest can be any OS that has a NVMe driver.
The goal is to get as good performance as vhost-scsi. But for now, peformance is poor.
MMIO is the bottleneck.
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-ext
Qemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0
Thanks for any comment,
Ming
RFC PATCH 1/9 nvme-vhost: add initial commit
From: Ming Lin <[email protected]>
--- drivers/nvme/target/Kconfig | 11 +++++++++++ drivers/nvme/target/Makefile | 2 ++ drivers/nvme/target/vhost.c | 16 ++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 drivers/nvme/target/vhost.c
drivers/nvme/target/Kconfig +11 -0
@@ -12,3 +12,14 @@ config NVME_TARGET_LOOP to test NVMe host and target side features. If unsure, say N. + +config NVME_TARGET_VHOST + tristate "NVMe vhost support" + depends on BLK_DEV_NVME + select NVME_TARGET + select VHOST + select VHOST_RING + help + This enabled the NVMe vhost support. + + If unsure, say N.
drivers/nvme/target/Makefile +2 -0
@@ -1,6 +1,8 @@ obj-$(CONFIG_NVME_TARGET) += nvmet.o obj-$(CONFIG_NVME_TARGET_LOOP) += nvme-loop.o +obj-$(CONFIG_NVME_TARGET_VHOST) += nvme-vhost.o nvmet-y += core.o configfs.o admin-cmd.o io-cmd.o nvme-loop-y += loop.o +nvme-vhost-y += vhost.o
drivers/nvme/target/vhost.c new file +16 -0
@@ -0,0 +1,16 @@ +#include <linux/module.h> + +static int __init nvmet_vhost_init (void ) +{ +return 0 ; +} +module_init (nvmet_vhost_init); + +static void nvmet_vhost_exit (void ) +{ +} +module_exit (nvmet_vhost_exit); + +MODULE_AUTHOR ("Ming Lin <ming.l at ssi.samsung.com>" ); +MODULE_LICENSE ("GPL v2" ); +
Signature · 1 line
1.9.1
RFC PATCH 2/9 nvme-vhost: add basic ioctl handlers
In reply to Ming LinFrom: Ming Lin <[email protected]>
--- drivers/nvme/target/core.c | 1 + drivers/nvme/target/vhost.c | 264 +++++++++++++++++++++++++++++++++++++++++++- include/uapi/linux/vhost.h | 15 +++ 3 files changed, 279 insertions(+), 1 deletion(-)
drivers/nvme/target/core.c +1 -0
@@ -378,6 +378,7 @@ void nvmet_ctrl_put(struct nvmet_ctrl *ctrl) {kref_put (&ctrl->ref, nvmet_ctrl_free); } +EXPORT_SYMBOL_GPL (nvmet_ctrl_put);struct nvmet_subsys *nvmet_find_subsys (char *subsys_name) {
drivers/nvme/target/vhost.c +263 -1
276 lines not on this page · raw message
include/uapi/linux/vhost.h +15 -0
@@ -169,4 +169,19 @@ struct vhost_scsi_target {#define VHOST_SCSI_SET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x43, __u32)#define VHOST_SCSI_GET_EVENTS_MISSED _IOW(VHOST_VIRTIO, 0x44, __u32) +struct vhost_nvme_target { +char vhost_wwpn[224 ];/* TRANSPORT_IQN_LEN */ +}; + +struct nvmet_vhost_eventfd { +int num; +int fd; +int *irq_enabled; +int *vector; +}; + +#define VHOST_NVME_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x47, struct vhost_nvme_target) +#define VHOST_NVME_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x48, struct vhost_nvme_target) +#define VHOST_NVME_SET_EVENTFD _IOW(VHOST_VIRTIO, 0x45, struct nvmet_vhost_eventfd) +#endif
Signature · 1 line
1.9.1
RFC PATCH 3/9 nvme-vhost: add basic nvme bar read/write
In reply to Ming LinFrom: Ming Lin <[email protected]>
--- drivers/nvme/target/vhost.c | 102 ++++++++++++++++++++++++++++++++++++++++++++ include/uapi/linux/vhost.h | 17 ++++++-- 2 files changed, 116 insertions(+), 3 deletions(-)
drivers/nvme/target/vhost.c +102 -0
@@ -8,6 +8,8 @@#include "../../vhost/vhost.h" #include "nvmet.h" +#define NVMET_VHOST_AQ_DEPTH 256 +struct nvmet_vhost_ctrl_eventfd {struct file *call;struct eventfd_ctx *call_ctx;
@@ -35,6 +37,10 @@ struct nvmet_vhost_ctrl {struct nvmet_vhost_cq **cqs;struct nvmet_vhost_sq **sqs; + +u32 aqa; +u64 asq; +u64 acq; }; static int
@@ -127,6 +133,100 @@ static int nvmet_vhost_set_eventfd(struct nvmet_vhost_ctrl *n, void __user *argpreturn 0 ; } +static int nvmet_vhost_bar_read (struct nvmet_ctrl *ctrl,int offset,u64 *val) +{ +int status =NVME_SC_SUCCESS ; + +switch (offset) { +case NVME_REG_CAP : + *val = ctrl->cap; +break ; +case NVME_REG_CAP +4 : + *val = ctrl->cap >>32 ; +case NVME_REG_VS : + *val = ctrl->subsys->ver; +break ; +case NVME_REG_CC : + *val = ctrl->cc; +break ; +case NVME_REG_CSTS : + *val = ctrl->csts; +break ; +case NVME_REG_AQA : + *val = (NVMET_VHOST_AQ_DEPTH -1 ) | + (((NVMET_VHOST_AQ_DEPTH -1 ) <<16 )); +break ; +default : +printk ("Unknown offset: 0x%x\n" , offset); + status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ; +break ; + } + +return status; +} + +static int nvmet_bar_write (struct nvmet_vhost_ctrl *n,int offset,u64 val) +{ +struct nvmet_ctrl *ctrl = n->ctrl; +int status =NVME_SC_SUCCESS ; + +switch (offset) { +case NVME_REG_CC : +nvmet_update_cc (ctrl, val); +break ; +case NVME_REG_AQA : + n->aqa = val &0xffffffff ; +break ; +case NVME_REG_ASQ : + n->asq = val; +break ; +case NVME_REG_ASQ +4 : + n->asq |= val <<32 ; +break ; +case NVME_REG_ACQ : + n->acq = val; +break ; +case NVME_REG_ACQ +4 : + n->acq |= val <<32 ; +break ; +default : + status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ; +break ; + } + +return status; +} + +static int nvmet_vhost_bar_write (struct nvmet_vhost_ctrl *n,int offset,u64 val) +{ +if (offset <0x1000 ) +return nvmet_bar_write (n, offset, val); + +return -1 ; +} + +static int nvmet_vhost_ioc_bar (struct nvmet_vhost_ctrl *n,void __user *argp) +{ +struct nvmet_vhost_bar bar; +struct nvmet_vhost_bar __user *user_bar = argp; +int ret = -EINVAL ; + + ret =copy_from_user (&bar, argp,sizeof (bar)); +if (unlikely (ret)) +return ret; + +if (bar.type ==VHOST_NVME_BAR_READ ) { +u64 val; + ret =nvmet_vhost_bar_read (n->ctrl, bar.offset, &val); +if (ret !=NVME_SC_SUCCESS ) +return ret; + ret =copy_to_user (&user_bar->val, &val,sizeof (u64)); + }else if (bar.type ==VHOST_NVME_BAR_WRITE ) + ret =nvmet_vhost_bar_write (n, bar.offset, bar.val); + +return ret; +} +static int nvmet_vhost_open (struct inode *inode,struct file *f) {struct nvmet_vhost_ctrl *n =kzalloc (sizeof (*n),GFP_KERNEL );
@@ -223,6 +323,8 @@ static long nvmet_vhost_ioctl(struct file *f, unsigned int ioctl,case VHOST_NVME_SET_EVENTFD : r =nvmet_vhost_set_eventfd (n, argp);return r; +case VHOST_NVME_BAR : +return nvmet_vhost_ioc_bar (n, argp);case VHOST_GET_FEATURES : features =VHOST_FEATURES ;if (copy_to_user (featurep, &features,sizeof (features)))
include/uapi/linux/vhost.h +14 -3
@@ -180,8 +180,19 @@ struct nvmet_vhost_eventfd { int *vector; }; -#define VHOST_NVME_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x47, struct vhost_nvme_target) -#define VHOST_NVME_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x48, struct vhost_nvme_target) -#define VHOST_NVME_SET_EVENTFD _IOW(VHOST_VIRTIO, 0x45, struct nvmet_vhost_eventfd) +#define VHOST_NVME_BAR_READ 0 +#define VHOST_NVME_BAR_WRITE 1 + +struct nvmet_vhost_bar { +int type;/* read/write */ +u64 offset; +unsigned size; +u64 val; +}; + +#define VHOST_NVME_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x45, struct vhost_nvme_target) +#define VHOST_NVME_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x46, struct vhost_nvme_target) +#define VHOST_NVME_SET_EVENTFD _IOW(VHOST_VIRTIO, 0x47, struct nvmet_vhost_eventfd) +#define VHOST_NVME_BAR _IOW(VHOST_VIRTIO, 0x48, struct nvmet_vhost_bar)#endif
Signature · 1 line
1.9.1
RFC PATCH 4/9 nvmet: add a controller "start" hook
In reply to Ming LinFrom: Ming Lin <[email protected]>
This is used to execute controller specific start code
--- drivers/nvme/target/core.c | 3 +++ drivers/nvme/target/nvmet.h | 3 +++ 2 files changed, 6 insertions(+)
drivers/nvme/target/core.c +3 -0
@@ -251,6 +251,9 @@ static void nvmet_start_ctrl(struct nvmet_ctrl *ctrl) } ctrl->csts =NVME_CSTS_RDY ; + +if (ctrl->start) + ctrl->start (ctrl->opaque); }static void nvmet_clear_ctrl (struct nvmet_ctrl *ctrl)
drivers/nvme/target/nvmet.h +3 -0
@@ -59,6 +59,9 @@ struct nvmet_ctrl {struct kref ref;#define NVMET_SUBSYS_NAME_LEN 256char subsys_name[NVMET_SUBSYS_NAME_LEN ]; + +void *opaque; +void (*start)(void *); };struct nvmet_subsys {
Signature · 1 line
1.9.1
Christoph Hellwig
maintainerRFC PATCH 4/9 nvmet: add a controller "start" hook
drivers/nvme/target/nvmet.h struct nvmet_ctrl { · line 65
Quoted 6 lines · Ming Lin
#define
NVMET_SUBSYS_NAME_LEN256drivers/nvme/target/nvmet.h +3
-0char subsys_name[NVMET_SUBSYS_NAME_LEN ]; + +void *opaque; +void (*start)(void *); };
Why can't vhost use container_of to get at the containing structure similar to what the loop driver does?
In addition I think we'll eventually need an ops structure here, but I can take care of that later.
RFC PATCH 4/9 nvmet: add a controller "start" hook
Quoted 10 lines · Christoph Hellwig
drivers/nvme/target/nvmet.h struct nvmet_ctrl { · line 65
Quoted 6 lines · Ming Lin
#define
NVMET_SUBSYS_NAME_LEN256drivers/nvme/target/nvmet.h +3
-0char subsys_name[NVMET_SUBSYS_NAME_LEN ]; + +void *opaque; +void (*start)(void *); };Why can't vhost use
container_ofto get at the containing structure similar to what the loop driver does?
struct nvme_loop_ctrl {
struct nvme_ctrl ctrl;
struct nvmet_ctrl *target_ctrl;
};static inline struct nvme_loop_ctrl *to_loop_ctrl(struct nvme_ctrl *ctrl)
{
return container_of(ctrl, struct nvme_loop_ctrl, ctrl);
}loop driver's container_of is used for "nvme_ctrl", not "nvmet_ctrl"
Possibly change:
struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_subsys *subsys,
const char *subsys_name)
to
int nvmet_init_ctrl(struct nvmet_ctrl *, ....)
?
So we can embed "struct nvmet_ctrl" to upper layer structure.
Quoted 3 lines
In addition I think we'll eventually need an ops structure here, but I can take care of that later.
Christoph Hellwig
maintainerRe: [RFC PATCH 4/9] nvmet: add a controller "start" hook
In reply to Ming Lindrivers/nvme/target/nvmet.h struct nvmet_ctrl { · line 65
Quoted 6 lines · Ming Lin
#define
NVMET_SUBSYS_NAME_LEN256drivers/nvme/target/nvmet.h +3
-0char subsys_name[NVMET_SUBSYS_NAME_LEN ]; + +void *opaque; +void (*start)(void *); };
Why can't vhost use container_of to get at the containing structure similar to what the loop driver does?
In addition I think we'll eventually need an ops structure here, but I can take care of that later.
RFC PATCH 5/9 nvme-vhost: add controller "start" callback
In reply to Ming LinFrom: Ming Lin <[email protected]>
--- drivers/nvme/target/vhost.c | 106 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+)
drivers/nvme/target/vhost.c +106 -0
@@ -10,6 +10,35 @@#define NVMET_VHOST_AQ_DEPTH 256 +enum NvmeCcShift { +CC_MPS_SHIFT =7 , +CC_IOSQES_SHIFT =16 , +CC_IOCQES_SHIFT =20 , +}; + +enum NvmeCcMask { +CC_MPS_MASK =0xf , +CC_IOSQES_MASK =0xf , +CC_IOCQES_MASK =0xf , +}; + +#define NVME_CC_MPS (cc) ((cc >> CC_MPS_SHIFT) & CC_MPS_MASK) +#define NVME_CC_IOSQES (cc) ((cc >> CC_IOSQES_SHIFT) & CC_IOSQES_MASK) +#define NVME_CC_IOCQES (cc) ((cc >> CC_IOCQES_SHIFT) & CC_IOCQES_MASK) + +enum NvmeAqaShift { +AQA_ASQS_SHIFT =0 , +AQA_ACQS_SHIFT =16 , +}; + +enum NvmeAqaMask { +AQA_ASQS_MASK =0xfff , +AQA_ACQS_MASK =0xfff , +}; + +#define NVME_AQA_ASQS (aqa) ((aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK) +#define NVME_AQA_ACQS (aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK) +struct nvmet_vhost_ctrl_eventfd {struct file *call;struct eventfd_ctx *call_ctx;
@@ -19,12 +48,23 @@ struct nvmet_vhost_ctrl_eventfd {struct nvmet_vhost_cq {struct nvmet_cq cq; +struct nvmet_vhost_ctrl *ctrl; +u32 head; +u32 tail; +u8 phase; +u64 dma_addr;struct eventfd_ctx *eventfd; };struct nvmet_vhost_sq {struct nvmet_sq sq; +struct nvmet_vhost_ctrl *ctrl; + +u32 head; +u32 tail; +u64 dma_addr; +u16 cqid; };struct nvmet_vhost_ctrl {
@@ -37,12 +77,76 @@ struct nvmet_vhost_ctrl {struct nvmet_vhost_cq **cqs;struct nvmet_vhost_sq **sqs; +struct nvmet_vhost_cq admin_cq; +struct nvmet_vhost_sq admin_sq;u32 aqa;u64 asq;u64 acq; +u16 cqe_size; +u16 sqe_size; +u16 max_prp_ents; +u16 page_bits; +u32 page_size; }; +static int nvmet_vhost_init_cq (struct nvmet_vhost_cq *cq, +struct nvmet_vhost_ctrl *n,u64 dma_addr, +u16 cqid,u16 size,struct eventfd_ctx *eventfd, +u16 vector,u16 irq_enabled) +{ + cq->ctrl = n; + cq->dma_addr = dma_addr; + cq->phase =1 ; + cq->head = cq->tail =0 ; + cq->eventfd = eventfd; + n->cqs[cqid] = cq; + +nvmet_cq_init (n->ctrl, &cq->cq, cqid, size); + +return 0 ; +} + +static int nvmet_vhost_init_sq (struct nvmet_vhost_sq *sq, +struct nvmet_vhost_ctrl *n,u64 dma_addr, +u16 sqid,u16 cqid,u16 size) +{ + sq->ctrl = n; + sq->dma_addr = dma_addr; + sq->cqid = cqid; + sq->head = sq->tail =0 ; + n->sqs[sqid] = sq; + +nvmet_sq_init (n->ctrl, &sq->sq, sqid, size); + +return 0 ; +} + +static void nvmet_vhost_start_ctrl (void *opaque) +{ +struct nvmet_vhost_ctrl *n = opaque; +u32 page_bits =NVME_CC_MPS (n->ctrl->cc) +12 ; +u32 page_size =1 << page_bits; +int ret; + + n->page_bits = page_bits; + n->page_size = page_size; + n->max_prp_ents = n->page_size /sizeof (uint64_t ); + n->cqe_size =1 <<NVME_CC_IOCQES (n->ctrl->cc); + n->sqe_size =1 <<NVME_CC_IOSQES (n->ctrl->cc); + +nvmet_vhost_init_cq (&n->admin_cq, n, n->acq,0 , +NVME_AQA_ACQS (n->aqa) +1 , n->eventfd[0 ].call_ctx, +0 ,1 ); + + ret =nvmet_vhost_init_sq (&n->admin_sq, n, n->asq,0 ,0 , +NVME_AQA_ASQS (n->aqa) +1 ); +if (ret) { +pr_warn ("nvmet_vhost_init_sq failed!!!\n" ); +BUG_ON (1 ); + } +} +static int nvmet_vhost_set_endpoint (struct nvmet_vhost_ctrl *n,struct vhost_nvme_target *c)
@@ -67,6 +171,8 @@ nvmet_vhost_set_endpoint(struct nvmet_vhost_ctrl *n, n->cntlid = ctrl->cntlid; n->ctrl = ctrl; n->num_queues = subsys->max_qid +1 ; + ctrl->opaque = n; + ctrl->start = nvmet_vhost_start_ctrl; num_queues = ctrl->subsys->max_qid +1 ; n->cqs =kzalloc (sizeof (*n->cqs) * num_queues,GFP_KERNEL );
Signature · 1 line
1.9.1
RFC PATCH 6/9 nvmet: add a "parse_extra_admin_cmd" hook
In reply to Ming LinFrom: Ming Lin <[email protected]>
This is used to execute controller specific cmd parse code
--- drivers/nvme/target/admin-cmd.c | 7 +++++++ drivers/nvme/target/nvmet.h | 3 +++ 2 files changed, 10 insertions(+)
drivers/nvme/target/admin-cmd.c +7 -0
@@ -346,6 +346,13 @@ int nvmet_parse_admin_cmd(struct nvmet_req *req) req->data =0 ;return 0 ;#endif +default : +if (req->sq->ctrl->parse_extra_admin_cmd) { +int ret = req->sq->ctrl->parse_extra_admin_cmd (req); + +if (!ret) +return 0 ; + } }pr_err ("nvmet: unhandled cmd %d\n" , cmd->common.opcode);
drivers/nvme/target/nvmet.h +3 -0
@@ -44,6 +44,8 @@ struct nvmet_sq {u16 size; }; +struct nvmet_req ; +struct nvmet_ctrl {struct nvmet_subsys *subsys;struct nvmet_cq **cqs;
@@ -62,6 +64,7 @@ struct nvmet_ctrl {void *opaque;void (*start)(void *); +int (*parse_extra_admin_cmd)(struct nvmet_req *); };struct nvmet_subsys {
Signature · 1 line
1.9.1
RFC PATCH 7/9 nvme-vhost: add "parse_extra_admin_cmd" callback
In reply to Ming LinFrom: Ming Lin <[email protected]>
--- drivers/nvme/target/vhost.c | 153 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+)
drivers/nvme/target/vhost.c +153 -0
@@ -39,6 +39,11 @@ enum NvmeAqaMask {#define NVME_AQA_ASQS (aqa) ((aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK)#define NVME_AQA_ACQS (aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK) +#define NVME_CQ_FLAGS_PC (cq_flags) (cq_flags & 0x1) +#define NVME_CQ_FLAGS_IEN (cq_flags) ((cq_flags >> 1) & 0x1) + +#define NVME_SQ_FLAGS_PC (sq_flags) (sq_flags & 0x1) +struct nvmet_vhost_ctrl_eventfd {struct file *call;struct eventfd_ctx *call_ctx;
@@ -90,6 +95,19 @@ struct nvmet_vhost_ctrl {u32 page_size; }; +#define sq_to_vsq (sq) container_of(sq, struct nvmet_vhost_sq, sq) +#define cq_to_vcq (cq) container_of(cq, struct nvmet_vhost_cq, cq) + +static int nvmet_vhost_check_sqid (struct nvmet_ctrl *n,u16 sqid) +{ +return sqid <= n->subsys->max_qid && n->sqs[sqid] !=NULL ?0 :-1 ; +} + +static int nvmet_vhost_check_cqid (struct nvmet_ctrl *n,u16 cqid) +{ +return cqid <= n->subsys->max_qid && n->cqs[cqid] !=NULL ?0 :-1 ; +} + static int nvmet_vhost_init_cq(struct nvmet_vhost_cq *cq, struct nvmet_vhost_ctrl *n, u64 dma_addr, u16 cqid, u16 size, struct eventfd_ctx *eventfd,
@@ -147,6 +165,140 @@ static void nvmet_vhost_start_ctrl(void *opaque) } } +static void nvmet_vhost_create_cq (struct nvmet_req *req) +{ +struct nvmet_cq *cq; +struct nvmet_vhost_cq *vcq; +struct nvmet_vhost_ctrl *n; +struct nvme_create_cq *c; +u16 cqid; +u16 vector; +u16 qsize; +u16 qflags; +u64 prp1; +int status; +int ret; + + cq = req->cq; + vcq =cq_to_vcq (cq); + n = vcq->ctrl; + c = &req->cmd->create_cq; + cqid =le16_to_cpu (c->cqid); + vector =le16_to_cpu (c->irq_vector); + qsize =le16_to_cpu (c->qsize); + qflags =le16_to_cpu (c->cq_flags); + prp1 =le64_to_cpu (c->prp1); + status =NVME_SC_SUCCESS ; + +if (!cqid || (cqid && !nvmet_vhost_check_cqid (n->ctrl, cqid))) { + status =NVME_SC_QID_INVALID |NVME_SC_DNR ; + goto out; + } +if (!qsize || qsize >NVME_CAP_MQES (n->ctrl->cap)) { + status =NVME_SC_QUEUE_SIZE |NVME_SC_DNR ; + goto out; + } +if (!prp1) { + status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ; + goto out; + } +if (vector > n->num_queues) { + status =NVME_SC_INVALID_VECTOR |NVME_SC_DNR ; + goto out; + } +if (!(NVME_CQ_FLAGS_PC (qflags))) { + status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ; + goto out; + } + + vcq =kmalloc (sizeof (*vcq),GFP_KERNEL ); +if (!vcq) { + status =NVME_SC_INTERNAL |NVME_SC_DNR ; + goto out; + } + + ret =nvmet_vhost_init_cq (vcq, n, prp1, cqid, qsize+1 , + n->eventfd[cqid].call_ctx, vector, +NVME_CQ_FLAGS_IEN (qflags)); +if (ret) + status =NVME_SC_INTERNAL |NVME_SC_DNR ; + +out: +nvmet_req_complete (req, status); +} + +static void nvmet_vhost_create_sq (struct nvmet_req *req) +{ +struct nvme_create_sq *c = &req->cmd->create_sq; +u16 cqid =le16_to_cpu (c->cqid); +u16 sqid =le16_to_cpu (c->sqid); +u16 qsize =le16_to_cpu (c->qsize); +u16 qflags =le16_to_cpu (c->sq_flags); +u64 prp1 =le64_to_cpu (c->prp1); + +struct nvmet_sq *sq = req->sq; +struct nvmet_vhost_sq *vsq; +struct nvmet_vhost_ctrl *n; +int status; +int ret; + + status =NVME_SC_SUCCESS ; + vsq =sq_to_vsq (sq); + n = vsq->ctrl; + +if (!cqid ||nvmet_vhost_check_cqid (n->ctrl, cqid)) { + status =NVME_SC_CQ_INVALID |NVME_SC_DNR ; + goto out; + } +if (!sqid || (sqid && !nvmet_vhost_check_sqid (n->ctrl, sqid))) { + status =NVME_SC_QID_INVALID |NVME_SC_DNR ; + goto out; + } +if (!qsize || qsize >NVME_CAP_MQES (n->ctrl->cap)) { + status =NVME_SC_QUEUE_SIZE |NVME_SC_DNR ; + goto out; + } +if (!prp1 || prp1 & (n->page_size -1 )) { + status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ; + goto out; + } +if (!(NVME_SQ_FLAGS_PC (qflags))) { + status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ; + goto out; + } + + vsq =kmalloc (sizeof (*vsq),GFP_KERNEL ); +if (!sq) { + status =NVME_SC_INTERNAL |NVME_SC_DNR ; + goto out; + } + + ret =nvmet_vhost_init_sq (vsq, n, prp1, sqid, cqid, qsize +1 ); +if (ret) + status =NVME_SC_INTERNAL |NVME_SC_DNR ; + +out: +nvmet_req_complete (req, status); +} + +static int nvmet_vhost_parse_admin_cmd (struct nvmet_req *req) +{ +struct nvme_command *cmd = req->cmd; + +switch (cmd->common.opcode) { +case nvme_admin_create_cq: + req->execute = nvmet_vhost_create_cq; + req->data_len =0 ; +return 0 ; +case nvme_admin_create_sq: + req->execute = nvmet_vhost_create_sq; + req->data_len =0 ; +return 0 ; + } + +return -1 ; +} +static int nvmet_vhost_set_endpoint (struct nvmet_vhost_ctrl *n,struct vhost_nvme_target *c)
@@ -173,6 +325,7 @@ nvmet_vhost_set_endpoint(struct nvmet_vhost_ctrl *n, n->num_queues = subsys->max_qid +1 ; ctrl->opaque = n; ctrl->start = nvmet_vhost_start_ctrl; + ctrl->parse_extra_admin_cmd = nvmet_vhost_parse_admin_cmd; num_queues = ctrl->subsys->max_qid +1 ; n->cqs =kzalloc (sizeof (*n->cqs) * num_queues,GFP_KERNEL );
Signature · 1 line
1.9.1
RFC PATCH 8/9 nvme-vhost: add vhost memory helpers
In reply to Ming LinFrom: Ming Lin <[email protected]>
This borrows code from Hannes Reinecke's rts-megasas.
--- drivers/nvme/target/vhost.c | 108 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)
drivers/nvme/target/vhost.c +108 -0
@@ -5,6 +5,7 @@#include <linux/miscdevice.h> #include <linux/mutex.h> #include <linux/file.h> +#include <linux/highmem.h> #include "../../vhost/vhost.h" #include "nvmet.h"
@@ -95,6 +96,113 @@ struct nvmet_vhost_ctrl {u32 page_size; }; +const struct vhost_memory_region * +find_region (struct vhost_dev *hba,__u64 addr,__u32 len) +{ +struct vhost_memory *mem; +struct vhost_memory_region *reg; +int i; + +if (!hba->memory) +return NULL ; + + mem = hba->memory; +/* linear search is not brilliant, but we really have on the order of 6 +* regions in practice */ +for (i =0 ; i < mem->nregions; ++i) { + reg = mem->regions + i; +if (reg->guest_phys_addr <= addr && + reg->guest_phys_addr + reg->memory_size -1 >= addr) +return reg; + } +return NULL ; +} + +static bool check_region_boundary (const struct vhost_memory_region *reg, +uint64_t addr,size_t len) +{ +unsigned long max_size; + + max_size = reg->memory_size - addr + reg->guest_phys_addr; +return (max_size < len); +} + +static void __user *map_to_region (const struct vhost_memory_region *reg, +uint64_t addr) +{ +return (void __user *)(unsigned long ) + (reg->userspace_addr + addr - reg->guest_phys_addr); +} + +static void __user *map_guest_to_host (struct vhost_dev *dev, +uint64_t addr,int size) +{ +const struct vhost_memory_region *reg =NULL ; + + reg =find_region (dev, addr, size); +if (unlikely (!reg)) +return ERR_PTR (-EPERM ); + +if (unlikely (check_region_boundary (reg, addr, size))) +return ERR_PTR (-EFAULT ); + +return map_to_region (reg, addr); +} + +static int nvmet_vhost_rw (struct vhost_dev *dev,u64 guest_pa, +void *buf,uint32_t size,int write) +{ +void __user *host_user_va; +void *host_kernel_va; +struct page *page; +uintptr_t offset; +int ret; + + host_user_va =map_guest_to_host (dev, guest_pa, size); +if (unlikely (!host_user_va)) { +pr_warn ("cannot map guest addr %p, error %ld\n" , + (void *)guest_pa,PTR_ERR (host_user_va)); +return -EINVAL ; + } + + ret =get_user_pages (current, dev->mm, + (unsigned long )host_user_va,1 , + false,0 , &page,NULL ); +if (unlikely (ret !=1 )) { +pr_warn ("get_user_pages fail!!!\n" ); +return -EINVAL ; + } + + host_kernel_va =kmap (page); +if (unlikely (!host_kernel_va)) { +pr_warn ("kmap fail!!!\n" ); +put_page (page); +return -EINVAL ; + } + + offset = (uintptr_t )host_user_va & ~PAGE_MASK ; +if (write) +memcpy (host_kernel_va + offset, buf, size); +else +memcpy (buf, host_kernel_va + offset, size); +kunmap (host_kernel_va); +put_page (page); + +return 0 ; +} + +int nvmet_vhost_read (struct vhost_dev *dev,u64 guest_pa, +void *buf,uint32_t size) +{ +return nvmet_vhost_rw (dev, guest_pa, buf, size,0 ); +} + +int nvmet_vhost_write (struct vhost_dev *dev,u64 guest_pa, +void *buf,uint32_t size) +{ +return nvmet_vhost_rw (dev, guest_pa, buf, size,1 ); +} +#define sq_to_vsq (sq) container_of(sq, struct nvmet_vhost_sq, sq)#define cq_to_vcq (cq) container_of(cq, struct nvmet_vhost_cq, cq)
Signature · 1 line
1.9.1
RFC PATCH 9/9 nvme-vhost: add nvme queue handlers
In reply to Ming LinFrom: Ming Lin <[email protected]>
This adds nvme submission/completion queue handlers, which are ported from qemu-nvme.
And hooks into nvme-target to do the real job.
--- drivers/nvme/target/vhost.c | 420 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 416 insertions(+), 4 deletions(-)
drivers/nvme/target/vhost.c +416 -4
@@ -6,10 +6,12 @@#include <linux/mutex.h> #include <linux/file.h> #include <linux/highmem.h> +#include <linux/kthread.h> #include "../../vhost/vhost.h" #include "nvmet.h" #define NVMET_VHOST_AQ_DEPTH 256 +#define NVMET_VHOST_MAX_SEGMENTS 32enum NvmeCcShift {CC_MPS_SHIFT =7 ,
@@ -52,6 +54,15 @@ struct nvmet_vhost_ctrl_eventfd {int __user *vector; }; +struct nvmet_vhost_iod { +struct nvmet_vhost_sq *sq; +struct scatterlist sg[NVMET_VHOST_MAX_SEGMENTS ]; +struct nvme_command cmd; +struct nvme_completion rsp; +struct nvmet_req req; +struct list_head entry; +}; +struct nvmet_vhost_cq {struct nvmet_cq cq;struct nvmet_vhost_ctrl *ctrl;
@@ -61,6 +72,12 @@ struct nvmet_vhost_cq {u8 phase;u64 dma_addr;struct eventfd_ctx *eventfd; + +struct list_head sq_list; +struct list_head req_list; +spinlock_t lock; +struct task_struct *thread; +int scheduled; };struct nvmet_vhost_sq {
@@ -71,6 +88,13 @@ struct nvmet_vhost_sq {u32 tail;u64 dma_addr;u16 cqid; + +struct nvmet_vhost_iod *io_req; +struct list_head req_list; +struct list_head entry; +struct mutex lock; +struct task_struct *thread; +int scheduled; };struct nvmet_vhost_ctrl {
@@ -191,13 +215,13 @@ static int nvmet_vhost_rw(struct vhost_dev *dev, u64 guest_pa,return 0 ; } -int nvmet_vhost_read (struct vhost_dev *dev,u64 guest_pa, +static int nvmet_vhost_read (struct vhost_dev *dev,u64 guest_pa,void *buf,uint32_t size) {return nvmet_vhost_rw (dev, guest_pa, buf, size,0 ); } -int nvmet_vhost_write (struct vhost_dev *dev,u64 guest_pa, +static int nvmet_vhost_write (struct vhost_dev *dev,u64 guest_pa,void *buf,uint32_t size) {return nvmet_vhost_rw (dev, guest_pa, buf, size,1 );
@@ -216,6 +240,289 @@ static int nvmet_vhost_check_cqid(struct nvmet_ctrl *n, u16 cqid)return cqid <= n->subsys->max_qid && n->cqs[cqid] !=NULL ?0 :-1 ; } +static void nvmet_vhost_inc_cq_tail (struct nvmet_vhost_cq *cq) +{ + cq->tail++; +if (cq->tail >= cq->cq.size) { + cq->tail =0 ; + cq->phase = !cq->phase; + } +} + +static void nvmet_vhost_inc_sq_head (struct nvmet_vhost_sq *sq) +{ + sq->head = (sq->head +1 ) % sq->sq.size; +} + +static uint8_t nvmet_vhost_cq_full (struct nvmet_vhost_cq *cq) +{ +return (cq->tail +1 ) % cq->cq.size == cq->head; +} + +static uint8_t nvmet_vhost_sq_empty (struct nvmet_vhost_sq *sq) +{ +return sq->head == sq->tail; +} + +static void nvmet_vhost_post_cqes (struct nvmet_vhost_cq *cq) +{ +struct nvmet_vhost_ctrl *n = cq->ctrl; +struct nvmet_vhost_iod *req; +struct list_head *p, *tmp; +int signal =0 ; +unsigned long flags; + +spin_lock_irqsave (&cq->lock, flags); +list_for_each_safe (p, tmp, &cq->req_list) { +struct nvmet_vhost_sq *sq; +u64 addr; + +if (nvmet_vhost_cq_full (cq)) + goto unlock; + + req = list_entry(p, struct nvmet_vhost_iod, entry); +list_del (p); + + sq = req->sq; + req->rsp.status |= cq->phase; + req->rsp.sq_id =cpu_to_le16 (sq->sq.qid); + req->rsp.sq_head =cpu_to_le16 (sq->head); + addr = cq->dma_addr + cq->tail * n->cqe_size; +nvmet_vhost_inc_cq_tail (cq); +spin_unlock_irqrestore (&cq->lock, flags); + +nvmet_vhost_write (&n->dev, addr, (void *)&req->rsp, +sizeof (req->rsp)); + +mutex_lock (&sq->lock); +list_add_tail (p, &sq->req_list); +mutex_unlock (&sq->lock); + + signal =1 ; + +spin_lock_irqsave (&cq->lock, flags); + } + +if (signal) +eventfd_signal (cq->eventfd,1 ); + +unlock: + cq->scheduled =0 ; +spin_unlock_irqrestore (&cq->lock, flags); +} + +static int nvmet_vhost_cq_thread (void *arg) +{ +struct nvmet_vhost_cq *sq = arg; + +while (1 ) { +if (kthread_should_stop ()) +break ; + +nvmet_vhost_post_cqes (sq); + +schedule (); + } + +return 0 ; +} + +static void nvmet_vhost_enqueue_req_completion ( +struct nvmet_vhost_cq *cq,struct nvmet_vhost_iod *iod) +{ +unsigned long flags; + +BUG_ON (cq->cq.qid != iod->sq->sq.qid); +spin_lock_irqsave (&cq->lock, flags); +list_add_tail (&iod->entry, &cq->req_list); +if (!cq->scheduled) { +wake_up_process (cq->thread); + cq->scheduled =1 ; + } +spin_unlock_irqrestore (&cq->lock, flags); +} + +static void nvmet_vhost_queue_response (struct nvmet_req *req) +{ +struct nvmet_vhost_iod *iod = + container_of(req, struct nvmet_vhost_iod, req); +struct nvmet_vhost_sq *sq = iod->sq; +struct nvmet_vhost_ctrl *n = sq->ctrl; +struct nvmet_vhost_cq *cq = n->cqs[sq->sq.qid]; + +nvmet_vhost_enqueue_req_completion (cq, iod); +} + +static int nvmet_vhost_sglist_add (struct nvmet_vhost_ctrl *n,struct scatterlist *sg, +u64 guest_addr,int len,int is_write) +{ +void __user *host_addr; +struct page *page; +unsigned int offset, nbytes; +int ret; + + host_addr =map_guest_to_host (&n->dev, guest_addr, len); +if (unlikely (!host_addr)) { +pr_warn ("cannot map guest addr %p, error %ld\n" , + (void *)guest_addr,PTR_ERR (host_addr)); +return PTR_ERR (host_addr); + } + + ret =get_user_pages (current, n->dev.mm, (unsigned long )host_addr,1 , + is_write,0 , &page,NULL ); +BUG_ON (ret ==0 );/* we should either get our page or fail */ +if (ret <0 ) { +pr_warn ("get_user_pages faild: host_addr %p, %d\n" , + host_addr, ret); +return ret; + } + + offset = (uintptr_t )host_addr & ~PAGE_MASK ; + nbytes = min_t(unsigned int, PAGE_SIZE - offset, len); +sg_set_page (sg, page, nbytes, offset); + +return 0 ; +} + +static int nvmet_vhost_map_prp (struct nvmet_vhost_ctrl *n,struct scatterlist *sgl, +u64 prp1,u64 prp2,unsigned int len) +{ +unsigned int trans_len = n->page_size - (prp1 % n->page_size); +int num_prps = (len >> n->page_bits) +1 ; +//FIXME +int is_write =1 ; + + trans_len =min (len, trans_len); +if (!prp1) +return -1 ; + +sg_init_table (sgl, num_prps); + +nvmet_vhost_sglist_add (n, sgl, prp1, trans_len, is_write); + + len -= trans_len; +if (len) { +if (!prp2) + goto error; +if (len > n->page_size) { +u64 prp_list[n->max_prp_ents]; +u16 nents, prp_trans; +int i =0 ; + + nents = (len + n->page_size -1 ) >> n->page_bits; + prp_trans =min (n->max_prp_ents, nents) *sizeof (u64); +nvmet_vhost_read (&n->dev, prp2, (void *)prp_list, prp_trans); + +while (len !=0 ) { +u64 prp_ent =le64_to_cpu (prp_list[i]); + +if (i == n->max_prp_ents -1 && len > n->page_size) { +if (!prp_ent || prp_ent & (n->page_size -1 )) + goto error; + i =0 ; + nents = (len + n->page_size -1 ) >> n->page_bits; + prp_trans =min (n->max_prp_ents, nents) *sizeof (u64); +nvmet_vhost_read (&n->dev, prp_ent, (void *)prp_list, prp_trans); + prp_ent =le64_to_cpu (prp_list[i]); + } + +if (!prp_ent || prp_ent & (n->page_size -1 )) + goto error; + + trans_len =min (len, n->page_size); +nvmet_vhost_sglist_add (n, sgl, prp_ent, trans_len, is_write); + sgl++; + len -= trans_len; + i++; + } + }else { +if (prp2 & (n->page_size -1 )) + goto error; +nvmet_vhost_sglist_add (n, sgl, prp2, trans_len, is_write); + } + } + +return num_prps; + +error: +return -1 ; +} + +static void nvmet_vhost_process_sq (struct nvmet_vhost_sq *sq) +{ +struct nvmet_vhost_ctrl *n = sq->ctrl; +struct nvmet_vhost_cq *cq = n->cqs[sq->sq.qid]; +struct nvmet_vhost_iod *iod; +struct nvme_command *cmd; +int ret; + +mutex_lock (&sq->lock); + +while (!(nvmet_vhost_sq_empty (sq) ||list_empty (&sq->req_list))) { +u64 addr = sq->dma_addr + sq->head * n->sqe_size;; + +nvmet_vhost_inc_sq_head (sq); + iod =list_first_entry (&sq->req_list, + struct nvmet_vhost_iod, entry); +list_del (&iod->entry); +mutex_unlock (&sq->lock); + + cmd = &iod->cmd; + ret =nvmet_vhost_read (&n->dev, addr, + (void *)cmd,sizeof (*cmd)); +if (ret) { +pr_warn ("nvmet_vhost_read fail\n" ); + goto out; + } + + ret =nvmet_req_init (&iod->req, &cq->cq, &sq->sq, + nvmet_vhost_queue_response); +if (ret) { +pr_warn ("nvmet_req_init error: ret 0x%x, qid %d\n" , ret, sq->sq.qid); + goto out; + } +if (iod->req.data_len) { + ret =nvmet_vhost_map_prp (n, iod->sg, cmd->common.prp1, + cmd->common.prp2, iod->req.data_len); +if (ret >0 ) { + iod->req.sg = iod->sg; + iod->req.sg_cnt = ret; + }else { +pr_warn ("map prp error\n" ); + goto out; + } + } + iod->req.execute (&iod->req); +mutex_lock (&sq->lock); + } + +unlock: + sq->scheduled =0 ; +mutex_unlock (&sq->lock); +return ; + +out: +mutex_lock (&sq->lock); +list_add_tail (&iod->entry, &sq->req_list); + goto unlock; +} + +static int nvmet_vhost_sq_thread (void *opaque) +{ +struct nvmet_vhost_sq *sq = opaque; + +while (1 ) { +if (kthread_should_stop ()) +break ; + +nvmet_vhost_process_sq (sq); + +schedule (); + } + +return 0 ; +} + static int nvmet_vhost_init_cq(struct nvmet_vhost_cq *cq, struct nvmet_vhost_ctrl *n, u64 dma_addr, u16 cqid, u16 size, struct eventfd_ctx *eventfd,
@@ -228,6 +535,12 @@ static int nvmet_vhost_init_cq(struct nvmet_vhost_cq *cq, cq->eventfd = eventfd; n->cqs[cqid] = cq; +spin_lock_init (&cq->lock); +INIT_LIST_HEAD (&cq->req_list); +INIT_LIST_HEAD (&cq->sq_list); + cq->scheduled =0 ; + cq->thread =kthread_create (nvmet_vhost_cq_thread, cq,"nvmet_vhost_cq" ); +nvmet_cq_init (n->ctrl, &cq->cq, cqid, size);return 0 ;
@@ -237,12 +550,36 @@ static int nvmet_vhost_init_sq(struct nvmet_vhost_sq *sq, struct nvmet_vhost_ctrl *n, u64 dma_addr, u16 sqid, u16 cqid, u16 size) { + struct nvmet_vhost_cq *cq; +struct nvmet_vhost_iod *iod; +int i; + sq->ctrl = n; sq->dma_addr = dma_addr; sq->cqid = cqid; sq->head = sq->tail =0 ; n->sqs[sqid] = sq; +mutex_init (&sq->lock); +INIT_LIST_HEAD (&sq->req_list); + sq->io_req =kmalloc (sizeof (struct nvmet_vhost_iod ) * size,GFP_KERNEL ); +if (!sq->io_req) +return -ENOMEM ; +for (i =0 ; i < size; i++) { + iod = &sq->io_req[i]; + + iod->req.cmd = &iod->cmd; + iod->req.rsp = &iod->rsp; + iod->sq = sq; +list_add_tail (&iod->entry, &sq->req_list); + } + sq->scheduled =0 ; + sq->thread =kthread_create (nvmet_vhost_sq_thread, sq,"nvmet_vhost_sq" ); + + cq = n->cqs[cqid]; +list_add_tail (&sq->entry, &cq->sq_list); + n->sqs[sqid] = sq; +nvmet_sq_init (n->ctrl, &sq->sq, sqid, size);return 0 ;
@@ -564,12 +901,84 @@ static int nvmet_bar_write(struct nvmet_vhost_ctrl *n, int offset, u64 val)return status; } +static int nvmet_vhost_process_db (struct nvmet_ctrl *ctrl,int offset,u64 val) +{ +u16 qid; + +if (offset & ((1 <<2 ) -1 )) +return -EINVAL ; + +if (((offset -0x1000 ) >>2 ) &1 ) { +u16 new_head = val &0xffff ; +int start_sqs; +struct nvmet_vhost_cq *vcq; +struct nvmet_cq *cq; +unsigned long flags; + + qid = (offset - (0x1000 + (1 <<2 ))) >>3 ; +if (nvmet_vhost_check_cqid (ctrl, qid)) +return -EINVAL ; + + cq = ctrl->cqs[qid]; +if (new_head >= cq->size) +return -EINVAL ; + + vcq =cq_to_vcq (cq); +spin_lock_irqsave (&vcq->lock, flags); + start_sqs =nvmet_vhost_cq_full (vcq) ?1 :0 ; + vcq->head = new_head; +spin_unlock_irqrestore (&vcq->lock, flags); +if (start_sqs) { +struct nvmet_vhost_sq *sq; +struct list_head *p; + +list_for_each (p, &vcq->sq_list) { + sq = list_entry(p, struct nvmet_vhost_sq, entry); +if (!sq->scheduled) { + sq->scheduled =1 ; +wake_up_process (sq->thread); + } + } +if (!vcq->scheduled) { + vcq->scheduled =1 ; +wake_up_process (vcq->thread); + } + } + +if (vcq->tail != vcq->head) +eventfd_signal (vcq->eventfd,1 ); + }else { +struct nvmet_vhost_sq *vsq; +struct nvmet_sq *sq; +u16 new_tail = val &0xffff ; + + qid = (offset -0x1000 ) >>3 ; +if (nvmet_vhost_check_sqid (ctrl, qid)) +return -EINVAL ; + + sq = ctrl->sqs[qid]; +if (new_tail >= sq->size) +return -ENOSPC ; + + vsq =sq_to_vsq (sq); +mutex_lock (&vsq->lock); + vsq->tail = new_tail; +if (!vsq->scheduled) { + vsq->scheduled =1 ; +wake_up_process (vsq->thread); + } +mutex_unlock (&vsq->lock); + } + +return 0 ; +} +static int nvmet_vhost_bar_write (struct nvmet_vhost_ctrl *n,int offset,u64 val) {if (offset <0x1000 )return nvmet_bar_write (n, offset, val); - -return -1 ; +else +return nvmet_vhost_process_db (n->ctrl, offset, val); }static int nvmet_vhost_ioc_bar (struct nvmet_vhost_ctrl *n,void __user *argp)
@@ -612,6 +1021,8 @@ static void nvme_free_sq(struct nvmet_vhost_sq *sq, struct nvmet_vhost_ctrl *n) { n->sqs[sq->sq.qid] =NULL ; +kthread_stop (sq->thread); +kfree (sq->io_req);if (sq->sq.qid)kfree (sq); }
@@ -620,6 +1031,7 @@ static void nvme_free_cq(struct nvmet_vhost_cq *cq, struct nvmet_vhost_ctrl *n) { n->cqs[cq->cq.qid] =NULL ; +kthread_stop (cq->thread);if (cq->cq.qid)kfree (cq); }
Signature · 1 line
1.9.1
Christoph Hellwig
maintainercover vhost-nvme: new qemu nvme backend using nvme target
In reply to Ming LinThanks Ming,
from a first quick view this looks great. I'll look over it in a bit more detail once I get a bit more time.
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 4 lines · Christoph Hellwig
Thanks Ming,
from a first quick view this looks great. I'll look over it in a bit more detail once I get a bit more time.
Thanks to CC Nic :-)
But funny, I double-checked bash history. I actually CCed Nic. Don't know why it's lost.
mlin at ssi:~$ history |grep "nab"
1861 git send-email --from "Ming Lin <mlin at kernel.org>" --to
"linux-nvme at lists.infradead.org" --cc "qemu-devel at nongnu.org" --cc "virtualization at lists.linux-foundation.org" --cc "Christoph Hellwig <hch at lst.de>" --cc "Nicholas A. Bellinger <nab at linux-iscsi.org>" --compose ~/patches/*.patch
Christoph Hellwig
maintainerThanks Ming,
from a first quick view this looks great. I'll look over it in a bit more detail once I get a bit more time.
cover vhost-nvme: new qemu nvme backend using nvme target
In reply to Ming LinQuoted 8 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-extQemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0
How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
Note that in all likelihood, QEMU can actually do better than 190 MB/s, and gain more parallelism too, by moving the processing of the ioeventfds to a separate thread. This is similar to hw/block/dataplane/virtio-blk.c.
It's actually pretty easy to do. Even though hw/block/dataplane/virtio-blk.c is still using some old APIs, all memory access in QEMU is now thread-safe. I have pending patches for 2.6 that cut that file down to a mere 200 lines of code, NVMe would probably be about the same.
Paolo
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 6 lines · Paolo Bonzini
Quoted 4 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-ext
Quoted 7 lines
Quoted 4 lines
Qemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
There is still some bug. I'll update.
Quoted 11 lines
Note that in all likelihood, QEMU can actually do better than 190 MB/s, and gain more parallelism too, by moving the processing of the ioeventfds to a separate thread. This is similar to
hw/block/dataplane/virtio-blk.c.It's actually pretty easy to do. Even though
hw/block/dataplane/virtio-blk.cis still using some old APIs, all memory access in QEMU is now thread-safe. I have pending patches for 2.6 that cut that file down to a mere 200 lines of code, NVMe would probably be about the same.
Is there a git tree for your patches?
Did you mean some pseduo code as below?
1. need a iothread for each cq/sq?
2. need a AioContext for each cq/sq?
hw/block/nvme.c | 32 ++++++++++++++++++++++++++++++-- hw/block/nvme.h | 8 ++++++++ 2 files changed, 38 insertions(+), 2 deletions(-)
hw/block/nvme.c +30 -2
@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h"
@@ -558,9 +560,22 @@ static void nvme_init_cq_eventfd(NvmeCQueue *cq)uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); + +object_initialize (&cq->internal_iothread_obj, +sizeof (cq->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&cq->internal_iothread_obj), &error_abort); + cq->iothread = &cq->internal_iothread_obj; + cq->ctx =iothread_get_aio_context (cq->iothread); +//Question: Need a conf.blk for each cq/sq??? +//blk_set_aio_context(cq->conf->conf.blk, cq->ctx); + +aio_context_acquire (cq->ctx); +aio_set_event_notifier (cq->ctx, &cq->notifier, true, + nvme_cq_notifier); +aio_context_release (cq->ctx); }static void nvme_sq_notifier (EventNotifier *e)
@@ -578,9 +593,22 @@ static void nvme_init_sq_eventfd(NvmeSQueue *sq)uint16_t offset = sq->sqid *2 * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&sq->notifier,0 ); -event_notifier_set_handler (&sq->notifier, nvme_sq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &sq->notifier); + +object_initialize (&sq->internal_iothread_obj, +sizeof (sq->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&sq->internal_iothread_obj), &error_abort); + sq->iothread = &sq->internal_iothread_obj; + sq->ctx =iothread_get_aio_context (sq->iothread); +//Question: Need a conf.blk for each cq/sq??? +//blk_set_aio_context(sq->conf->conf.blk, sq->ctx); + +aio_context_acquire (sq->ctx); +aio_set_event_notifier (sq->ctx, &sq->notifier, true, + nvme_sq_notifier); +aio_context_release (sq->ctx); }static uint16_t nvme_set_db_memory (NvmeCtrl *n,const NvmeCmd *cmd)
hw/block/nvme.h +8 -0
@@ -667,6 +667,10 @@ typedef struct NvmeSQueue {* do not go over this value will not result in MMIO writes (but will * still write the tail pointer to the "db_addr" location above). */ uint64_t eventidx_addr; + +IOThread *iothread; +IOThread internal_iothread_obj; +AioContext *ctx;EventNotifier notifier; } NvmeSQueue;
@@ -690,6 +694,10 @@ typedef struct NvmeCQueue {* do not go over this value will not result in MMIO writes (but will * still write the head pointer to the "db_addr" location above). */ uint64_t eventidx_addr; + +IOThread *iothread; +IOThread internal_iothread_obj; +AioContext *ctx;EventNotifier notifier; } NvmeCQueue;
Quoted 2 lines
Paolo
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 12 lines · Ming Lin
Quoted 11 lines · Paolo Bonzini
Quoted 8 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-extQemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0
Quoted 4 lines
Quoted 2 lines
How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
There is still some bug. I'll update.
Sure.
Quoted 5 lines
Quoted 5 lines
Note that in all likelihood, QEMU can actually do better than 190 MB/s, and gain more parallelism too, by moving the processing of the ioeventfds to a separate thread. This is similar to
hw/block/dataplane/virtio-blk.c.
Quoted 7 lines
Quoted 5 lines
It's actually pretty easy to do. Even though
hw/block/dataplane/virtio-blk.cis still using some old APIs, all memory access in QEMU is now thread-safe. I have pending patches for 2.6 that cut that file down to a mere 200 lines of code, NVMe would probably be about the same.Is there a git tree for your patches?
No, not yet. I'll post them today or tomorrow, will make sure to Cc you.
Quoted 23 lines
Did you mean some pseduo code as below?
1. need a iothread for each cq/sq?
2. need a AioContext for each cq/sq?hw/block/nvme.c | 32 ++++++++++++++++++++++++++++++-- hw/block/nvme.h | 8 ++++++++ 2 files changed, 38 insertions(+), 2 deletions(-)hw/block/nvme.c +2
-0@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h" @@ -558,9 +560,22 @@ static void nvme_init_cq_eventfd(NvmeCQueue *cq)uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));
hw/block/nvme.c static void nvme_init_cq_eventfd(NvmeCQueue *cq) · line 569
Quoted 10 lines
hw/block/nvme.c +5
-1event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); + +object_initialize (&cq->internal_iothread_obj, +sizeof (cq->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&cq->internal_iothread_obj), &error_abort);
For now, you have to use one iothread for all cq/sq of a single NVMe device; multiqueue block layer is planned for 2.7 or 2.8. Otherwise yes, it's very close to just these changes.
If you use "-object iothread,id=NN" and a iothread property, you can also use an N:M model with multiple disks attached to the same iothread.
Defining the iothread property is like
object_property_add_link(obj, "iothread", TYPE_IOTHREAD, (Object **)&s->conf.iothread, qdev_prop_allow_set_link_before_realize, OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);
Thanks,
Paolo
Quoted 64 lines
hw/block/nvme.c +22
-1+ cq->iothread = &cq->internal_iothread_obj; + cq->ctx =iothread_get_aio_context (cq->iothread); +//Question: Need a conf.blk for each cq/sq??? +//blk_set_aio_context(cq->conf->conf.blk, cq->ctx); +aio_context_acquire (cq->ctx); +aio_set_event_notifier (cq->ctx, &cq->notifier, true, + nvme_cq_notifier); +aio_context_release (cq->ctx); }static void nvme_sq_notifier (EventNotifier *e)@@ -578,9 +593,22 @@ static void nvme_init_sq_eventfd(NvmeSQueue *sq)uint16_t offset = sq->sqid *2 * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&sq->notifier,0 ); -event_notifier_set_handler (&sq->notifier, nvme_sq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &sq->notifier); + +object_initialize (&sq->internal_iothread_obj, +sizeof (sq->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&sq->internal_iothread_obj), &error_abort); + sq->iothread = &sq->internal_iothread_obj; + sq->ctx =iothread_get_aio_context (sq->iothread); +//Question: Need a conf.blk for each cq/sq??? +//blk_set_aio_context(sq->conf->conf.blk, sq->ctx); + +aio_context_acquire (sq->ctx); +aio_set_event_notifier (sq->ctx, &sq->notifier, true, + nvme_sq_notifier); +aio_context_release (sq->ctx); }static uint16_t nvme_set_db_memory (NvmeCtrl *n,const NvmeCmd *cmd)hw/block/nvme.h +8
-0@@ -667,6 +667,10 @@ typedef struct NvmeSQueue {* do not go over this value will not result in MMIO writes (but will * still write the tail pointer to the "db_addr" location above). */ uint64_t eventidx_addr; + +IOThread *iothread; +IOThread internal_iothread_obj; +AioContext *ctx;EventNotifier notifier; } NvmeSQueue;@@ -690,6 +694,10 @@ typedef struct NvmeCQueue {* do not go over this value will not result in MMIO writes (but will * still write the head pointer to the "db_addr" location above). */ uint64_t eventidx_addr; + +IOThread *iothread; +IOThread internal_iothread_obj; +AioContext *ctx;EventNotifier notifier; } NvmeCQueue;Quoted 3 lines
Paolo
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 60 lines · Paolo Bonzini
Quoted 12 lines · Ming Lin
Quoted 11 lines · Paolo Bonzini
Quoted 8 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-extQemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0Quoted 4 lines
Quoted 2 lines
How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
There is still some bug. I'll update.
Sure.
Quoted 5 lines
Quoted 5 lines
Note that in all likelihood, QEMU can actually do better than 190 MB/s, and gain more parallelism too, by moving the processing of the ioeventfds to a separate thread. This is similar to
hw/block/dataplane/virtio-blk.c.Quoted 7 lines
Quoted 5 lines
It's actually pretty easy to do. Even though
hw/block/dataplane/virtio-blk.cis still using some old APIs, all memory access in QEMU is now thread-safe. I have pending patches for 2.6 that cut that file down to a mere 200 lines of code, NVMe would probably be about the same.Is there a git tree for your patches?
No, not yet. I'll post them today or tomorrow, will make sure to Cc you.
Quoted 24 lines
Did you mean some pseduo code as below?
1. need a iothread for each cq/sq?
2. need a AioContext for each cq/sq?hw/block/nvme.c | 32 ++++++++++++++++++++++++++++++-- hw/block/nvme.h | 8 ++++++++ 2 files changed, 38 insertions(+), 2 deletions(-)hw/block/nvme.c +2
-0@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h" @@ -558,9 +560,22 @@ static void nvme_init_cq_eventfd(NvmeCQueue *cq)uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));
Quoted 13 lines
hw/block/nvme.c static void nvme_init_cq_eventfd(NvmeCQueue *cq) · line 569
Quoted 9 lines
hw/block/nvme.c +5
-1event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); + +object_initialize (&cq->internal_iothread_obj, +sizeof (cq->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&cq->internal_iothread_obj), &error_abort);For now, you have to use one iothread for all cq/sq of a single NVMe device; multiqueue block layer is planned for 2.7 or 2.8. Otherwise yes, it's very close to just these changes.
Here is the call stack of iothread for virtio-blk-dataplane.
handle_notify (qemu/hw/block/dataplane/virtio-blk.c:126) aio_dispatch (qemu/aio-posix.c:329)aio_poll (qemu/aio-posix.c:474)iothread_run (qemu/iothread.c:45)start_thread (pthread_create.c:312) /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)
I think I'll have a "nvme_dev_notify" similar as "handle_notify"
static void nvme_dev_notify(EventNotifier *e)
{
....
}But then how can I know this notify is for cq or sq?
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 84 lines · Ming Lin
Quoted 60 lines · Paolo Bonzini
Quoted 12 lines · Ming Lin
Quoted 11 lines · Paolo Bonzini
Quoted 8 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-extQemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0Quoted 4 lines
Quoted 2 lines
How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
There is still some bug. I'll update.
Sure.
Quoted 5 lines
Quoted 5 lines
Note that in all likelihood, QEMU can actually do better than 190 MB/s, and gain more parallelism too, by moving the processing of the ioeventfds to a separate thread. This is similar to
hw/block/dataplane/virtio-blk.c.Quoted 7 lines
Quoted 5 lines
It's actually pretty easy to do. Even though
hw/block/dataplane/virtio-blk.cis still using some old APIs, all memory access in QEMU is now thread-safe. I have pending patches for 2.6 that cut that file down to a mere 200 lines of code, NVMe would probably be about the same.Is there a git tree for your patches?
No, not yet. I'll post them today or tomorrow, will make sure to Cc you.
Quoted 24 lines
Did you mean some pseduo code as below?
1. need a iothread for each cq/sq?
2. need a AioContext for each cq/sq?hw/block/nvme.c | 32 ++++++++++++++++++++++++++++++-- hw/block/nvme.h | 8 ++++++++ 2 files changed, 38 insertions(+), 2 deletions(-)hw/block/nvme.c +2
-0@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h" @@ -558,9 +560,22 @@ static void nvme_init_cq_eventfd(NvmeCQueue *cq)uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));Quoted 13 lines
hw/block/nvme.c static void nvme_init_cq_eventfd(NvmeCQueue *cq) · line 569
Quoted 9 lines
hw/block/nvme.c +5
-1event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); + +object_initialize (&cq->internal_iothread_obj, +sizeof (cq->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&cq->internal_iothread_obj), &error_abort);For now, you have to use one iothread for all cq/sq of a single NVMe device; multiqueue block layer is planned for 2.7 or 2.8. Otherwise yes, it's very close to just these changes.
Here is the call stack of iothread for virtio-blk-dataplane.
handle_notify(qemu/hw/block/dataplane/virtio-blk.c:126)aio_dispatch(qemu/aio-posix.c:329)aio_poll(qemu/aio-posix.c:474)iothread_run(qemu/iothread.c:45)start_thread(pthread_create.c:312) /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)
Quoted 8 lines
I think I'll have a "
nvme_dev_notify" similar as "handle_notify"static void
nvme_dev_notify(EventNotifier *e){ .... }But then how can I know this notify is for cq or sq?
Did you mean patch as below?
But it doesn't work yet.
hw/block/nvme.c | 64 +++++++++++++++++++++++++++++++++++++++------------------ hw/block/nvme.h | 5 +++++ 2 files changed, 49 insertions(+), 20 deletions(-)
hw/block/nvme.c +44 -20
@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h"
@@ -543,42 +545,22 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)return NVME_SUCCESS ; } -static void nvme_cq_notifier (EventNotifier *e) -{ -NvmeCQueue *cq = -container_of (e, NvmeCQueue, notifier); - -event_notifier_test_and_clear (&cq->notifier); -nvme_post_cqes (cq); -} -static void nvme_init_cq_eventfd (NvmeCQueue *cq) {NvmeCtrl *n = cq->ctrl;uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); } -static void nvme_sq_notifier (EventNotifier *e) -{ -NvmeSQueue *sq = -container_of (e, NvmeSQueue, notifier); - -event_notifier_test_and_clear (&sq->notifier); -nvme_process_sq (sq); -} -static void nvme_init_sq_eventfd (NvmeSQueue *sq) {NvmeCtrl *n = sq->ctrl;uint16_t offset = sq->sqid *2 * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&sq->notifier,0 ); -event_notifier_set_handler (&sq->notifier, nvme_sq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &sq->notifier); }
@@ -900,6 +882,45 @@ static const MemoryRegionOps nvme_mmio_ops = { }, }; +static void nvme_dev_notify (EventNotifier *e) +{ +NvmeCtrl *n = +container_of (e, NvmeCtrl, notifier); +int i; + +event_notifier_test_and_clear (e); + +for (i =1 ; i < n->num_queues; i++) { +NvmeSQueue *sq = n->sq[i]; +NvmeCQueue *cq = n->cq[i]; + +if (sq !=NULL ) { +event_notifier_test_and_clear (&sq->notifier); +nvme_process_sq (sq); + } + +if (cq !=NULL ) { +event_notifier_test_and_clear (&cq->notifier); +nvme_post_cqes (cq); + } + } +} + +static void nvme_init_iothread (NvmeCtrl *n) +{ +object_initialize (&n->internal_iothread_obj, +sizeof (n->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&n->internal_iothread_obj), &error_abort); + n->iothread = &n->internal_iothread_obj; + n->ctx =iothread_get_aio_context (n->iothread); + +aio_context_acquire (n->ctx); +aio_set_event_notifier (n->ctx, &n->notifier, true, + nvme_dev_notify); +aio_context_release (n->ctx); +} +static int nvme_init (PCIDevice *pci_dev) {NvmeCtrl *n =NVME (pci_dev);
@@ -995,6 +1016,9 @@ static int nvme_init(PCIDevice *pci_dev)cpu_to_le64 (n->ns_size >> id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX (ns->id_ns.flbas)].ds); } + +nvme_init_iothread (n); +return 0 ; }
hw/block/nvme.h +5 -0
@@ -727,6 +727,11 @@ typedef struct NvmeCtrl {NvmeSQueue admin_sq;NvmeCQueue admin_cq;NvmeIdCtrl id_ctrl; + +IOThread *iothread; +IOThread internal_iothread_obj; +AioContext *ctx; + EventNotifier notifier; } NvmeCtrl;#endif /* HW_NVME_H */
cover vhost-nvme: new qemu nvme backend using nvme target
In reply to Ming LinQuoted 7 lines · Ming Lin
handle_notify(qemu/hw/block/dataplane/virtio-blk.c:126)aio_dispatch(qemu/aio-posix.c:329)aio_poll(qemu/aio-posix.c:474)iothread_run(qemu/iothread.c:45)start_thread(pthread_create.c:312) /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)
Quoted 8 lines
I think I'll have a "
nvme_dev_notify" similar as "handle_notify"static void
nvme_dev_notify(EventNotifier *e){ .... }But then how can I know this notify is for cq or sq?
virtio-blk has a single queue, so it has a single EventNotifier. Your code using multiple EventNotifiers is fine, you can use aio_set_event_notifier multiple times on the same iothread.
Paolo
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 16 lines · Paolo Bonzini
Quoted 14 lines · Ming Lin
handle_notify(qemu/hw/block/dataplane/virtio-blk.c:126)aio_dispatch(qemu/aio-posix.c:329)aio_poll(qemu/aio-posix.c:474)iothread_run(qemu/iothread.c:45)start_thread(pthread_create.c:312) /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)I think I'll have a "
nvme_dev_notify" similar as "handle_notify"static void
nvme_dev_notify(EventNotifier *e){ .... }
Quoted 5 lines
Quoted 1 line
But then how can I know this notify is for cq or sq?
virtio-blk has a single queue, so it has a single EventNotifier. Your code using multiple EventNotifiers is fine, you can use
aio_set_event_notifiermultiple times on the same iothread.
I feel below patch is close to right.
But I got "Co-routine re-entered recursively".
Program received signal SIGABRT, Aborted.0x00007ffff4a45cc9 in __GI_raise (sig=sig at entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 0x00007ffff4a45cc9 in __GI_raise (sig=sig at entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007ffff4a490d8 in __GI_abort () at abort.c:89 #2 0x0000555555b910d1 in qemu_coroutine_enter (co=0x5555577a9ce0, opaque=0x0) at /home/mlin/qemu/util/qemu-coroutine.c:111 #3 0x0000555555b282e0 in bdrv_co_io_em_complete (opaque=0x7fffd94e4a30, ret=0) at /home/mlin/qemu/block/io.c:2282 #4 0x0000555555ab8ba4 in thread_pool_completion_bh (opaque=0x555557d6d440) at /home/mlin/qemu/thread-pool.c:187 #5 0x0000555555ab7ab2 in aio_bh_call (bh=0x555557648110) at /home/mlin/qemu/async.c:64 #6 0x0000555555ab7b88 in aio_bh_poll (ctx=0x5555565b28f0) at /home/mlin/qemu/async.c:92 #7 0x0000555555acb3b6 in aio_dispatch (ctx=0x5555565b28f0) at /home/mlin/qemu/aio-posix.c:305 #8 0x0000555555ab8013 in aio_ctx_dispatch (source=0x5555565b28f0, callback=0x0, user_data=0x0) at /home/mlin/qemu/async.c:231 #9 0x00007ffff575ee04 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x0000555555ac8b35 in glib_pollfds_poll () at /home/mlin/qemu/main-loop.c:211
#11 0x0000555555ac8c36 in os_host_main_loop_wait (timeout=0) at /home/mlin/qemu/main-loop.c:256 #12 0x0000555555ac8cff in main_loop_wait (nonblocking=0) at /home/mlin/qemu/main-loop.c:504 #13 0x00005555558a41d4 in main_loop () at /home/mlin/qemu/vl.c:1920
#14 0x00005555558ac28a in main (argc=21, argv=0x7fffffffe4a8, envp=0x7fffffffe558) at /home/mlin/qemu/vl.c:4681 (gdb)
Would you help to take a look?
hw/block/nvme.c +27 -2
@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h"
@@ -549,7 +551,10 @@ static void nvme_cq_notifier(EventNotifier *e)container_of (e, NvmeCQueue, notifier);event_notifier_test_and_clear (&cq->notifier); + +aio_context_acquire (cq->ctrl->ctx);nvme_post_cqes (cq); +aio_context_release (cq->ctrl->ctx); }static void nvme_init_cq_eventfd (NvmeCQueue *cq)
@@ -558,9 +563,12 @@ static void nvme_init_cq_eventfd(NvmeCQueue *cq)uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); +aio_context_acquire (n->ctx); +aio_set_event_notifier (n->ctx, &cq->notifier, false, + nvme_cq_notifier); +aio_context_release (n->ctx); }static void nvme_sq_notifier (EventNotifier *e)
@@ -569,7 +577,9 @@ static void nvme_sq_notifier(EventNotifier *e)container_of (e, NvmeSQueue, notifier);event_notifier_test_and_clear (&sq->notifier); +aio_context_acquire (sq->ctrl->ctx);nvme_process_sq (sq); +aio_context_release (sq->ctrl->ctx); }static void nvme_init_sq_eventfd (NvmeSQueue *sq)
@@ -578,9 +588,22 @@ static void nvme_init_sq_eventfd(NvmeSQueue *sq)uint16_t offset = sq->sqid *2 * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&sq->notifier,0 ); -event_notifier_set_handler (&sq->notifier, nvme_sq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &sq->notifier); +aio_context_acquire (n->ctx); +aio_set_event_notifier (n->ctx, &sq->notifier, false, + nvme_sq_notifier); +aio_context_release (n->ctx); +} + +static void nvme_init_iothread (NvmeCtrl *n) +{ +object_initialize (&n->internal_iothread_obj, +sizeof (n->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&n->internal_iothread_obj), &error_abort); + n->iothread = &n->internal_iothread_obj; + n->ctx =iothread_get_aio_context (n->iothread); }static uint16_t nvme_set_db_memory (NvmeCtrl *n,const NvmeCmd *cmd)
@@ -595,6 +618,8 @@ static uint16_t nvme_set_db_memory(NvmeCtrl *n, const NvmeCmd *cmd)return NVME_INVALID_MEMORY_ADDRESS |NVME_DNR ; } +nvme_init_iothread (n); +/* This assumes all I/O queues are created before this command is handled. * We skip the admin queues. */ for (i =1 ; i < n->num_queues; i++) {
hw/block/nvme.h +4 -0
@@ -727,6 +727,10 @@ typedef struct NvmeCtrl {NvmeSQueue admin_sq;NvmeCQueue admin_cq;NvmeIdCtrl id_ctrl; + +IOThread *iothread; +IOThread internal_iothread_obj; + AioContext *ctx; } NvmeCtrl;#endif /* HW_NVME_H */
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 63 lines · Ming Lin
Quoted 16 lines · Paolo Bonzini
Quoted 14 lines · Ming Lin
handle_notify(qemu/hw/block/dataplane/virtio-blk.c:126)aio_dispatch(qemu/aio-posix.c:329)aio_poll(qemu/aio-posix.c:474)iothread_run(qemu/iothread.c:45)start_thread(pthread_create.c:312) /lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)I think I'll have a "
nvme_dev_notify" similar as "handle_notify"static void
nvme_dev_notify(EventNotifier *e){ .... }Quoted 5 lines
Quoted 1 line
But then how can I know this notify is for cq or sq?
virtio-blk has a single queue, so it has a single EventNotifier. Your code using multiple EventNotifiers is fine, you can use
aio_set_event_notifiermultiple times on the same iothread.I feel below patch is close to right.
But I got "Co-routine re-entered recursively".Program received signal SIGABRT, Aborted.
0x00007ffff4a45cc9in__GI_raise(sig=sig at entry=6) at../nptl/sysdeps/unix/sysv/linux/raise.c:56
56../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt#0 0x00007ffff4a45cc9 in __GI_raise (sig=sig at entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007ffff4a490d8 in __GI_abort () at abort.c:89 #2 0x0000555555b910d1 in qemu_coroutine_enter (co=0x5555577a9ce0, opaque=0x0) at /home/mlin/qemu/util/qemu-coroutine.c:111 #3 0x0000555555b282e0 in bdrv_co_io_em_complete (opaque=0x7fffd94e4a30, ret=0) at /home/mlin/qemu/block/io.c:2282 #4 0x0000555555ab8ba4 in thread_pool_completion_bh (opaque=0x555557d6d440) at /home/mlin/qemu/thread-pool.c:187 #5 0x0000555555ab7ab2 in aio_bh_call (bh=0x555557648110) at /home/mlin/qemu/async.c:64 #6 0x0000555555ab7b88 in aio_bh_poll (ctx=0x5555565b28f0) at /home/mlin/qemu/async.c:92 #7 0x0000555555acb3b6 in aio_dispatch (ctx=0x5555565b28f0) at /home/mlin/qemu/aio-posix.c:305 #8 0x0000555555ab8013 in aio_ctx_dispatch (source=0x5555565b28f0, callback=0x0, user_data=0x0) at /home/mlin/qemu/async.c:231 #9 0x00007ffff575ee04 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0#10
0x0000555555ac8b35inglib_pollfds_poll() at/home/mlin/qemu/main-loop.c:211
#110x0000555555ac8c36inos_host_main_loop_wait(timeout=0) at/home/mlin/qemu/main-loop.c:256#120x0000555555ac8cffinmain_loop_wait(nonblocking=0) at/home/mlin/qemu/main-loop.c:504#130x00005555558a41d4inmain_loop() at/home/mlin/qemu/vl.c:1920
#140x00005555558ac28ain main (argc=21, argv=0x7fffffffe4a8, envp=0x7fffffffe558) at/home/mlin/qemu/vl.c:4681(gdb)Would you help to take a look?
hw/block/nvme.c +2
-0@@ -28,6 +28,8 @@#include "sysemu/sysemu.h" #include "qapi/visitor.h" #include "sysemu/block-backend.h" +#include "sysemu/iothread.h" +#include "qom/object_interfaces.h" #include "nvme.h"
hw/block/nvme.c static void nvme_cq_notifier(EventNotifier *e) · line 557
Quoted 7 lines
hw/block/nvme.c +3
-0@@ -549,7 +551,10 @@ static void nvme_cq_notifier(EventNotifier *e)container_of (e, NvmeCQueue, notifier);event_notifier_test_and_clear (&cq->notifier); + +aio_context_acquire (cq->ctrl->ctx);nvme_post_cqes (cq); +aio_context_release (cq->ctrl->ctx);
This is not yet needed in upstream.
hw/block/nvme.c static void nvme_init_cq_eventfd(NvmeCQueue *cq) · line 569
Quoted 12 lines
hw/block/nvme.c +2
-1}static void nvme_init_cq_eventfd (NvmeCQueue *cq)@@ -558,9 +563,12 @@ static void nvme_init_cq_eventfd(NvmeCQueue *cq)uint16_t offset = (cq->cqid*2 +1 ) * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&cq->notifier,0 ); -event_notifier_set_handler (&cq->notifier, nvme_cq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &cq->notifier); +aio_context_acquire (n->ctx); + aio_set_event_notifier(n->ctx, &cq->notifier, false,
This should be true, but shouldn't affect your bug.
hw/block/nvme.c static void nvme_sq_notifier(EventNotifier *e) · line 582
Quoted 12 lines
hw/block/nvme.c +4
-0+ nvme_cq_notifier); +aio_context_release (n->ctx); }static void nvme_sq_notifier (EventNotifier *e)@@ -569,7 +577,9 @@ static void nvme_sq_notifier(EventNotifier *e)container_of (e, NvmeSQueue, notifier);event_notifier_test_and_clear (&sq->notifier); +aio_context_acquire (sq->ctrl->ctx);nvme_process_sq (sq); +aio_context_release (sq->ctrl->ctx);
Same as above.
hw/block/nvme.c static void nvme_init_sq_eventfd(NvmeSQueue *sq) · line 594
Quoted 12 lines
hw/block/nvme.c +2
-1}static void nvme_init_sq_eventfd (NvmeSQueue *sq)@@ -578,9 +588,22 @@ static void nvme_init_sq_eventfd(NvmeSQueue *sq)uint16_t offset = sq->sqid *2 * (4 <<NVME_CAP_DSTRD (n->bar.cap));event_notifier_init (&sq->notifier,0 ); -event_notifier_set_handler (&sq->notifier, nvme_sq_notifier);memory_region_add_eventfd (&n->iomem,0x1000 + offset,4 , false,0 , &sq->notifier); +aio_context_acquire (n->ctx); + aio_set_event_notifier(n->ctx, &sq->notifier, false,
Also true.
hw/block/nvme.c static void nvme_init_sq_eventfd(NvmeSQueue *sq) · line 606
Quoted 12 lines
hw/block/nvme.c +12
-0+ nvme_sq_notifier); +aio_context_release (n->ctx); +} + +static void nvme_init_iothread (NvmeCtrl *n) +{ +object_initialize (&n->internal_iothread_obj, +sizeof (n->internal_iothread_obj), +TYPE_IOTHREAD ); +user_creatable_complete (OBJECT (&n->internal_iothread_obj), &error_abort); + n->iothread = &n->internal_iothread_obj; + n->ctx =iothread_get_aio_context (n->iothread);
Do you still have a blk_set_aio_context somewhere? I'm losing track of the changes.
In any case, I think using a separate I/O thread is a bit premature, except for benchmarking. In the meanwhile I think the best option is to post a two-patch series with the vendor extension and the ioeventfd respectively.
Paolo
Quoted 29 lines
hw/block/nvme.c +2
-0}static uint16_t nvme_set_db_memory (NvmeCtrl *n,const NvmeCmd *cmd)@@ -595,6 +618,8 @@ static uint16_t nvme_set_db_memory(NvmeCtrl *n, const NvmeCmd *cmd)return NVME_INVALID_MEMORY_ADDRESS |NVME_DNR ; } +nvme_init_iothread (n); +/* This assumes all I/O queues are created before this command is handled. * We skip the admin queues. */ for (i =1 ; i < n->num_queues; i++) {hw/block/nvme.h +4
-0@@ -727,6 +727,10 @@ typedef struct NvmeCtrl {NvmeSQueue admin_sq;NvmeCQueue admin_cq;NvmeIdCtrl id_ctrl; + +IOThread *iothread; +IOThread internal_iothread_obj; + AioContext *ctx; } NvmeCtrl;#endif /* HW_NVME_H */
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 2 lines · Paolo Bonzini
Do you still have a
blk_set_aio_contextsomewhere? I'm losing track of the changes.
No.
Quoted 5 lines
In any case, I think using a separate I/O thread is a bit premature, except for benchmarking. In the meanwhile I think the best option is to post a two-patch series with the vendor extension and the ioeventfd respectively.
I'll post.
BTW, I'm not sure about qemu upstream policy. Do I need to first make the kernel side patch upstream?
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-ext
I think the kernel side patch will take a bit longer time to upstream.
Quoted 2 lines
Paolo
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 4 lines · Ming Lin
Quoted 2 lines
Do you still have a
blk_set_aio_contextsomewhere? I'm losing track of the changes.No.
You'll need it. That's what causes your error.
Quoted 3 lines
BTW, I'm not sure about qemu upstream policy. Do I need to first make the kernel side patch upstream?
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-ext
No problem, there's time. QEMU is in freeze right now, and the 2.6 cycle will last until March or so.
Paolo
cover vhost-nvme: new qemu nvme backend using nvme target
In reply to Paolo BonziniQuoted 14 lines · Paolo Bonzini
Quoted 12 lines · Ming Lin
Quoted 11 lines · Paolo Bonzini
Quoted 8 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-extQemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
There is still some bug. I'll update.
Sure.
Fixed it after Thanksgiving holiday :)
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0-ext
Combined with previous test results(lower to higher):
qemu-nvme: 148MB/s
vhost-nvme + google-ext: 230MB/s
qemu-nvme + google-ext + eventfd: 294MB/s
virtio-scsi: 296MB/s
virtio-blk: 344MB/s
"vhost-nvme + google-ext" didn't get good enough performance. Still tuning.
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 7 lines · Ming Lin
qemu-nvme: 148MB/s
vhost-nvme + google-ext: 230MB/s
qemu-nvme + google-ext + eventfd: 294MB/s
virtio-scsi: 296MB/s
virtio-blk: 344MB/s"vhost-nvme + google-ext" didn't get good enough performance.
I'd expect it to be on par of qemu-nvme with ioeventfd but the question
is: why should it be better? For vhost-net, the answer is that more zerocopy can be done if you put the data path in the kernel.
But qemu-nvme is already using io_submit for the data path, perhaps there's not much to gain from vhost-nvme...
Paolo
Quoted 1 line
Still tuning.
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 8 lines · Paolo Bonzini
Quoted 6 lines · Ming Lin
qemu-nvme: 148MB/s
vhost-nvme + google-ext: 230MB/s
qemu-nvme + google-ext + eventfd: 294MB/s
virtio-scsi: 296MB/s
virtio-blk: 344MB/s
Quoted 8 lines
Quoted 1 line
"vhost-nvme + google-ext" didn't get good enough performance.
I'd expect it to be on par of qemu-nvme with ioeventfd but the question
is: why should it be better? For vhost-net, the answer is that more zerocopy can be done if you put the data path in the kernel.But qemu-nvme is already using
io_submitfor the data path, perhaps there's not much to gain from vhost-nvme...
What do you think about virtio-nvme+vhost-nvme?
I also have patch for vritio-nvme:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-split/virtio
Just need to change vhost-nvme to work with it.
Quoted 4 lines
Paolo
Quoted 1 line
Still tuning.
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 2 lines
What do you think about virtio-nvme+vhost-nvme?
What would be the advantage over virtio-blk? Multiqueue is not supported by QEMU but it's already supported by Linux (commit 6a27b656fc).
To me, the advantage of nvme is that it provides more than decent performance on unmodified Windows guests, and thanks to your vendor extension can be used on Linux as well with speeds comparable to virtio-blk. So it's potentially a very good choice for a cloud provider that wants to support Windows guests (together with e.g. a fast SAS emulated controller to replace virtio-scsi, and emulated igb or ixgbe to replace virtio-net).
Which features are supported by NVMe and not virtio-blk?
Paolo
Quoted 12 lines
I also have patch for vritio-nvme:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-split/virtioJust need to change vhost-nvme to work with it.
Quoted 4 lines
Paolo
Quoted 1 line
Still tuning.
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 4 lines · Paolo Bonzini
Quoted 1 line
What do you think about virtio-nvme+vhost-nvme?
What would be the advantage over virtio-blk? Multiqueue is not supported by QEMU but it's already supported by Linux (commit
6a27b656fc).
I expect performance would be better.
Seems google cloud VM uses both nvme and virtio-scsi. Not sure if virtio-blk is also used.
https://cloud.google.com/compute/docs/disks/local-ssd#runscript
Quoted 7 lines
To me, the advantage of nvme is that it provides more than decent performance on unmodified Windows guests, and thanks to your vendor extension can be used on Linux as well with speeds comparable to virtio-blk. So it's potentially a very good choice for a cloud provider that wants to support Windows guests (together with e.g. a fast SAS emulated controller to replace virtio-scsi, and emulated igb or ixgbe to replace virtio-net).
vhost-nvme patches are learned from rts-megasas, which could possibly be a fast SAS emulated controller.
https://github.com/Datera/rts-megasas
Quoted 2 lines
Which features are supported by NVMe and not virtio-blk?
Rob (CCed),
Would you share whether google uses any NVMe specific feature?
Thanks.
cover vhost-nvme: new qemu nvme backend using nvme target
Quoted 7 lines · Ming Lin
Quoted 4 lines · Paolo Bonzini
Quoted 1 line
What do you think about virtio-nvme+vhost-nvme?
What would be the advantage over virtio-blk? Multiqueue is not supported by QEMU but it's already supported by Linux (commit
6a27b656fc).I expect performance would be better.
Why? nvme and virtio-blk are almost the same, even more so with the doorbell extension. virtio is designed to only hit paths that are not slowed down by virtualization. It's really hard to do better, except perhaps with VFIO (and then you don't need your vendor extension).
Quoted 10 lines
Quoted 6 lines
To me, the advantage of nvme is that it provides more than decent performance on unmodified Windows guests, and thanks to your vendor extension can be used on Linux as well with speeds comparable to virtio-blk. So it's potentially a very good choice for a cloud provider that wants to support Windows guests (together with e.g. a fast SAS emulated controller to replace virtio-scsi, and emulated igb or ixgbe to replace virtio-net).
vhost-nvme patches are learned from rts-megasas, which could possibly be a fast SAS emulated controller.
https://github.com/Datera/rts-megasas
Why the hate for userspace? :)
I don't see a reason why vhost-nvme would be faster than a userspace implementation. vhost-blk was never committed upstream for similar
reasons: it lost all the userspace features (snapshots, storage migration, etc.)---which are nice to have and do not cost performance if you do not use them---without any compelling performance gain.
Without the doorbell extension you'd have to go back to userspace on every write and ioctl to vhost (see MEGASAS_IOC_FRAME in rts-megasas). With the doorbell extension you're doing exactly the same work, and then kernel thread vs. userspace thread shouldn't matter much given similar optimization effort. A userspace NVMe, however, will gain all optimization that is done to QEMU's block layer for free. We have done a lot and have more planned.
Quoted 1 line
Quoted 1 line
Which features are supported by NVMe and not virtio-blk?
Having read the driver, the main improvements of NVMe compared to virtio-blk are support for discard and FUA. Discard is easy to add to virtio-blk. In the past the idea was "just use virtio-scsi", but it may be worth adding it now that SSDs are more common.
Thus, FUA is pretty much the only reason for a kernel-based implementation, because it is not exported in userspace. However, does it actually make a difference on real-world workloads? Local SSDs on Google Cloud are not even persistent, so you never need to flush to them.
Paolo
Quoted 8 lines · Ming Lin
One improvment could be to use google's NVMe vendor extension that I send in another thread, aslo here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=nvme-google-extQemu side:
http://www.minggr.net/cgit/cgit.cgi/qemu/log/?h=vhost-nvme.0
Kernel side also here:
https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=vhost-nvme.0
How much do you get with vhost-nvme plus vendor extension, compared to 190 MB/s for QEMU?
Note that in all likelihood, QEMU can actually do better than 190 MB/s, and gain more parallelism too, by moving the processing of the ioeventfds to a separate thread. This is similar to hw/block/dataplane/virtio-blk.c.
It's actually pretty easy to do. Even though hw/block/dataplane/virtio-blk.c is still using some old APIs, all memory access in QEMU is now thread-safe. I have pending patches for 2.6 that cut that file down to a mere 200 lines of code, NVMe would probably be about the same.
Paolo