Re: [Xen-devel] [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

[Jason Gunthorpe <jgg@xxxxxxxxxxxx>]

On Thu, Nov 07, 2019 at 09:00:34PM -0500, Jerome Glisse wrote:
> On Fri, Nov 08, 2019 at 12:32:25AM +0000, Jason Gunthorpe wrote:
> > On Thu, Nov 07, 2019 at 04:04:08PM -0500, Jerome Glisse wrote:
> > > On Thu, Nov 07, 2019 at 08:11:06PM +0000, Jason Gunthorpe wrote:
> > > > On Wed, Nov 06, 2019 at 09:08:07PM -0500, Jerome Glisse wrote:
> > > > 
> > > > > > 
> > > > > > Extra credit: IMHO, this clearly deserves to all be in a new 
> > > > > > mmu_range_notifier.h
> > > > > > header file, but I know that's extra work. Maybe later as a 
> > > > > > follow-up patch,
> > > > > > if anyone has the time.
> > > > > 
> > > > > The range notifier should get the event too, it would be a waste, i 
> > > > > think it is
> > > > > an oversight here. The release event is fine so NAK to you separate 
> > > > > event. Event
> > > > > is really an helper for notifier i had a set of patch for nouveau to 
> > > > > leverage
> > > > > this i need to resucite them. So no need to split thing, i would just 
> > > > > forward
> > > > > the event ie add event to mmu_range_notifier_ops.invalidate() i 
> > > > > failed to catch
> > > > > that in v1 sorry.
> > > > 
> > > > I think what you mean is already done?
> > > > 
> > > > struct mmu_range_notifier_ops {
> > > >         bool (*invalidate)(struct mmu_range_notifier *mrn,
> > > >                            const struct mmu_notifier_range *range,
> > > >                            unsigned long cur_seq);
> > > 
> > > Yes it is sorry, i got confuse with mmu_range_notifier and 
> > > mmu_notifier_range :)
> > > It is almost a palyndrome structure ;)
> > 
> > Lets change the name then, this is clearly not working. I'll reflow
> > everything tomorrow
> 
> Semantic patch to do that run from your linux kernel directory with your patch
> applied (you can run it one patch after the other and the git commit -a 
> --fixup HEAD)
> 
> spatch --sp-file name-of-the-file-below --dir . --all-includes --in-place
> 
> %< ------------------------------------------------------------------
> @@
> @@
> struct
> -mmu_range_notifier
> +mmu_interval_notifier
> 
> @@
> @@
> struct
> -mmu_range_notifier
> +mmu_interval_notifier
> {...};
> 
> // Change mrn name to mmu_in
> @@
> struct mmu_interval_notifier *mrn;
> @@
> -mrn
> +mmu_in
> 
> @@
> identifier fn;
> @@
> fn(..., 
> -struct mmu_interval_notifier *mrn,
> +struct mmu_interval_notifier *mmu_in,
> ...) {...}
> 
> You need coccinelle (which provides spatch). It is untested but it should work
> also i could not come up with a nice name to update mrn as min is way too
> confusing. If you have better name feel free to use it.

I used 'mni' as we already use 'mn' to refer to the notifier, and
'mmu_in' looks like some input parameter or something

It mostly worked, lots of comments to fix manually though:

https://github.com/jgunthorpe/linux/commits/mmu_notifier

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel