On Thu, Dec 9, 2021 at 12:45 AM Mike Christie <michael.christie@xxxxxxxxxx> wrote: > > On 12/7/21 10:07 PM, Jason Wang wrote: > > On Tue, Dec 7, 2021 at 10:45 AM Mike Christie > > <michael.christie@xxxxxxxxxx> 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. You are right. So Acked-by: Jason Wang <jasowang@xxxxxxxxxx> > > > > > > Is this safe or the kthread_stop() in vhost_dev_cleanup() makes us safe? > > > > Thanks > > > >> > >> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx> > >> --- > >> 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 Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization