On Wed, Nov 04, 2020 at 04:12:45PM -0800, Ira Weiny wrote:
> > -int hfi1_mmu_rb_register(void *ops_arg, struct mm_struct *mm,
> > +int hfi1_mmu_rb_register(void *ops_arg,
> > struct mmu_rb_ops *ops,
> > struct workqueue_struct *wq,
> > struct mmu_rb_handler **handler)
> > @@ -110,18 +98,20 @@ int hfi1_mmu_rb_register(void *ops_arg, struct mm_struct *mm,
> > INIT_HLIST_NODE(&handlr->mn.hlist);
> > spin_lock_init(&handlr->lock);
> > handlr->mn.ops = &mn_opts;
> > - handlr->mm = mm;
> > INIT_WORK(&handlr->del_work, handle_remove);
> > INIT_LIST_HEAD(&handlr->del_list);
> > INIT_LIST_HEAD(&handlr->lru_list);
> > handlr->wq = wq;
> >
> > - ret = mmu_notifier_register(&handlr->mn, handlr->mm);
> > + ret = mmu_notifier_register(&handlr->mn, current->mm);
> > if (ret) {
> > kfree(handlr);
> > return ret;
> > }
> >
> > + mmget(current->mm);
>
> I flagged this initially but then reviewed the commit message for why you need
> this reference. I think it is worth a comment here as well as below.
> Specifically mentioning the order of calls in do_exit().
Oh? a mmget should not be held for a long time, and a notifier already
holds a mmgrab while it is registered
If hfi later needs the mmget then the only the part that needs it
should convert the mmgrab to a mmget with mmget_not_zero
Jason
