Module Name:    src
Committed By:   mlelstv
Date:           Thu Aug 27 05:51:50 UTC 2015

Modified Files:
        src/sys/arch/xen/xen: xbd_xenbus.c
        src/sys/dev: cgd.c dksubr.c dkvar.h ld.c

Log Message:
Make dksubr use a spin-mutex again, since some drivers still call dk_done
from hardware interrupt. Instead, release mutex while calling start routine.

The buffer peek/use/get sequence which can no longer be atomic. So consume
the buffer directly and on error privately save and retry the buffer later.
The dk_drain function is used to flush such a deferred buffer together with
the buffer queue.
Adjust drivers to use dk_drain.

Fix an error path where dk_done was called while the lock was already held.


To generate a diff of this commit:
cvs rdiff -u -r1.72 -r1.73 src/sys/arch/xen/xen/xbd_xenbus.c
cvs rdiff -u -r1.103 -r1.104 src/sys/dev/cgd.c
cvs rdiff -u -r1.73 -r1.74 src/sys/dev/dksubr.c
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/dkvar.h
cvs rdiff -u -r1.91 -r1.92 src/sys/dev/ld.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/arch/xen/xen/xbd_xenbus.c
diff -u src/sys/arch/xen/xen/xbd_xenbus.c:1.72 src/sys/arch/xen/xen/xbd_xenbus.c:1.73
--- src/sys/arch/xen/xen/xbd_xenbus.c:1.72	Sun Aug 16 18:00:03 2015
+++ src/sys/arch/xen/xen/xbd_xenbus.c	Thu Aug 27 05:51:50 2015
@@ -1,4 +1,4 @@
-/*      $NetBSD: xbd_xenbus.c,v 1.72 2015/08/16 18:00:03 mlelstv Exp $      */
+/*      $NetBSD: xbd_xenbus.c,v 1.73 2015/08/27 05:51:50 mlelstv Exp $      */
 
 /*
  * Copyright (c) 2006 Manuel Bouyer.
@@ -50,7 +50,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xbd_xenbus.c,v 1.72 2015/08/16 18:00:03 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xbd_xenbus.c,v 1.73 2015/08/27 05:51:50 mlelstv Exp $");
 
 #include "opt_xen.h"
 
@@ -363,11 +363,9 @@ xbd_xenbus_detach(device_t dev, int flag
 		/* Delete all of our wedges. */
 		dkwedge_delall(&sc->sc_dksc.sc_dkdev);
 
-		s = splbio();
 		/* Kill off any queued buffers. */
-		bufq_drain(sc->sc_dksc.sc_bufq);
+		dk_drain(&sc->sc_dksc);
 		bufq_free(sc->sc_dksc.sc_bufq);
-		splx(s);
 
 		/* detach disk */
 		disk_detach(&sc->sc_dksc.sc_dkdev);
@@ -701,12 +699,11 @@ again:
 next:
 		if (bp->b_data != xbdreq->req_data)
 			xbd_unmap_align(xbdreq);
-		disk_unbusy(&sc->sc_dksc.sc_dkdev,
-		    (bp->b_bcount - bp->b_resid),
-		    (bp->b_flags & B_READ));
+
 		rnd_add_uint32(&sc->sc_rnd_source,
 		    bp->b_blkno);
-		biodone(bp);
+		dk_done(&sc->sc_dksc, bp);
+
 		SLIST_INSERT_HEAD(&sc->sc_xbdreq_head, xbdreq, req_next);
 	}
 done:

