[src/trunk]: src/sys/kern fix race between wapbl_discard() and wapbl_biodone(...

[jdolecek <jdolecek%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/c936da210631
branches:  trunk
changeset: 1009085:c936da210631
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Sun Apr 12 08:51:41 2020 +0000

description:
fix race between wapbl_discard() and wapbl_biodone() on forced
unmount on shutdown with slow I/O device

wapbl_discard() needs to hold both wl_mtx and bufcache_lock while
manipulating wl_entries - the rw lock is not enough, because
wapbl_biodone() only takes wl_mtx while removing the finished entry
from list

wapbl_biodone() must take bufcache_lock before reading we->we_wapbl,
so it's blocked until wapbl_discard() finishes, and takes !wl path
appropriately

this is supposed to fix panic on shutdown:
[ 67549.6304123] forcefully unmounting / (/dev/wd0a)...
...
[ 67549.7272030] panic: mutex_vector_enter,510: uninitialized lock (lock=0xffffa722a4f4f5b0, from=ffffffff80a884fa)
...
[ 67549.7272030] wapbl_biodone() at netbsd:wapbl_biodone+0x4d
[ 67549.7272030] biointr() at netbsd:biointr+0x7d
[ 67549.7272030] softint_dispatch() at netbsd:softint_dispatch+0x12c
[ 67549.7272030] Xsoftintr() at netbsd:Xsoftintr+0x4f

diffstat:

 sys/kern/vfs_wapbl.c |  19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diffs (68 lines):

diff -r 0d5498395752 -r c936da210631 sys/kern/vfs_wapbl.c
--- a/sys/kern/vfs_wapbl.c      Sun Apr 12 07:49:58 2020 +0000
+++ b/sys/kern/vfs_wapbl.c      Sun Apr 12 08:51:41 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_wapbl.c,v 1.106 2020/03/16 21:20:10 pgoyette Exp $ */
+/*     $NetBSD: vfs_wapbl.c,v 1.107 2020/04/12 08:51:41 jdolecek Exp $ */
 
 /*-
  * Copyright (c) 2003, 2008, 2009 The NetBSD Foundation, Inc.
@@ -36,7 +36,7 @@
 #define WAPBL_INTERNAL
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_wapbl.c,v 1.106 2020/03/16 21:20:10 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_wapbl.c,v 1.107 2020/04/12 08:51:41 jdolecek Exp $");
 
 #include <sys/param.h>
 #include <sys/bitops.h>
@@ -226,7 +226,7 @@
        u_long wl_inohashmask;
        int wl_inohashcnt;
 
-       SIMPLEQ_HEAD(, wapbl_entry) wl_entries; /* On disk transaction
+       SIMPLEQ_HEAD(, wapbl_entry) wl_entries; /* m: On disk transaction
                                                   accounting */
 
        /* buffers for wapbl_buffered_write() */
@@ -786,12 +786,10 @@
                        mutex_enter(&wl->wl_mtx);
                }
        }
-       mutex_exit(&wl->wl_mtx);
-       mutex_exit(&bufcache_lock);
 
        /*
         * Remove references to this wl from wl_entries, free any which
-        * no longer have buffers, others will be freed in wapbl_biodone
+        * no longer have buffers, others will be freed in wapbl_biodone()
         * when they no longer have any buffers.
         */
        while ((we = SIMPLEQ_FIRST(&wl->wl_entries)) != NULL) {
@@ -807,6 +805,9 @@
                }
        }
 
+       mutex_exit(&wl->wl_mtx);
+       mutex_exit(&bufcache_lock);
+
        /* Discard list of deallocs */
        while ((wd = TAILQ_FIRST(&wl->wl_dealloclist)) != NULL)
                wapbl_deallocation_free(wl, wd, true);
@@ -1604,11 +1605,15 @@
 wapbl_biodone(struct buf *bp)
 {
        struct wapbl_entry *we = bp->b_private;
-       struct wapbl *wl = we->we_wapbl;
+       struct wapbl *wl;
 #ifdef WAPBL_DEBUG_BUFBYTES
        const int bufsize = bp->b_bufsize;
 #endif
 
+       mutex_enter(&bufcache_lock);
+       wl = we->we_wapbl;
+       mutex_exit(&bufcache_lock);
+
        /*
         * Handle possible flushing of buffers after log has been
         * decomissioned.