On 17.01.22 08:55, Michael S. Tsirkin wrote: > On Mon, Jan 17, 2022 at 02:40:11PM +0800, Jason Wang wrote: >> >> 在 2022/1/15 上午5:43, Michael S. Tsirkin 写道: >>> A common pattern for device reset is currently: >>> vdev->config->reset(vdev); >>> .. cleanup .. >>> >>> reset prevents new interrupts from arriving and waits for interrupt >>> handlers to finish. >>> >>> However if - as is common - the handler queues a work request which is >>> flushed during the cleanup stage, we have code adding buffers / trying >>> to get buffers while device is reset. Not good. >>> >>> This was reproduced by running >>> modprobe virtio_console >>> modprobe -r virtio_console >>> in a loop, and this reasoning seems to apply to virtio mem though >>> I could not reproduce it there. >>> >>> Fix this up by calling virtio_break_device + flush before reset. >>> >>> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx> >>> --- >>> drivers/virtio/virtio_mem.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c >>> index 38becd8d578c..33b8a118a3ae 100644 >>> --- a/drivers/virtio/virtio_mem.c >>> +++ b/drivers/virtio/virtio_mem.c >>> @@ -2888,6 +2888,8 @@ static void virtio_mem_remove(struct virtio_device *vdev) >>> virtio_mem_deinit_hotplug(vm); >>> /* reset the device and cleanup the queues */ >>> + virtio_break_device(vdev); >>> + flush_work(&vm->wq); >> >> >> We set vm->removing to true and call cancel_work_sync() in >> virtio_mem_deinit_hotplug(). Isn't is sufficient? >> >> Thanks > > > Hmm I think you are right. David, I will drop this for now. > Up to you to consider whether some central capability will be > helpful as a replacement for the virtio-mem specific "removing" flag. It's all a bit tricky because we also have to handle pending timers and pending memory onlining/offlining operations in a controlled way. Maybe we could convert to virtio_break_device() and use the &dev->vqs_list_lock as a replacement for the removal_lock. However, I'm not 100% sure if it's nice to use that lock from drivers/virtio/virtio_mem.c directly. -- Thanks, David / dhildenb _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