Index: src/sys/dev/cgd.c
diff -u src/sys/dev/cgd.c:1.103 src/sys/dev/cgd.c:1.104
--- src/sys/dev/cgd.c:1.103	Fri Aug 21 09:33:53 2015
+++ src/sys/dev/cgd.c	Thu Aug 27 05:51:50 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: cgd.c,v 1.103 2015/08/21 09:33:53 christos Exp $ */
+/* $NetBSD: cgd.c,v 1.104 2015/08/27 05:51:50 mlelstv Exp $ */
 
 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cgd.c,v 1.103 2015/08/21 09:33:53 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cgd.c,v 1.104 2015/08/27 05:51:50 mlelstv Exp $");
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -565,7 +565,7 @@ cgdioctl(dev_t dev, u_long cmd, void *da
 		return VOP_IOCTL(cs->sc_tvn, cmd, data, flag, l->l_cred);
 	case DIOCGSTRATEGY:
 	case DIOCSSTRATEGY:
-		if ((dksc->sc_flags & DKF_INITED) == 0)
+		if (!DK_ATTACHED(dksc))
 			return ENOENT;
 		/*FALLTHROUGH*/
 	default:
@@ -722,7 +722,6 @@ bail:
 static int
 cgd_ioctl_clr(struct cgd_softc *cs, struct lwp *l)
 {
-	int	s;
 	struct	dk_softc *dksc = &cs->sc_dksc;
 
 	if (!DK_ATTACHED(dksc))
@@ -732,9 +731,7 @@ cgd_ioctl_clr(struct cgd_softc *cs, stru
 	dkwedge_delall(&dksc->sc_dkdev);
 
 	/* Kill off any queued buffers. */
-	s = splbio();
-	bufq_drain(dksc->sc_bufq);
-	splx(s);
+	dk_drain(dksc);
 	bufq_free(dksc->sc_bufq);
 
 	(void)vn_close(cs->sc_tvn, FREAD|FWRITE, l->l_cred);

Index: src/sys/dev/dksubr.c
diff -u src/sys/dev/dksubr.c:1.73 src/sys/dev/dksubr.c:1.74
--- src/sys/dev/dksubr.c:1.73	Sun Aug 23 07:47:52 2015
+++ src/sys/dev/dksubr.c	Thu Aug 27 05:51:50 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: dksubr.c,v 1.73 2015/08/23 07:47:52 mlelstv Exp $ */
+/* $NetBSD: dksubr.c,v 1.74 2015/08/27 05:51:50 mlelstv Exp $ */
 
 /*-
  * Copyright (c) 1996, 1997, 1998, 1999, 2002, 2008 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dksubr.c,v 1.73 2015/08/23 07:47:52 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dksubr.c,v 1.74 2015/08/27 05:51:50 mlelstv Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -74,6 +74,7 @@ static int dk_subr_modcmd(modcmd_t, void
 
 static void	dk_makedisklabel(struct dk_softc *);
 static int	dk_translate(struct dk_softc *, struct buf *);
+static void	dk_done1(struct dk_softc *, struct buf *, bool);
 
 void
 dk_init(struct dk_softc *dksc, device_t dev, int dtype)
@@ -90,7 +91,7 @@ dk_init(struct dk_softc *dksc, device_t 
 void
 dk_attach(struct dk_softc *dksc)
 {
-	mutex_init(&dksc->sc_iolock, MUTEX_DEFAULT, IPL_NONE);
+	mutex_init(&dksc->sc_iolock, MUTEX_DEFAULT, IPL_VM);
 	dksc->sc_flags |= DKF_INITED;
 #ifdef DIAGNOSTIC
 	dksc->sc_flags |= DKF_WARNLABEL | DKF_LABELSANITY;
@@ -294,41 +295,55 @@ dk_start(struct dk_softc *dksc, struct b
 {
 	const struct dkdriver *dkd = dksc->sc_dkdev.dk_driver;
 	int error;
-	struct buf *qbp __diagused;
 
 	mutex_enter(&dksc->sc_iolock);
 
 	if (bp != NULL)
 		bufq_put(dksc->sc_bufq, bp);
 
-	while ((bp = bufq_peek(dksc->sc_bufq)) != NULL) {
+	/*
+	 * Peeking at the buffer queue and committing the operation
+	 * only after success isn't atomic.
+	 *
+	 * So when a diskstart fails, the buffer is saved
+	 * and tried again before the next buffer is fetched.
+	 * dk_drain() handles flushing of a saved buffer.
+	 *
+	 * This keeps order of I/O operations, unlike bufq_put.
+	 */
+
+	bp = dksc->sc_deferred;
+	dksc->sc_deferred = NULL;
+
+	if (bp == NULL)
+		bp = bufq_get(dksc->sc_bufq);
+
+	while (bp != NULL) {
 
 		disk_busy(&dksc->sc_dkdev);
+		mutex_exit(&dksc->sc_iolock);
 		error = dkd->d_diskstart(dksc->sc_dev, bp);
+		mutex_enter(&dksc->sc_iolock);
 		if (error == EAGAIN) {
+			dksc->sc_deferred = bp;
 			disk_unbusy(&dksc->sc_dkdev, 0, (bp->b_flags & B_READ));
 			break;
 		}
 
-#ifdef DIAGNOSTIC
-		qbp = bufq_get(dksc->sc_bufq);
-		KASSERT(bp == qbp);
-#else
-		(void) bufq_get(dksc->sc_bufq);
-#endif
-
 		if (error != 0) {
 			bp->b_error = error;
 			bp->b_resid = bp->b_bcount;
-			dk_done(dksc, bp);
+			dk_done1(dksc, bp, false);
 		}
+
+		bp = bufq_get(dksc->sc_bufq);
 	}
 
 	mutex_exit(&dksc->sc_iolock);
 }
 
-void
-dk_done(struct dk_softc *dksc, struct buf *bp)
+static void
+dk_done1(struct dk_softc *dksc, struct buf *bp, bool lock)
 {
 	struct disk *dk = &dksc->sc_dkdev;
 
@@ -340,9 +355,11 @@ dk_done(struct dk_softc *dksc, struct bu
 		printf("\n");
 	}
 
-	mutex_enter(&dksc->sc_iolock);
+	if (lock)
+		mutex_enter(&dksc->sc_iolock);
 	disk_unbusy(dk, bp->b_bcount - bp->b_resid, (bp->b_flags & B_READ));
-	mutex_exit(&dksc->sc_iolock);
+	if (lock)
+		mutex_exit(&dksc->sc_iolock);
 
 #ifdef notyet
 	rnd_add_uint(&dksc->sc_rnd_source, bp->b_rawblkno);
@@ -351,6 +368,28 @@ dk_done(struct dk_softc *dksc, struct bu
 	biodone(bp);
 }
 
+void
+dk_done(struct dk_softc *dksc, struct buf *bp)
+{
+	dk_done1(dksc, bp, true);
+}
+
+void
+dk_drain(struct dk_softc *dksc)
+{
+	struct buf *bp;
+
+	mutex_enter(&dksc->sc_iolock);
+	bp = dksc->sc_deferred;
+	if (bp != NULL) {
+		bp->b_error = EIO;
+		bp->b_resid = bp->b_bcount;
+		biodone(bp); 
+	}
+	bufq_drain(dksc->sc_bufq);
+	mutex_exit(&dksc->sc_iolock);
+}
+
 int
 dk_discard(struct dk_softc *dksc, dev_t dev, off_t pos, off_t len)
 {

Index: src/sys/dev/dkvar.h
diff -u src/sys/dev/dkvar.h:1.21 src/sys/dev/dkvar.h:1.22
--- src/sys/dev/dkvar.h:1.21	Sun Aug 16 18:00:03 2015
+++ src/sys/dev/dkvar.h	Thu Aug 27 05:51:50 2015
@@ -1,4 +1,4 @@
-/* $NetBSD: dkvar.h,v 1.21 2015/08/16 18:00:03 mlelstv Exp $ */
+/* $NetBSD: dkvar.h,v 1.22 2015/08/27 05:51:50 mlelstv Exp $ */
 
 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -47,6 +47,7 @@ struct dk_softc {
 	kmutex_t		 sc_iolock;	/* protects buffer queue */
 	struct bufq_state	*sc_bufq;	/* buffer queue */
 	int			 sc_dtype;	/* disk type */
+	struct buf		*sc_deferred;	/* retry after start failed */
 };
 
 /* sc_flags:
@@ -62,6 +63,7 @@ struct dk_softc {
 #define DKF_TAKEDUMP	0x00200000 /* allow dumping */
 #define DKF_KLABEL      0x00400000 /* keep label on close */
 #define DKF_VLABEL      0x00800000 /* label is valid */
+#define DKF_SLEEP       0x80000000 /* dk_start/dk_done may sleep */
 
 /* Mask of flags that dksubr.c understands, other flags are fair game */
 #define DK_FLAGMASK	0xffff0000
@@ -89,6 +91,7 @@ void	dk_strategy(struct dk_softc *, stru
 int	dk_discard(struct dk_softc *, dev_t, off_t, off_t);
 void	dk_start(struct dk_softc *, struct buf *);
 void	dk_done(struct dk_softc *, struct buf *);
+void	dk_drain(struct dk_softc *);
 int	dk_size(struct dk_softc *, dev_t);
 int	dk_ioctl(struct dk_softc *, dev_t,
 		 u_long, void *, int, struct lwp *);

Index: src/sys/dev/ld.c
diff -u src/sys/dev/ld.c:1.91 src/sys/dev/ld.c:1.92
--- src/sys/dev/ld.c:1.91	Tue Aug 18 04:20:25 2015
+++ src/sys/dev/ld.c	Thu Aug 27 05:51:50 2015
@@ -1,4 +1,4 @@
-/*	$NetBSD: ld.c,v 1.91 2015/08/18 04:20:25 mlelstv Exp $	*/
+/*	$NetBSD: ld.c,v 1.92 2015/08/27 05:51:50 mlelstv Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2000 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ld.c,v 1.91 2015/08/18 04:20:25 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ld.c,v 1.92 2015/08/27 05:51:50 mlelstv Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -197,8 +197,6 @@ ldbegindetach(struct ld_softc *sc, int f
 	mutex_enter(&sc->sc_mutex);
 	sc->sc_maxqueuecnt = 0;
 
-	dk_detach(dksc);
-
 	while (sc->sc_queuecnt > 0) {
 		sc->sc_flags |= LDF_DRAIN;
 		cv_wait(&sc->sc_drain, &sc->sc_mutex);
@@ -224,11 +222,10 @@ ldenddetach(struct ld_softc *sc)
 		if (cv_timedwait(&sc->sc_drain, &sc->sc_mutex, 30 * hz))
 			printf("%s: not drained\n", dksc->sc_xname);
 	}
-
-	/* Kill off any queued buffers. */
-	bufq_drain(dksc->sc_bufq);
 	mutex_exit(&sc->sc_mutex);
 
+	/* Kill off any queued buffers. */
+	dk_drain(dksc);
 	bufq_free(dksc->sc_bufq);
 
 	/* Locate the major numbers. */
@@ -249,6 +246,8 @@ ldenddetach(struct ld_softc *sc)
 	disk_detach(&dksc->sc_dkdev);
 	disk_destroy(&dksc->sc_dkdev);
 
+	dk_detach(dksc);
+
 	/* Unhook the entropy source. */
 	rnd_detach_source(&sc->sc_rnd_source);
 

Reply via email to