minidump: a hack to prevent vm_page_dump bitmap change during dumping

[Andriy Gapon]

I see that rather frequently vm_page_dump bitmap gets changed during minidumpsys
execution.  Sometimes this results in number of pages to dump growing larger than
space we already reserved on disk, and thus in aborting dump near the end with
"Attempt to write outside dump device boundaries" error.  Sometimes it results in
dumped bitmap and dumped pages being out of sync (perhaps silently).

Below is a simple patch for amd64 minidump, a hack rather, that seems to work
around the issue by ignoring dump_add_page/dump_drop_page calls after minidumpsys
is called.
Not sure if mb() call there is needed or has any effect.

Proper fix, of course, would be to stop other CPUs and interrupts and also
avoiding memory allocations in dump path (or something to that effect).

Please review.
Thank!

diff --git a/sys/amd64/amd64/minidump_machdep.c b/sys/amd64/amd64/minidump_machdep.c
index a9af809..56849a0 100644
--- a/sys/amd64/amd64/minidump_machdep.c
+++ b/sys/amd64/amd64/minidump_machdep.c
@@ -53,6 +53,9 @@ CTASSERT(sizeof(struct kerneldumpheader) == 512);
 #define	MD_ALIGN(x)	(((off_t)(x) + PAGE_MASK) & ~PAGE_MASK)
 #define	DEV_ALIGN(x)	(((off_t)(x) + (DEV_BSIZE-1)) & ~(DEV_BSIZE-1))

+void	dump_add_page_priv(vm_paddr_t pa);
+void	dump_drop_page_priv(vm_paddr_t pa);
+
 extern uint64_t KPDPphys;

 uint64_t *vm_page_dump;
@@ -65,6 +68,7 @@ static off_t dumplo;
 static size_t fragsz;
 static void *dump_va;
 static size_t counter, progress;
+static int bitmap_frozen = 0;

 CTASSERT(sizeof(*vm_page_dump) == 8);

@@ -181,6 +185,10 @@ minidumpsys(struct dumperinfo *di)
 	int i, j, k, bit;
 	struct minidumphdr mdhdr;

+	/*XXX*/
+	bitmap_frozen = 1;
+	mb();
+
 	counter = 0;
 	/* Walk page table pages, set bits in vm_page_dump */
 	ptesize = 0;
@@ -202,7 +210,7 @@ minidumpsys(struct dumperinfo *di)
 			pa = pd[j] & PG_PS_FRAME;
 			for (k = 0; k < NPTEPG; k++) {
 				if (is_dumpable(pa))
-					dump_add_page(pa);
+					dump_add_page_priv(pa);
 				pa += PAGE_SIZE;
 			}
 			continue;
@@ -214,7 +222,7 @@ minidumpsys(struct dumperinfo *di)
 				if ((pt[k] & PG_V) == PG_V) {
 					pa = pt[k] & PG_FRAME;
 					if (is_dumpable(pa))
-						dump_add_page(pa);
+						dump_add_page_priv(pa);
 				}
 			}
 		} else {
@@ -235,7 +243,7 @@ minidumpsys(struct dumperinfo *di)
 			if (is_dumpable(pa)) {
 				dumpsize += PAGE_SIZE;
 			} else {
-				dump_drop_page(pa);
+				dump_drop_page_priv(pa);
 			}
 			bits &= ~(1ul << bit);
 		}
@@ -387,6 +395,9 @@ dump_add_page(vm_paddr_t pa)
 {
 	int idx, bit;

+	if (bitmap_frozen)
+		return;
+
 	pa >>= PAGE_SHIFT;
 	idx = pa >> 6;		/* 2^6 = 64 */
 	bit = pa & 63;
@@ -398,8 +409,33 @@ dump_drop_page(vm_paddr_t pa)
 {
 	int idx, bit;

+	if (bitmap_frozen)
+		return;
+
 	pa >>= PAGE_SHIFT;
 	idx = pa >> 6;		/* 2^6 = 64 */
 	bit = pa & 63;
 	atomic_clear_long(&vm_page_dump[idx], 1ul << bit);
 }
+
+void
+dump_add_page_priv(vm_paddr_t pa)
+{
+	int idx, bit;
+
+	pa >>= PAGE_SHIFT;
+	idx = pa >> 6;		/* 2^6 = 64 */
+	bit = pa & 63;
+	vm_page_dump[idx] |= 1ul << bit;
+}
+
+void
+dump_drop_page_priv(vm_paddr_t pa)
+{
+	int idx, bit;
+
+	pa >>= PAGE_SHIFT;
+	idx = pa >> 6;		/* 2^6 = 64 */
+	bit = pa & 63;
+	vm_page_dump[idx] &= ~(1ul << bit);
+}

-- 
Andriy Gapon