Mike Christie
2021-Dec-07 02:45 UTC
[PATCH 6/7] vhost-scsi: drop flush after vhost_dev_cleanup
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. Signed-off-by: Mike Christie <michael.christie at oracle.com> --- 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
Jason Wang
2021-Dec-08 04:07 UTC
[PATCH 6/7] vhost-scsi: drop flush after vhost_dev_cleanup
On Tue, Dec 7, 2021 at 10:45 AM Mike Christie <michael.christie at oracle.com> 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()? Is this safe or the kthread_stop() in vhost_dev_cleanup() makes us safe? Thanks> > Signed-off-by: Mike Christie <michael.christie at oracle.com> > --- > 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 >