Re: [PATCH 3/7] vhost_vdpa: implement IRQ offloading functions in vhost_vdpa

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2020/7/15 下午4:56, Zhu, Lingshan wrote:


On 7/15/2020 4:51 PM, Jason Wang wrote:

On 2020/7/13 下午5:47, Zhu, Lingshan wrote:


On 7/13/2020 4:22 PM, Jason Wang wrote:

On 2020/7/12 下午10:49, Zhu Lingshan wrote:
This patch introduce a set of functions for setup/unsetup
and update irq offloading respectively by register/unregister
and re-register the irq_bypass_producer.

Signed-off-by: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
---
  drivers/vhost/vdpa.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 69 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 2fcc422..92683e4 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -115,6 +115,63 @@ static irqreturn_t vhost_vdpa_config_cb(void *private)
      return IRQ_HANDLED;
  }
  +static void vhost_vdpa_setup_vq_irq(struct vdpa_device *dev, int qid, int irq)
+{
+    struct vhost_vdpa *v = vdpa_get_drvdata(dev);
+    struct vhost_virtqueue *vq = &v->vqs[qid];
+    int ret;
+
+    vq_err(vq, "setup irq bypass for vq %d with irq = %d\n", qid, irq);
+    spin_lock(&vq->call_ctx.ctx_lock);
+    if (!vq->call_ctx.ctx)
+        return;
+
+    vq->call_ctx.producer.token = vq->call_ctx.ctx;
+    vq->call_ctx.producer.irq = irq;
+    ret = irq_bypass_register_producer(&vq->call_ctx.producer);
+    spin_unlock(&vq->call_ctx.ctx_lock);
+
+    if (unlikely(ret))
+        vq_err(vq,
+        "irq bypass producer (token %p registration fails: %d\n",
+        vq->call_ctx.producer.token, ret);


Not sure this deserves a vq_err(), irq will be relayed through eventfd if irq bypass manager can't work.
OK, I see vq_err() will eventfd_signal err_ctx than just print a message, will remove all vq_err().


+}
+
+static void vhost_vdpa_unsetup_vq_irq(struct vdpa_device *dev, int qid)
+{
+    struct vhost_vdpa *v = vdpa_get_drvdata(dev);
+    struct vhost_virtqueue *vq = &v->vqs[qid];
+
+    spin_lock(&vq->call_ctx.ctx_lock);
+ irq_bypass_unregister_producer(&vq->call_ctx.producer);
+    spin_unlock(&vq->call_ctx.ctx_lock);
+
+    vq_err(vq, "unsetup irq bypass for vq %d\n", qid);


Why call vq_err() here?


+}
+
+static void vhost_vdpa_update_vq_irq(struct vhost_virtqueue *vq)
+{
+    struct eventfd_ctx *ctx;
+    void *token;
+
+    spin_lock(&vq->call_ctx.ctx_lock);
+    ctx = vq->call_ctx.ctx;
+    token = vq->call_ctx.producer.token;
+    if (ctx == token)
+        return;


Need do unlock here.
sure!


+
+    if (!ctx && token)
+ irq_bypass_unregister_producer(&vq->call_ctx.producer);
+
+    if (ctx && ctx != token) {
+ irq_bypass_unregister_producer(&vq->call_ctx.producer);
+        vq->call_ctx.producer.token = ctx;
+ irq_bypass_register_producer(&vq->call_ctx.producer);
+    }
+
+    spin_unlock(&vq->call_ctx.ctx_lock);


This should be rare so I'd use simple codes just do unregister and register.

do you mean remove "if (ctx && ctx != token)"? I think this could be useful, we should only update it when ctx!=NULL and ctx!= existing token.


I meant something like:

unregister();
vq->call_ctx.producer.token = ctx;
register();
This is what we are doing now, or I must missed somethig:
if (ctx && ctx != token) {
	irq_bypass_unregister_producer(&vq->call_ctx.producer);
	vq->call_ctx.producer.token = ctx;
	irq_bypass_register_producer(&vq->call_ctx.producer);

}

It just unregister and register.


I meant there's probably no need for the check and another check and unregister before. The whole function is as simple as I suggested above.

Thanks



Thanks,
BR
Zhu Lingshan




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux