On Wed, Dec 11, 2019 at 10:57:13PM +0000, Jason Gunthorpe wrote: > On Thu, Dec 05, 2019 at 11:03:24AM -0500, Jerome Glisse wrote: > > > > struct mmu_notifier_mm (ie the mm->mmu_notifier_mm) > > > -> mmn_mm > > > struct mm_struct > > > -> mm > > > struct mmu_notifier (ie the user subscription to the mm_struct) > > > -> mn > > > struct mmu_interval_notifier (the other kind of user subscription) > > > -> mni > > > > What about "interval" the context should already tell people > > it is related to mmu notifier and thus a notifier. I would > > just remove the notifier suffix, this would match the below > > range. > > Interval could be a good replacement for mni in the mm/mmu_notififer > file if we don't do the wholesale rename > > > > I think it would be overall nicer with better names for the original > > > structs. Perhaps: > > > > > > mmn_* - MMU notifier prefix > > > mmn_state <- struct mmu_notifier_mm > > > mmn_subscription (mmn_sub) <- struct mmu_notifier > > > mmn_range_subscription (mmn_range_sub) <- struct mmu_interval_notifier > > > mmn_invalidate_desc <- struct mmu_notifier_range > > > > This looks good. > > Well, lets just bite the bullet then and switch it. Do you like > 'state'? I thought that was the weakest one Since you're asking, here's my bikeshed. I kinda agree _state looks a bit strange for this, what about a _link suffix in the spirit of struct list_head link; The other common name is "node", but I think that's confusing in the context of mm code. The purpose of this struct is to link everything together (and yes it carries also some state, but the main job is to link a mm_struct to a mmu_notifier). At least for me a _state is configuration state for a specific object, not something that links a bunch of things together. But I'm biased on this, since we use that pattern in drm for all the display state tracking. Also feel free to ignore my bikeshed :-) Aside from this I think the proposed names are a solid improvement. -Daniel > > We could use mmnotif as the prefix, this makes the longest: > > struct mmnotif_range_subscription > > Which is reasonable enough > > > Maybe we can do a semantic patch to do convertion and then Linus > > can easily apply the patch by just re-running the coccinelle. > > I tried this last time I renamed everything, it was OK, but it missed > updating the comments. So it still needs some by-hand helping. > > I'll make some patches next week when I get back. > > Jason > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
