Nouveau - Nov 2019 - [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert

On Tue, Nov 05, 2019 at 10:16:46AM -0500, Boris Ostrovsky wrote:
> > So, I suppose it can be relaxed to a null test and a WARN_ON that it
> > hasn't changed?
> 
> You mean
> 
> if (use_ptemod) {
> ??????? WARN_ON(map->vma != vma);
> ??????? ...
> 
> 
> Yes, that sounds good.
I amended my copy of the patch with the above, has this rework shown
signs of working?

@@ -436,7 +436,8 @@ 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 && map->vma == vma) {
+       if (use_ptemod) {
+               WARN_ON(map->vma != vma);
                mmu_range_notifier_remove(&map->notifier);
                map->vma = NULL;
        }

Jason

On 11/7/19 3:36 PM, Jason Gunthorpe wrote:
> On Tue, Nov 05, 2019 at 10:16:46AM -0500, Boris Ostrovsky wrote:
>
>>> So, I suppose it can be relaxed to a null test and a WARN_ON that
it
>>> hasn't changed?
>> You mean
>>
>> if (use_ptemod) {
>> ??????? WARN_ON(map->vma != vma);
>> ??????? ...
>>
>>
>> Yes, that sounds good.
> I amended my copy of the patch with the above, has this rework shown
> signs of working?
Yes, it works fine.

But please don't forget notifier ops initialization.

With those two changes,

Reviewed-by: Boris Ostrovsky <boris.ostrovsky at oracle.com>
>
> @@ -436,7 +436,8 @@ 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 && map->vma == vma) {
> +       if (use_ptemod) {
> +               WARN_ON(map->vma != vma);
>                 mmu_range_notifier_remove(&map->notifier);
>                 map->vma = NULL;
>         }
>
> Jason

On Thu, Nov 07, 2019 at 05:54:52PM -0500, Boris Ostrovsky
wrote:
> On 11/7/19 3:36 PM, Jason Gunthorpe wrote:
> > On Tue, Nov 05, 2019 at 10:16:46AM -0500, Boris Ostrovsky wrote:
> >
> >>> So, I suppose it can be relaxed to a null test and a WARN_ON
that it
> >>> hasn't changed?
> >> You mean
> >>
> >> if (use_ptemod) {
> >> ??????? WARN_ON(map->vma != vma);
> >> ??????? ...
> >>
> >>
> >> Yes, that sounds good.
> > I amended my copy of the patch with the above, has this rework shown
> > signs of working?
> 
> Yes, it works fine.
> 
> But please don't forget notifier ops initialization.
> 
> With those two changes,
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky at oracle.com>
Thanks, I got both things. I'll forward this toward linux-next and
repost a v3 

Jason