NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/56900: panic in uvm_map_findspace
The following reply was made to PR kern/56900; it has been noted by GNATS.
From: Thomas Klausner <wiz%NetBSD.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc:
Subject: Re: kern/56900: panic in uvm_map_findspace
Date: Fri, 24 Jun 2022 20:04:44 +0200
--XwFBqNPNA03tvv54
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
I think it might be related to building openjdk17 or nodejs16, I had
work directories for these left-over after the panics.
And here's the diff I reverted locally to make my system stable again.
Thomas
--XwFBqNPNA03tvv54
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="uvm.diff"
Index: uvm_map.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_map.c,v
retrieving revision 1.394
retrieving revision 1.402
diff -u -r1.394 -r1.402
--- uvm_map.c 10 Apr 2022 09:50:46 -0000 1.394
+++ uvm_map.c 8 Jun 2022 16:55:00 -0000 1.402
@@ -1778,6 +1778,29 @@
return (0);
}
+static void
+uvm_findspace_invariants(struct vm_map *map, vaddr_t orig_hint, vaddr_t length,
+ struct uvm_object *uobj, voff_t uoffset, vsize_t align, int flags,
+ vaddr_t hint, struct vm_map_entry *entry, int line)
+{
+ const int topdown = map->flags & VM_MAP_TOPDOWN;
+
+ KASSERTMSG( topdown || hint >= orig_hint,
+ "map=%p hint=%#"PRIxVADDR" orig_hint=%#"PRIxVADDR
+ " length=%#"PRIxVSIZE" uobj=%p uoffset=%#llx align=%"PRIxVSIZE
+ " flags=%#x entry=%p (uvm_map_findspace line %d)",
+ map, hint, orig_hint,
+ length, uobj, (unsigned long long)uoffset, align,
+ flags, entry, line);
+ KASSERTMSG(!topdown || hint <= orig_hint,
+ "map=%p hint=%#"PRIxVADDR" orig_hint=%#"PRIxVADDR
+ " length=%#"PRIxVSIZE" uobj=%p uoffset=%#llx align=%"PRIxVSIZE
+ " flags=%#x entry=%p (uvm_map_findspace line %d)",
+ map, hint, orig_hint,
+ length, uobj, (unsigned long long)uoffset, align,
+ flags, entry, line);
+}
+
/*
* uvm_map_findspace: find "length" sized space in "map".
*
@@ -1796,10 +1819,14 @@
vaddr_t *result /* OUT */, struct uvm_object *uobj, voff_t uoffset,
vsize_t align, int flags)
{
- struct vm_map_entry *entry;
+#define INVARIANTS() \
+ uvm_findspace_invariants(map, orig_hint, length, uobj, uoffset, align,\
+ flags, hint, entry, __LINE__)
+ struct vm_map_entry *entry = NULL;
struct vm_map_entry *child, *prev, *tmp;
vaddr_t orig_hint __diagused;
const int topdown = map->flags & VM_MAP_TOPDOWN;
+ int avail;
UVMHIST_FUNC(__func__);
UVMHIST_CALLARGS(maphist, "(map=%#jx, hint=%#jx, len=%ju, flags=%#jx...",
(uintptr_t)map, hint, length, flags);
@@ -1813,12 +1840,17 @@
uvm_map_check(map, "map_findspace entry");
/*
- * remember the original hint. if we are aligning, then we
- * may have to try again with no alignment constraint if
- * we fail the first time.
+ * Clamp the hint to the VM map's min/max address, and remmeber
+ * the clamped original hint. Remember the original hint,
+ * clamped to the min/max address. If we are aligning, then we
+ * may have to try again with no alignment constraint if we
+ * fail the first time.
+ *
+ * We use the original hint to verify later that the search has
+ * been monotonic -- that is, nonincreasing or nondecreasing,
+ * according to topdown or !topdown respectively. But the
+ * clamping is not monotonic.
*/
-
- orig_hint = hint;
if (hint < vm_map_min(map)) { /* check ranges ... */
if (flags & UVM_FLAG_FIXED) {
UVMHIST_LOG(maphist,"<- VA below map range",0,0,0,0);
@@ -1831,6 +1863,8 @@
hint, vm_map_min(map), vm_map_max(map), 0);
return (NULL);
}
+ orig_hint = hint;
+ INVARIANTS();
UVMHIST_LOG(maphist,"<- VA %#jx vs range [%#jx->%#jx]",
hint, vm_map_min(map), vm_map_max(map), 0);
@@ -1839,8 +1873,10 @@
* hint may not be aligned properly; we need round up or down it
* before proceeding further.
*/
- if ((flags & UVM_FLAG_COLORMATCH) == 0)
+ if ((flags & UVM_FLAG_COLORMATCH) == 0) {
uvm_map_align_va(&hint, align, topdown);
+ INVARIANTS();
+ }
UVMHIST_LOG(maphist,"<- VA %#jx vs range [%#jx->%#jx]",
hint, vm_map_min(map), vm_map_max(map), 0);
@@ -1870,7 +1906,36 @@
* it, there would be four cases).
*/
- if ((flags & UVM_FLAG_FIXED) == 0 && hint == vm_map_min(map)) {
+ if ((flags & UVM_FLAG_FIXED) == 0 &&
+ hint == (topdown ? vm_map_max(map) : vm_map_min(map))) {
+ /*
+ * The uvm_map_findspace algorithm is monotonic -- for
+ * topdown VM it starts with a high hint and returns a
+ * lower free address; for !topdown VM it starts with a
+ * low hint and returns a higher free address. As an
+ * optimization, start with the first (highest for
+ * topdown, lowest for !topdown) free address.
+ *
+ * XXX This `optimization' probably doesn't actually do
+ * much in practice unless userland explicitly passes
+ * the VM map's minimum or maximum address, which
+ * varies from machine to machine (VM_MAX/MIN_ADDRESS,
+ * e.g. 0x7fbfdfeff000 on amd64 but 0xfffffffff000 on
+ * aarch64) and may vary according to other factors
+ * like sysctl vm.user_va0_disable. In particular, if
+ * the user specifies 0 as a hint to mmap, then mmap
+ * will choose a default address which is usually _not_
+ * VM_MAX/MIN_ADDRESS but something else instead like
+ * VM_MAX_ADDRESS - stack size - guard page overhead,
+ * in which case this branch is never hit.
+ *
+ * In fact, this branch appears to have been broken for
+ * two decades between when topdown was introduced in
+ * ~2003 and when it was adapted to handle the topdown
+ * case without violating the monotonicity assertion in
+ * 2022. Maybe Someone^TM should either ditch the
+ * optimization or find a better way to do it.
+ */
entry = map->first_free;
} else {
if (uvm_map_lookup_entry(map, hint, &entry)) {
@@ -1896,8 +1961,10 @@
/*
* See if given hint fits in this gap.
*/
- switch (uvm_map_space_avail(&hint, length,
- uoffset, align, flags, topdown, entry)) {
+ avail = uvm_map_space_avail(&hint, length,
+ uoffset, align, flags, topdown, entry);
+ INVARIANTS();
+ switch (avail) {
case 1:
goto found;
case -1:
@@ -1928,8 +1995,11 @@
/* Check slot before any entry */
hint = topdown ? entry->next->start - length : entry->end;
- switch (uvm_map_space_avail(&hint, length, uoffset, align, flags,
- topdown, entry)) {
+ INVARIANTS();
+ avail = uvm_map_space_avail(&hint, length, uoffset, align, flags,
+ topdown, entry);
+ INVARIANTS();
+ switch (avail) {
case 1:
goto found;
case -1:
@@ -1996,8 +2066,11 @@
if (hint < tmp->end)
hint = tmp->end;
}
- switch (uvm_map_space_avail(&hint, length, uoffset, align,
- flags, topdown, tmp)) {
+ INVARIANTS();
+ avail = uvm_map_space_avail(&hint, length, uoffset, align,
+ flags, topdown, tmp);
+ INVARIANTS();
+ switch (avail) {
case 1:
entry = tmp;
goto found;
@@ -2018,8 +2091,11 @@
KASSERT(orig_hint <= prev->end);
hint = prev->end;
}
- switch (uvm_map_space_avail(&hint, length, uoffset, align,
- flags, topdown, prev)) {
+ INVARIANTS();
+ avail = uvm_map_space_avail(&hint, length, uoffset, align,
+ flags, topdown, prev);
+ INVARIANTS();
+ switch (avail) {
case 1:
entry = prev;
goto found;
@@ -2059,8 +2135,11 @@
KASSERT(orig_hint <= tmp->end);
hint = tmp->end;
}
- switch (uvm_map_space_avail(&hint, length, uoffset, align,
- flags, topdown, tmp)) {
+ INVARIANTS();
+ avail = uvm_map_space_avail(&hint, length, uoffset, align,
+ flags, topdown, tmp);
+ INVARIANTS();
+ switch (avail) {
case 1:
entry = tmp;
goto found;
@@ -2080,13 +2159,17 @@
* entry->next->start = VA of end of current gap
*/
+ INVARIANTS();
for (;;) {
/* Update hint for current gap. */
hint = topdown ? entry->next->start - length : entry->end;
+ INVARIANTS();
/* See if it fits. */
- switch (uvm_map_space_avail(&hint, length, uoffset, align,
- flags, topdown, entry)) {
+ avail = uvm_map_space_avail(&hint, length, uoffset, align,
+ flags, topdown, entry);
+ INVARIANTS();
+ switch (avail) {
case 1:
goto found;
case -1:
@@ -2115,10 +2198,7 @@
SAVE_HINT(map, map->hint, entry);
*result = hint;
UVMHIST_LOG(maphist,"<- got it! (result=%#jx)", hint, 0,0,0);
- KASSERTMSG( topdown || hint >= orig_hint, "hint: %#jx, orig_hint: %#jx",
- (uintmax_t)hint, (uintmax_t)orig_hint);
- KASSERTMSG(!topdown || hint <= orig_hint, "hint: %#jx, orig_hint: %#jx",
- (uintmax_t)hint, (uintmax_t)orig_hint);
+ INVARIANTS();
KASSERT(entry->end <= hint);
KASSERT(hint + length <= entry->next->start);
return (entry);
@@ -2132,6 +2212,7 @@
UVMHIST_LOG(maphist, "<- failed (notfound)", 0,0,0,0);
return (NULL);
+#undef INVARIANTS
}
/*
@@ -5089,12 +5170,13 @@
entry->aref.ar_pageoff);
(*pr)(
"\tsubmap=%c, cow=%c, nc=%c, prot(max)=%d/%d, inh=%d, "
- "wc=%d, adv=%d\n",
+ "wc=%d, adv=%d%s\n",
(entry->etype & UVM_ET_SUBMAP) ? 'T' : 'F',
(entry->etype & UVM_ET_COPYONWRITE) ? 'T' : 'F',
(entry->etype & UVM_ET_NEEDSCOPY) ? 'T' : 'F',
entry->protection, entry->max_protection,
- entry->inheritance, entry->wired_count, entry->advice);
+ entry->inheritance, entry->wired_count, entry->advice,
+ entry == map->first_free ? " (first_free)" : "");
}
}
Index: uvm_mmap.c
===================================================================
RCS file: /cvsroot/src/sys/uvm/uvm_mmap.c,v
retrieving revision 1.179
retrieving revision 1.180
diff -u -r1.179 -r1.180
--- uvm_mmap.c 19 Apr 2022 22:53:34 -0000 1.179
+++ uvm_mmap.c 4 Jun 2022 20:54:03 -0000 1.180
@@ -277,7 +277,8 @@
vsize_t size, pageoff, newsize;
vm_prot_t prot, maxprot, extraprot;
int flags, fd, advice;
- vaddr_t defaddr;
+ vaddr_t defaddr = 0; /* XXXGCC */
+ bool addrhint = false;
struct file *fp = NULL;
struct uvm_object *uobj;
int error;
@@ -349,6 +350,12 @@
addr = MAX(addr, defaddr);
else
addr = MIN(addr, defaddr);
+
+ /*
+ * If addr is nonzero and not the default, then the
+ * address is a hint.
+ */
+ addrhint = (addr != 0 && addr != defaddr);
}
/*
@@ -399,11 +406,30 @@
pax_aslr_mmap(l, &addr, orig_addr, flags);
/*
- * now let kernel internal function uvm_mmap do the work.
+ * Now let kernel internal function uvm_mmap do the work.
+ *
+ * If the user provided a hint, take a reference to uobj in
+ * case the first attempt to satisfy the hint fails, so we can
+ * try again with the default address.
*/
-
+ if (addrhint) {
+ if (uobj)
+ (*uobj->pgops->pgo_reference)(uobj);
+ }
error = uvm_mmap(&p->p_vmspace->vm_map, &addr, size, prot, maxprot,
flags, advice, uobj, pos, p->p_rlimit[RLIMIT_MEMLOCK].rlim_cur);
+ if (addrhint) {
+ if (error) {
+ addr = defaddr;
+ pax_aslr_mmap(l, &addr, orig_addr, flags);
+ error = uvm_mmap(&p->p_vmspace->vm_map, &addr, size,
+ prot, maxprot, flags, advice, uobj, pos,
+ p->p_rlimit[RLIMIT_MEMLOCK].rlim_cur);
+ } else if (uobj) {
+ /* Release the exta reference we took. */
+ (*uobj->pgops->pgo_detach)(uobj);
+ }
+ }
/* remember to add offset */
*retval = (register_t)(addr + pageoff);
@@ -818,9 +844,12 @@
* - used by sys_mmap and various framebuffers
* - uobj is a struct uvm_object pointer or NULL for MAP_ANON
* - caller must page-align the file offset
+ *
+ * XXX This appears to leak the uobj in various error branches? Need
+ * to clean up the contract around uobj reference.
*/
-int
+static int
uvm_mmap(struct vm_map *map, vaddr_t *addr, vsize_t size, vm_prot_t prot,
vm_prot_t maxprot, int flags, int advice, struct uvm_object *uobj,
voff_t foff, vsize_t locklimit)
--XwFBqNPNA03tvv54--
Home |
Main Index |
Thread Index |
Old Index