On 12/7/21 10:07 PM, Jason Wang wrote: > On Tue, Dec 7, 2021 at 10:45 AM Mike Christie > <[email protected]> wrote: >> >> The flush after vhost_dev_cleanup is not needed because: >> >> 1. It doesn't do anything. vhost_dev_cleanup will stop the worker thread >> so the flush call will just return since the worker has not device. >> >> 2. It's not needed for the re-queue case. vhost_scsi_evt_handle_kick grabs >> the mutex and if the backend is NULL will return without queueing a work. >> vhost_scsi_clear_endpoint will set the backend to NULL under the vq->mutex >> then drops the mutex and does a flush. So we know when >> vhost_scsi_clear_endpoint has dropped the mutex after clearing the backend >> no evt related work will be able to requeue. The flush would then make sure >> any queued evts are run and return. > > What happens if a kick after vhost_scsi_clear_endpoint() but before > vhost_dev_stop()?
vhost_dev_stop also does a flush, so: 1. The kick handler would see the backend is null and return immediately. 2. The flush in vhost_dev_stop would wait for those kicks in #1 to complete. > > Is this safe or the kthread_stop() in vhost_dev_cleanup() makes us safe? > > Thanks > >> >> Signed-off-by: Mike Christie <[email protected]> >> --- >> drivers/vhost/scsi.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c >> index 532e204f2b1b..94535c813ef7 100644 >> --- a/drivers/vhost/scsi.c >> +++ b/drivers/vhost/scsi.c >> @@ -1827,8 +1827,6 @@ static int vhost_scsi_release(struct inode *inode, >> struct file *f) >> vhost_scsi_clear_endpoint(vs, &t); >> vhost_dev_stop(&vs->dev); >> vhost_dev_cleanup(&vs->dev); >> - /* Jobs can re-queue themselves in evt kick handler. Do extra flush. >> */ >> - vhost_scsi_flush(vs); >> kfree(vs->dev.vqs); >> kvfree(vs); >> return 0; >> -- >> 2.25.1 >> > _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
