[PATCH 2/2] drm/i915/gvt: Fix inconsistent locks holding sequence

[Dong, Chuanxiao]

> -----Original Message-----
> From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com]
> Sent: Tuesday, June 20, 2017 10:52 PM
> To: Dong, Chuanxiao <chuanxiao.dong at intel.com>
> Cc: intel-gvt-dev at lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/i915/gvt: Fix inconsistent locks holding
> sequence
> 
> On 2017.06.20 14:23:22 +0000, Dong, Chuanxiao wrote:
> > > > In this case, rb_first() won't be protected by cache_lock for getting the
> > > node.  We may still want this to be protected.
> > > >
> > >
> > > oh, yeah, you're right, might just use for(;;) though.
> >
> > Backing to your suggestion, how about change to below:
> >
> > mutex_lock(&cache_lock);
> > while ((node = rb_first(...))) {
> >       dma = rb_entry(node, struct gvt_dma, node);
> >       gvt_dma_unmap_iova(vgpu, dma->iova);
> >       gfn = dma->gfn;
> >       __gvt_cache_remove_entry(vgpu, dma);
> >       mutex_unlock(&cache_lock);
> >       vfio_unpin_pages(dev, &gfn, 1);
> >      mutex_lock(&cache_lock);
> > }
> > mutex_unlock(&cache_lock);
> >
> 
> Not nice, that's why I suggest to use for style.

I see. So it should be:
for(;;) {
      mutex_lock(&cache_lock);
      node = rb_first(...);
      if (!node) {
          mutex_unlock(&vgpu->vdev.cache_lock);
          break;
      }
      dma = rb_entry(node, struct gvt_dma, node);
      gvt_dma_unmap_iova(vgpu, dma->iova);
       gfn = dma->gfn;
       __gvt_cache_remove_entry(vgpu, dma);
       mutex_unlock(&cache_lock);
       vfio_unpin_pages(dev, &gfn, 1);
}

Will send out v2 for this one.

Thanks
Chuanxiao