Re: [PATCH v3] mm/mmu_notifier: prevent unpaired invalidate_start and invalidate_end

[Jason Gunthorpe <jgg@mellanox.com>]

On Tue, Feb 11, 2020 at 01:28:42PM -0800, Ralph Campbell wrote:
> > +/*
> > + * Add a hlist subscription to the list. The list is kept sorted by the
> > + * existence of ops->invalidate_range_end. If there is more than one
> > + * invalidate_range_end in the list then this process can no longer support
> > + * non-blocking invalidation.
> > + *
> > + * non-blocking invalidation is problematic as a requirement to block results in
> > + * the invalidation being aborted, however due to the use of RCU we have no
> > + * reliable way to ensure that every sueessful invalidate_range_start() results
> 
> s/sueessful/successful

woops, yes, two spellos, thanks

> > +{
> > +	struct mmu_notifier *last = NULL;
> > +	struct mmu_notifier *itr;
> > +
> > +	hlist_for_each_entry(itr, &subscriptions->list, hlist)
> > +		last = itr;
> > +
> > +	if (last && last->ops->invalidate_range_end &&
> > +	    subscription->ops->invalidate_range_end) {
> > +		subscriptions->no_blocking = true;
> > +		pr_warn_once(
> > +			"%s (%d) created two mmu_notifier's with invalidate_range_end(): %ps and %ps, non-blocking notifiers disabled\n",
> 
> line length?

Style guide is to keep strings across the 80 cols for grepability

> >   /*
> >    * Same as mmu_notifier_register but here the caller must hold the mmap_sem in
> >    * write mode. A NULL mn signals the notifier is being registered for itree
> > @@ -660,8 +707,8 @@ int __mmu_notifier_register(struct mmu_notifier *subscription,
> >   		subscription->users = 1;
> >   		spin_lock(&mm->notifier_subscriptions->lock);
> > -		hlist_add_head_rcu(&subscription->hlist,
> > -				   &mm->notifier_subscriptions->list);
> > +		mn_hist_add_subscription(mm->notifier_subscriptions,
> > +					 subscription);
> >   		spin_unlock(&mm->notifier_subscriptions->lock);
> >   	} else
> >   		mm->notifier_subscriptions->has_itree = true;
> > 
> 
> Other than some nits, looks good to me so you can add:
> Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>

Great, thanks!

Jason