Re: [PATCH 6/7] vhost-scsi: drop flush after vhost_dev_cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux