Messages in this thread | | | Date | Thu, 7 Apr 2022 14:25:44 +0800 | Subject | Re: [PATCH V2 3/5] virtio: introduce config op to synchronize vring callbacks | From | Jason Wang <> |
| |
在 2022/4/6 下午7:59, Michael S. Tsirkin 写道: > On Wed, Apr 06, 2022 at 04:35:36PM +0800, Jason Wang wrote: >> This patch introduce > introduces > >> a new > new > >> virtio config ops to vring >> callbacks. Transport specific method is required to call >> synchornize_irq() on the IRQs. For the transport that doesn't provide >> synchronize_vqs(), use synchornize_rcu() as a fallback. >> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: "Paul E. McKenney" <paulmck@kernel.org> >> Cc: Marc Zyngier <maz@kernel.org> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> include/linux/virtio_config.h | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h >> index b341dd62aa4d..08b73d9bbff2 100644 >> --- a/include/linux/virtio_config.h >> +++ b/include/linux/virtio_config.h >> @@ -57,6 +57,8 @@ struct virtio_shm_region { >> * include a NULL entry for vqs unused by driver >> * Returns 0 on success or error status >> * @del_vqs: free virtqueues found by find_vqs(). >> + * @synchronize_vqs: synchronize with the virtqueue callbacks. >> + * vdev: the virtio_device > I think I prefer synchronize_callbacks
Ok, I will rename it.
> >> * @get_features: get the array of feature bits for this device. >> * vdev: the virtio_device >> * Returns the first 64 feature bits (all we currently need). >> @@ -89,6 +91,7 @@ struct virtio_config_ops { >> const char * const names[], const bool *ctx, >> struct irq_affinity *desc); >> void (*del_vqs)(struct virtio_device *); >> + void (*synchronize_vqs)(struct virtio_device *); >> u64 (*get_features)(struct virtio_device *vdev); >> int (*finalize_features)(struct virtio_device *vdev); >> const char *(*bus_name)(struct virtio_device *vdev); >> @@ -217,6 +220,19 @@ int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs, >> desc); >> } >> >> +/** >> + * virtio_synchronize_vqs - synchronize with virtqueue callbacks >> + * @vdev: the device >> + */ >> +static inline >> +void virtio_synchronize_vqs(struct virtio_device *dev) >> +{ >> + if (dev->config->synchronize_vqs) >> + dev->config->synchronize_vqs(dev); >> + else >> + synchronize_rcu(); > I am not sure about this fallback and the latency impact.
Unless each transport can implement their own synchronization routine, we need something that can do best effort as fallback here.
> Maybe synchronize_rcu_expedited is better here.
Not sure, it might lead IPIs and according to the Documentation/RCU/checklist.rst:
"""
The expedited forms of these primitives have the same semantics as the non-expedited forms, but expediting is both expensive and (with the exception of synchronize_srcu_expedited()) unfriendly to real-time workloads. Use of the expedited primitives should be restricted to rare configuration-change operations that would not normally be undertaken while a real-time workload is running. However, real-time workloads can use rcupdate.rcu_normal kernel boot parameter to completely disable expedited grace periods, though this might have performance implications.
"""
It will be expensive for real time workloads.
Thanks
> >> +} >> + >> /** >> * virtio_device_ready - enable vq use in probe function >> * @vdev: the device >> -- >> 2.25.1
| |