Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev cgd(4): Fix criterion for detach when wedgies are held.



details:   https://anonhg.NetBSD.org/src/rev/2dd9be31ba4e
branches:  trunk
changeset: 1029217:2dd9be31ba4e
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Dec 27 22:57:19 2021 +0000

description:
cgd(4): Fix criterion for detach when wedgies are held.

The somewhat confusingly named DK_BUSY(dksc, pmask) answers the
following question:

        Suppose I hold either the character or the block device (but
        not both) of all of the partitions in pmask.  Is anyone else
        using the disk, rendering it unsafe to detach?

This is useful for ioctls like CGDIOCCLR and VNDIOCCLR, which must be
issued on open file descriptors for the disk, so the question cannot
simply be answered by testing whether dk_openmask != 0.

Instead, DK_BUSY breaks the question into the following criteria:

        1. Are there any _other_ partitions than those in pmask open
           at all?  If so, it must be someone else, since I only hold
           partitions in pmask -- hence the disk is busy.

        2. Are any of the partitions in pmask open _both_ as a block
           device _and_ as a character device?  If so, it must be
           someone else, since I only hold _either_ the character
           _or_ the block device open but not both -- hence the disk
           is busy.

When config_detach_all runs at shutdown time, it tries to detach
cgd(4), which has DVF_DETACH_SHUTDOWN; this is important so we submit
queued writes to the underlying disk and wait for them to complete
with dk_drain.

If cgd(4) has any dk wedges with file systems mounted still
configured on it, it isn't ready to detach yet.  But asking
DK_BUSY(dksc, 1 << RAW_PART) returns false, because the dk wedges
only hold RAW_PART open as a block device -- so if nobody has
RAW_PART open as a character device, or any other partitions open,
cgd_detach blithely goes on its way to forcibly detach the wedges.

Instead, ask DK_BUSY(dksc, 0), because the caller -- cgd_detach
issued by config_detach_all -- does not, in fact, hold any partitions
open, so it doesn't need to work around them like ioctl(CGDIOCCLR)
does.  Fixes hang in zfs on dk on cgd during shutdown (and probably
also zfs on cgd without any intervening dk but I haven't tested).

(This change might have the side effect that `drvctl -d cgdN' doesn't
work, but I don't care.)

XXX pullup-9
XXX pullup-8 (...-7, -6, -5...)

diffstat:

 sys/dev/cgd.c |  7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diffs (31 lines):

diff -r f089139378a1 -r 2dd9be31ba4e sys/dev/cgd.c
--- a/sys/dev/cgd.c     Mon Dec 27 22:22:48 2021 +0000
+++ b/sys/dev/cgd.c     Mon Dec 27 22:57:19 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: cgd.c,v 1.141 2021/12/13 21:15:26 riastradh Exp $ */
+/* $NetBSD: cgd.c,v 1.142 2021/12/27 22:57:19 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cgd.c,v 1.141 2021/12/13 21:15:26 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cgd.c,v 1.142 2021/12/27 22:57:19 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -529,11 +529,10 @@
 cgd_detach(device_t self, int flags)
 {
        int ret;
-       const int pmask = 1 << RAW_PART;
        struct cgd_softc *sc = device_private(self);
        struct dk_softc *dksc = &sc->sc_dksc;
 
-       if (DK_BUSY(dksc, pmask))
+       if (DK_BUSY(dksc, 0))
                return EBUSY;
 
        if (DK_ATTACHED(dksc) &&



Home | Main Index | Thread Index | Old Index