Skip to content

Commit

Permalink
dk(4): Use disk_begindetach and rely on vdevgone to close instances.
Browse files Browse the repository at this point in the history
The first step is to decide whether we can detach (if forced, yes; if
not forced, only if not already open), and prevent new opens if so.
There's no need to start closing open instances at this point --
we're just making a decision to detach, and preventing new opens by
transitioning state that dkopen will respect[*].

The second step is to force all open instances to close.  This is
done by vdevgone.  By the time vdevgone returns, there can be no open
instances, so if there _were_ any, closing them via vdevgone will
have passed through dklastclose.

After that point, there can be no opens and no I/O operations, so
dk_openmask must already be zero and the bufq must be empty.

Thus, there's no need to have an explicit call to dklastclose (via
dkwedge_cleanup_parent) before or after making the decision to
detach.

[*] Currently access to this state is racy: nothing serializes
    dkwedge_detach's state transition with dkopen's test.  TBD in a
    separate commit shortly.
  • Loading branch information
riastradh authored and riastradh committed Apr 21, 2023
1 parent 5d980e1 commit 8041272
Showing 1 changed file with 15 additions and 28 deletions.
43 changes: 15 additions & 28 deletions sys/dev/dkwedge/dk.c
@@ -1,4 +1,4 @@
/* $NetBSD: dk.c,v 1.142 2023/04/21 18:30:32 riastradh Exp $ */
/* $NetBSD: dk.c,v 1.143 2023/04/21 18:30:52 riastradh Exp $ */

/*-
* Copyright (c) 2004, 2005, 2006, 2007 The NetBSD Foundation, Inc.
Expand Down Expand Up @@ -30,7 +30,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: dk.c,v 1.142 2023/04/21 18:30:32 riastradh Exp $");
__KERNEL_RCSID(0, "$NetBSD: dk.c,v 1.143 2023/04/21 18:30:52 riastradh Exp $");

#ifdef _KERNEL_OPT
#include "opt_dkwedge.h"
Expand Down Expand Up @@ -108,7 +108,6 @@ static void dkminphys(struct buf *);

static int dkfirstopen(struct dkwedge_softc *, int);
static void dklastclose(struct dkwedge_softc *);
static int dkwedge_cleanup_parent(struct dkwedge_softc *, int);
static int dkwedge_detach(device_t, int);
static void dkwedge_delall1(struct disk *, bool);
static int dkwedge_del1(struct dkwedge_info *, int);
Expand Down Expand Up @@ -668,28 +667,6 @@ dkwedge_del1(struct dkwedge_info *dkw, int flags)
return config_detach(sc->sc_dev, flags);
}

static int
dkwedge_cleanup_parent(struct dkwedge_softc *sc, int flags)
{
struct disk *dk = &sc->sc_dk;
int rc;

rc = 0;
mutex_enter(&dk->dk_openlock);
if (dk->dk_openmask == 0) {
/* nothing to do */
} else if ((flags & DETACH_FORCE) == 0) {
rc = EBUSY;
} else {
mutex_enter(&sc->sc_parent->dk_rawlock);
dklastclose(sc);
mutex_exit(&sc->sc_parent->dk_rawlock);
}
mutex_exit(&sc->sc_dk.dk_openlock);

return rc;
}

/*
* dkwedge_detach:
*
Expand All @@ -709,7 +686,8 @@ dkwedge_detach(device_t self, int flags)
}
if (unit == ndkwedges)
rc = ENXIO;
else if ((rc = dkwedge_cleanup_parent(sc, flags)) == 0) {
else if ((rc = disk_begindetach(&sc->sc_dk, /*lastclose*/NULL, self,
flags)) == 0) {
/* Mark the wedge as dying. */
sc->sc_state = DKW_STATE_DYING;
}
Expand Down Expand Up @@ -742,8 +720,17 @@ dkwedge_detach(device_t self, int flags)
vdevgone(bmaj, unit, unit, VBLK);
vdevgone(cmaj, unit, unit, VCHR);

/* Clean up the parent. */
dkwedge_cleanup_parent(sc, flags | DETACH_FORCE);
/*
* At this point, all block device opens have been closed,
* synchronously flushing any buffered writes; and all
* character device I/O operations have completed
* synchronously, and character device opens have been closed.
*
* So there can be no more opens or queued buffers by now.
*/
KASSERT(sc->sc_dk.dk_openmask == 0);
KASSERT(bufq_peek(sc->sc_bufq) == NULL);
bufq_drain(sc->sc_bufq);

/* Announce our departure. */
aprint_normal("%s at %s (%s) deleted\n", device_xname(sc->sc_dev),
Expand Down

0 comments on commit 8041272

Please sign in to comment.