On Mon, Nov 04, 2019 at 05:03:31PM -0500, Boris Ostrovsky wrote:
> On 10/28/19 4:10 PM, Jason Gunthorpe wrote:
> > @@ -445,17 +438,9 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
> > struct gntdev_priv *priv = file->private_data;
> >
> > pr_debug("gntdev_vma_close %p\n", vma);
> > - if (use_ptemod) {
> > - /* It is possible that an mmu notifier could be running
> > - * concurrently, so take priv->lock to ensure that the vma won't
> > - * vanishing during the unmap_grant_pages call, since we will
> > - * spin here until that completes. Such a concurrent call will
> > - * not do any unmapping, since that has been done prior to
> > - * closing the vma, but it may still iterate the unmap_ops list.
> > - */
> > - mutex_lock(&priv->lock);
> > + if (use_ptemod && map->vma == vma) {
>
>
> Is it possible for map->vma not to be equal to vma?
It could be NULL at least if use_ptemod is not set.
Otherwise, I'm not sure, the confusing bit is that the map comes from
here:
map = gntdev_find_map_index(priv, index, count);
It looks like the intent is that the map->vma is always set to the
only vma that has the map as private_data.
So, I suppose it can be relaxed to a null test and a WARN_ON that it
hasn't changed?
Jason
