On Fri, Jun 07, 2019 at 01:46:30PM -0700, Ralph Campbell 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> > > Reviewed-by: Ralph Campbell <rcampbell@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. */ > > - if (hmm == NULL || range->end <= range->start) > > + if (WARN_ON(range->end <= range->start)) > > return; > > WARN_ON() is definitely better than silent return but I wonder how > useful it is since the caller shouldn't be modifying the hmm_range > once it is registered. Other fields could be changed too... I deleted the warn on (see the other thread), but I'm confused by your "shouldn't be modified" statement. The only thing that needs to be set and remain unchanged for register is the virtual start/end address. Everything else should be done once it is clear to proceed based on the collision-retry locking scheme this uses. Basically the range register only setups a 'detector' for colliding invalidations. The other stuff in the struct is just random temporary storage for the API. AFAICS at least.. Jason
