On Thu, Jun 06, 2019 at 08:37:42PM -0700, John Hubbard wrote: > On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > > > Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON > > and poison bytes to detect this condition. > > > > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > Reviewed-by: Jérôme Glisse <jglisse@xxxxxxxxxx> > > v2 > > - Keep range start/end valid after unregistration (Jerome) > > mm/hmm.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/mm/hmm.c b/mm/hmm.c > > index 6802de7080d172..c2fecb3ecb11e1 100644 > > +++ b/mm/hmm.c > > @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) > > struct hmm *hmm = range->hmm; > > > > /* Sanity check this really should not happen. */ > > That comment can also be deleted, as it has the same meaning as > the WARN_ON() that you just added. > > > - if (hmm == NULL || range->end <= range->start) > > + if (WARN_ON(range->end <= range->start)) > > return; > > > > mutex_lock(&hmm->lock); > > @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) > > range->valid = false; > > mmput(hmm->mm); > > hmm_put(hmm); > > - range->hmm = NULL; > > + > > + /* The range is now invalid, leave it poisoned. */ > > To be precise, we are poisoning the range's back pointer to it's > owning hmm instance. Maybe this is clearer: > > /* > * The range is now invalid, so poison it's hmm pointer. > * Leave other range-> fields in place, for the caller's use. > */ > > ...or something like that? > > > + range->valid = false; > > + memset(&range->hmm, POISON_INUSE, sizeof(range->hmm)); > > } > > EXPORT_SYMBOL(hmm_range_unregister); > > > > > > The above are very minor documentation points, so: > > Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx> done thanks Jason
