Re: [PATCH V4 4/6] vhost_vdpa: implement IRQ offloading in vhost_vdpa

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

 




On 2020/7/28 下午5:18, Zhu, Lingshan wrote:

       * status to 0.
@@ -167,6 +220,15 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
      if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
          return -EINVAL;
  +    /* vq irq is not expected to be changed once DRIVER_OK is set */


So this basically limit the usage of get_vq_irq() in the context vhost_vdpa_set_status() and other vDPA bus drivers' set_status(). If this is true, there's even no need to introduce any new config ops but just let set_status() to return the irqs used for the device. Or if we want this to be more generic, we need vpda's own irq manager (which should be similar to irq bypass manager). That is:
I think there is no need for a driver to free / re-request its irqs after DRIVER_OK though it can do so. If a driver changed its irq of a vq after DRIVER_OK, the vq is still operational but will lose irq offloading that is reasonable.
If we want set_status() return irqs, we need to record the irqs somewhere in vdpa_device,


Why, we can simply pass an array to the driver I think?

void (*set_status)(struct vdpa_device *vdev, u8 status, int *irqs);


as we discussed in a previous thread, this may need initialize and cleanup works, so a new ops
with proper comments (don't say they could not change irq, but highlight if irq changes, irq offloading will not work till next DRIVER_OK) could be more elegant.
However if we really need to change irq after DRIVER_OK, I think maybe we still need vDPA vq irq allocate / free helpers, then the helpers can not be used in probe() as we discussed before, it is a step back to V3 series.


Still, it's not about whether driver may change irq after DRIVER_OK but implication of the assumption. If one bus ops must be called in another ops, it's better to just implement them in one ops.



- bus driver can register itself as consumer
- vDPA device driver can register itself as producer
- matching via queue index
IMHO, is it too heavy for this feature,


Do you mean LOCs? We can:

1) refactor irq bypass manager
2) invent it our own (a much simplified version compared to bypass manager)
3) enforcing them via vDPA bus

Each of the above should be not a lot of coding. I think method 3 is partially done in your previous series but in an implicit manner:

- bus driver that has alloc_irq/free_irq implemented could be implicitly treated as consumer registering
- every vDPA device driver could be treated as producer
- vdpa_devm_alloc_irq() could be treated as producer registering
- alloc_irq/free_irq is the add_producer/del_procuer

We probably just lack some synchronization with driver probe/remove.


and how can they match if two individual adapters both have vq idx = 1.


The matching is per vDPA device.

Thanks


Thanks!
- deal with registering/unregistering of consumer/producer

So there's no need to care when or where the vDPA device driver to allocate the irq, and we don't need to care at which context the vDPA bus driver can use the irq. Either side may get notified when the other side is gone (though we only care about the gone of producer probably).

Any thought on this?

Thanks






[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