Module Name:    src
Committed By:   riastradh
Date:           Sat Apr 22 13:11:50 UTC 2023

Modified Files:
        src/sys/dev/dkwedge: dk.c

Log Message:
dk(4): Convert tests to assertions in various devsw operations.

.d_cancel, .d_strategy, .d_read, .d_write, .d_ioctl, and .d_discard
are only ever used between successful .d_open return and entry to
.d_close.  .d_open doesn't return until sc is nonnull and sc_state is
RUNNING, and dkwedge_detach waits for the last .d_close before
setting sc_state to DEAD.  So there is no possibility for sc to be
null or for sc_state to be anything other than RUNNING or DYING.

There is a small functional change here but only in the event of a
race: in the short window between when dkwedge_detach is entered, and
when .d_close runs, any I/O operations (read, write, ioctl, &c.) may
be issued that would have failed with ENXIO before.

This shouldn't matter for anything: disk I/O operations are supposed
to complete reasonably promptly, and these operations _could_ have
begun milliseconds prior, before dkwedge_detach was entered, so it's
not a significant distinction.

Notes:

- .d_open must still contend with trying to open a nonexistent wedge,
  of course.

- .d_close must also contend with closing a nonexistent wedge, in
  case there were two calls to open in quick succession and the first
  failed while the second hadn't yet determined it would fail.

- .d_size and .d_dump are used from ddb without any open/close.


To generate a diff of this commit:
cvs rdiff -u -r1.149 -r1.150 src/sys/dev/dkwedge/dk.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/dkwedge/dk.c
diff -u src/sys/dev/dkwedge/dk.c:1.149 src/sys/dev/dkwedge/dk.c:1.150
--- src/sys/dev/dkwedge/dk.c:1.149	Sat Apr 22 12:33:46 2023
+++ src/sys/dev/dkwedge/dk.c	Sat Apr 22 13:11:50 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: dk.c,v 1.149 2023/04/22 12:33:46 riastradh Exp $	*/
+/*	$NetBSD: dk.c,v 1.150 2023/04/22 13:11:50 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2004, 2005, 2006, 2007 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dk.c,v 1.149 2023/04/22 12:33:46 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dk.c,v 1.150 2023/04/22 13:11:50 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_dkwedge.h"
@@ -1423,6 +1423,8 @@ dkcancel(dev_t dev, int flags, int fmt, 
 
 	KASSERT(sc != NULL);
 	KASSERT(sc->sc_dev != NULL);
+	KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+	KASSERT(sc->sc_state != DKW_STATE_DEAD);
 
 	/*
 	 * Disk I/O is expected to complete or fail within a reasonable
@@ -1448,16 +1450,10 @@ dkstrategy(struct buf *bp)
 	struct dkwedge_softc *sc = dkwedge_lookup(bp->b_dev);
 	uint64_t p_size, p_offset;
 
-	if (sc == NULL) {
-		bp->b_error = ENXIO;
-		goto done;
-	}
-
-	if (sc->sc_state != DKW_STATE_RUNNING ||
-	    sc->sc_parent->dk_rawvp == NULL) {
-		bp->b_error = ENXIO;
-		goto done;
-	}
+	KASSERT(sc != NULL);
+	KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+	KASSERT(sc->sc_state != DKW_STATE_DEAD);
+	KASSERT(sc->sc_parent->dk_rawvp != NULL);
 
 	/* If it's an empty transfer, wake up the top half now. */
 	if (bp->b_bcount == 0)
@@ -1647,12 +1643,11 @@ dkminphys(struct buf *bp)
 static int
 dkread(dev_t dev, struct uio *uio, int flags)
 {
-	struct dkwedge_softc *sc = dkwedge_lookup(dev);
+	struct dkwedge_softc *sc __diagused = dkwedge_lookup(dev);
 
-	if (sc == NULL)
-		return ENXIO;
-	if (sc->sc_state != DKW_STATE_RUNNING)
-		return ENXIO;
+	KASSERT(sc != NULL);
+	KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+	KASSERT(sc->sc_state != DKW_STATE_DEAD);
 
 	return physio(dkstrategy, NULL, dev, B_READ, dkminphys, uio);
 }
@@ -1665,12 +1660,11 @@ dkread(dev_t dev, struct uio *uio, int f
 static int
 dkwrite(dev_t dev, struct uio *uio, int flags)
 {
-	struct dkwedge_softc *sc = dkwedge_lookup(dev);
+	struct dkwedge_softc *sc __diagused = dkwedge_lookup(dev);
 
-	if (sc == NULL)
-		return ENXIO;
-	if (sc->sc_state != DKW_STATE_RUNNING)
-		return ENXIO;
+	KASSERT(sc != NULL);
+	KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+	KASSERT(sc->sc_state != DKW_STATE_DEAD);
 
 	return physio(dkstrategy, NULL, dev, B_WRITE, dkminphys, uio);
 }
@@ -1686,12 +1680,10 @@ dkioctl(dev_t dev, u_long cmd, void *dat
 	struct dkwedge_softc *sc = dkwedge_lookup(dev);
 	int error = 0;
 
-	if (sc == NULL)
-		return ENXIO;
-	if (sc->sc_state != DKW_STATE_RUNNING)
-		return ENXIO;
-	if (sc->sc_parent->dk_rawvp == NULL)
-		return ENXIO;
+	KASSERT(sc != NULL);
+	KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+	KASSERT(sc->sc_state != DKW_STATE_DEAD);
+	KASSERT(sc->sc_parent->dk_rawvp != NULL);
 
 	/*
 	 * We pass NODEV instead of our device to indicate we don't
@@ -1763,12 +1755,10 @@ dkdiscard(dev_t dev, off_t pos, off_t le
 	off_t offset, maxlen;
 	int error;
 
-	if (sc == NULL)
-		return ENXIO;
-	if (sc->sc_state != DKW_STATE_RUNNING)
-		return ENXIO;
-	if (sc->sc_parent->dk_rawvp == NULL)
-		return ENXIO;
+	KASSERT(sc != NULL);
+	KASSERT(sc->sc_state != DKW_STATE_LARVAL);
+	KASSERT(sc->sc_state != DKW_STATE_DEAD);
+	KASSERT(sc->sc_parent->dk_rawvp != NULL);
 
 	/* XXX check bounds on size/offset up front */
 	shift = (sc->sc_parent->dk_blkshift + DEV_BSHIFT);

Reply via email to