From: Martin Pieuchot <mpi@grenadille.net>
Subject: Re: uvm_fault: logic for using shared lock
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Tue, 24 Dec 2024 09:11:04 +0100

On 23/12/24(Mon) 10:28, Theo Buehler wrote:
> > > @@ -1412,17 +1417,21 @@ uvm_fault_lower(struct uvm_faultinfo *uf
> > >  		}
> > >  	}
> > >  
> > > -	/* note: pg is either the uobjpage or the new page in the new anon */
> > > +	/*
> > > +	 * anon must be write locked (promotion).  uobj can be either.
> > > +	 *
> > > +	 * Note: pg is either the uobjpage or the new page in the new anon.
> > > +	 */
> > > +	KASSERT(amap == NULL ||
> > > +	    rw_write_held(amap->am_lock));
> > > +	KASSERT(uobj == NULL ||
> > > +	    rw_status(uobj->vmobjlock) == flt->lower_lock_type);
> > > +	KASSERT(anon == NULL || anon->an_lock == amap->am_lock);
> 
> Previously this last KASSERT was only reachable if amap != NULL. So
> perhaps this should reflect that and add a NULL check before dereferencing
> it? But maybe hitting an assert or crashing because of NULL deref makes
> no difference?

Thanks for the review. 

Yes we could do as you suggest.  However when anon is non-NULl then amap
MUST be valid.

Since the aim of these asserts is mostly to ease comparison with NetBSD
I'll keep them as they are.