On 01/10/20(Thu) 14:18, Martin Pieuchot wrote:
> Use more KASSERT()s instead of the "if (x) panic()" idiom for sanity
> checks and add a couple of local variables to reduce the difference
> with NetBSD and help for upcoming locking.
deraadt@ mentioned that KASSERT()s are not effective in RAMDISK kernels.
So the revisited diff below only converts checks that are redundant with
NULL dereferences.
ok?
Index: uvm/uvm_amap.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
retrieving revision 1.84
diff -u -p -r1.84 uvm_amap.c
--- uvm/uvm_amap.c 25 Sep 2020 08:04:48 -0000 1.84
+++ uvm/uvm_amap.c 7 Oct 2020 14:40:53 -0000
@@ -669,9 +669,7 @@ ReStart:
pg = anon->an_page;
/* page must be resident since parent is wired */
- if (pg == NULL)
- panic("amap_cow_now: non-resident wired page"
- " in anon %p", anon);
+ KASSERT(pg != NULL);
/*
* if the anon ref count is one, we are safe (the child
@@ -740,6 +738,7 @@ ReStart:
void
amap_splitref(struct vm_aref *origref, struct vm_aref *splitref, vaddr_t
offset)
{
+ struct vm_amap *amap = origref->ar_amap;
int leftslots;
AMAP_B2SLOT(leftslots, offset);
@@ -747,17 +746,18 @@ amap_splitref(struct vm_aref *origref, s
panic("amap_splitref: split at zero offset");
/* now: we have a valid am_mapped array. */
- if (origref->ar_amap->am_nslot - origref->ar_pageoff - leftslots <= 0)
+ if (amap->am_nslot - origref->ar_pageoff - leftslots <= 0)
panic("amap_splitref: map size check failed");
#ifdef UVM_AMAP_PPREF
- /* establish ppref before we add a duplicate reference to the amap */
- if (origref->ar_amap->am_ppref == NULL)
- amap_pp_establish(origref->ar_amap);
+ /* Establish ppref before we add a duplicate reference to the amap. */
+ if (amap->am_ppref == NULL)
+ amap_pp_establish(amap);
#endif
- splitref->ar_amap = origref->ar_amap;
- splitref->ar_amap->am_ref++; /* not a share reference */
+ /* Note: not a share reference. */
+ amap->am_ref++;
+ splitref->ar_amap = amap;
splitref->ar_pageoff = origref->ar_pageoff + leftslots;
}
@@ -1104,12 +1104,11 @@ amap_add(struct vm_aref *aref, vaddr_t o
slot = UVM_AMAP_SLOTIDX(slot);
if (replace) {
- if (chunk->ac_anon[slot] == NULL)
- panic("amap_add: replacing null anon");
- if (chunk->ac_anon[slot]->an_page != NULL &&
- (amap->am_flags & AMAP_SHARED) != 0) {
- pmap_page_protect(chunk->ac_anon[slot]->an_page,
- PROT_NONE);
+ struct vm_anon *oanon = chunk->ac_anon[slot];
+
+ KASSERT(oanon != NULL);
+ if (oanon->an_page && (amap->am_flags & AMAP_SHARED) != 0) {
+ pmap_page_protect(oanon->an_page, PROT_NONE);
/*
* XXX: suppose page is supposed to be wired somewhere?
*/
@@ -1138,14 +1137,13 @@ amap_unadd(struct vm_aref *aref, vaddr_t
AMAP_B2SLOT(slot, offset);
slot += aref->ar_pageoff;
- KASSERT(slot < amap->am_nslot);
+ if (chunk->ac_anon[slot] == NULL)
+ panic("amap_unadd: nothing there");
chunk = amap_chunk_get(amap, slot, 0, PR_NOWAIT);
- if (chunk == NULL)
- panic("amap_unadd: chunk for slot %d not present", slot);
+ KASSERT(chunk != NULL);
slot = UVM_AMAP_SLOTIDX(slot);
- if (chunk->ac_anon[slot] == NULL)
- panic("amap_unadd: nothing there");
+ KASSERT(chunk->ac_anon[slot] != NULL);
chunk->ac_anon[slot] = NULL;
chunk->ac_usedmap &= ~(1 << slot);
