On Sat, Jun 15, 2019 at 06:59:06AM -0700, Christoph Hellwig wrote:
> On Thu, Jun 13, 2019 at 09:44:40PM -0300, Jason Gunthorpe wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> >
> > Ralph observes that hmm_range_register() can only be called by a driver
> > while a mirror is registered. Make this clear in the API by passing in the
> > mirror structure as a parameter.
> >
> > This also simplifies understanding the lifetime model for struct hmm, as
> > the hmm pointer must be valid as part of a registered mirror so all we
> > need in hmm_register_range() is a simple kref_get.
>
> Looks good, at least an an intermediate step:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>
> > index f6956d78e3cb25..22a97ada108b4e 100644
> > +++ b/mm/hmm.c
> > @@ -914,13 +914,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
> > * Track updates to the CPU page table see include/linux/hmm.h
> > */
> > int hmm_range_register(struct hmm_range *range,
> > - struct mm_struct *mm,
> > + struct hmm_mirror *mirror,
> > unsigned long start,
> > unsigned long end,
> > unsigned page_shift)
> > {
> > unsigned long mask = ((1UL << page_shift) - 1UL);
> > - struct hmm *hmm;
> > + struct hmm *hmm = mirror->hmm;
> >
> > range->valid = false;
> > range->hmm = NULL;
> > @@ -934,20 +934,15 @@ int hmm_range_register(struct hmm_range *range,
> > range->start = start;
> > range->end = end;
>
> But while you're at it: the calling conventions of hmm_range_register
> are still rather odd, as the staet, end and page_shift arguments are
> only used to fill out fields in the range structure passed in. Might
> be worth cleaning up as well if we change the calling convention.
I'm thinking to tackle that as part of the mmu notififer invlock
idea.. Once the range looses the lock then we don't really need to
register it at all.
Thanks,
Jason
