Subject: change cgd(4) to use bufpool ?
To: None <elric@netbsd.org>
From: Darrin B. Jewell <dbj@netbsd.org>
List: tech-kern
Date: 07/18/2004 15:31:35
--=-=-=


Is there a good reason that the cgd(4) device uses its
own buffer structure and pool?  The following patch allows
it to use the regular bufpool by taking advantage of the
bp->b_private field to store the old buffer.

The patch also includes the following tweaks:
 . don't bother to raise splbio inside cgdiodone
 . use V_INCR_NUMOUTPUT

Darrin


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=cgd.c.buf.diff
Content-Description: patch to use bufpool in cgd

Index: cgd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/cgd.c,v
retrieving revision 1.16
diff -u -u -r1.16 cgd.c
--- cgd.c	27 Mar 2004 23:23:06 -0000	1.16
+++ cgd.c	18 Jul 2004 19:22:16 -0000
@@ -139,19 +139,6 @@
 #define DIAGCONDPANIC(x,y)
 #endif
 
-/* Component Buffer Pool structures and macros */
-
-struct cgdbuf {
-	struct buf		 cb_buf;	/* new I/O buf */
-	struct buf		*cb_obp;	/* ptr. to original I/O buf */
-	struct cgd_softc	*cb_sc;		/* pointer to cgd softc */
-};
-
-struct pool cgd_cbufpool;
-
-#define	CGD_GETBUF()		pool_get(&cgd_cbufpool, PR_NOWAIT)
-#define	CGD_PUTBUF(cbp)		pool_put(&cgd_cbufpool, cbp)
-
 /* Global variables */
 
 struct	cgd_softc *cgd_softc;
@@ -207,10 +194,6 @@
 	numcgd = num;
 	for (i=0; i<num; i++)
 		cgdsoftc_init(&cgd_softc[i], i);
-
-	/* Init component buffer pool. XXX, can we put this in dksubr.c? */
-	pool_init(&cgd_cbufpool, sizeof(struct cgdbuf), 0, 0, 0,
-	    "cgdpl", NULL);
 }
 
 int
@@ -301,11 +284,12 @@
 cgdstart(struct dk_softc *dksc, struct buf *bp)
 {
 	struct	cgd_softc *cs = dksc->sc_osc;
-	struct	cgdbuf *cbp;
+	struct	buf *nbp;
 	struct	partition *pp;
 	caddr_t	addr;
 	caddr_t	newaddr;
 	daddr_t	bn;
+	int s;
 
 	DPRINTF_FOLLOW(("cgdstart(%p, %p)\n", dksc, bp));
 	disk_busy(&dksc->sc_dkdev); /* XXX: put in dksubr.c */
@@ -326,9 +310,11 @@
 	 * We attempt to allocate all of our resources up front, so that
 	 * we can fail quickly if they are unavailable.
 	 */
-
-	cbp = CGD_GETBUF();
-	if (cbp == NULL) {
+	
+	s = splbio();
+	nbp = pool_get(&bufpool, PR_NOWAIT);
+	splx(s);
+	if (nbp == NULL) {
 		disk_unbusy(&dksc->sc_dkdev, 0, (bp->b_flags & B_READ));
 		return -1;
 	}
@@ -342,7 +328,9 @@
 	if ((bp->b_flags & B_READ) == 0) {
 		newaddr = cgd_getdata(dksc, bp->b_bcount);
 		if (!newaddr) {
-			CGD_PUTBUF(cbp);
+			s = splbio();
+			pool_put(&bufpool, nbp);
+			splx(s);
 			disk_unbusy(&dksc->sc_dkdev, 0, (bp->b_flags & B_READ));
 			return -1;
 		}
@@ -350,44 +338,39 @@
 		    DEV_BSIZE, CGD_CIPHER_ENCRYPT);
 	}
 
-	BUF_INIT(&cbp->cb_buf);
-	cbp->cb_buf.b_data = newaddr;
-	cbp->cb_buf.b_flags = bp->b_flags | B_CALL;
-	cbp->cb_buf.b_iodone = cgdiodone;
-	cbp->cb_buf.b_proc = bp->b_proc;
-	cbp->cb_buf.b_blkno = bn;
-	cbp->cb_buf.b_vp = cs->sc_tvn;
-	cbp->cb_buf.b_bcount = bp->b_bcount;
-
-	/* context for cgdiodone */
-	cbp->cb_obp = bp;
-	cbp->cb_sc = cs;
-
-	BIO_COPYPRIO(&cbp->cb_buf, bp);
-
-	if ((cbp->cb_buf.b_flags & B_READ) == 0)
-		cbp->cb_buf.b_vp->v_numoutput++;
-	VOP_STRATEGY(cs->sc_tvn, &cbp->cb_buf);
+	BUF_INIT(nbp);
+	nbp->b_data = newaddr;
+	nbp->b_flags = bp->b_flags | B_CALL;
+	nbp->b_iodone = cgdiodone;
+	nbp->b_proc = bp->b_proc;
+	nbp->b_blkno = bn;
+	nbp->b_vp = cs->sc_tvn;
+	nbp->b_bcount = bp->b_bcount;
+	nbp->b_private = bp;
+
+	BIO_COPYPRIO(nbp, bp);
+
+	if ((nbp->b_flags & B_READ) == 0) {
+		V_INCR_NUMOUTPUT(nbp->b_vp);
+	}
+	VOP_STRATEGY(cs->sc_tvn, nbp);
 	return 0;
 }
 
+/* expected to be called at splbio() */
 void
-cgdiodone(struct buf *vbp)
+cgdiodone(struct buf *nbp)
 {
-	struct	cgdbuf *cbp = (struct cgdbuf *)vbp;
-	struct	buf *obp = cbp->cb_obp;
-	struct	buf *nbp = &cbp->cb_buf;
-	struct	cgd_softc *cs = cbp->cb_sc;
+	struct	buf *obp = nbp->b_private;
+	struct	cgd_softc *cs = getcgd_softc(obp->b_dev);
 	struct	dk_softc *dksc = &cs->sc_dksc;
-	int	s;
 
-	DPRINTF_FOLLOW(("cgdiodone(%p)\n", vbp));
+	DPRINTF_FOLLOW(("cgdiodone(%p)\n", nbp));
 	DPRINTF(CGDB_IO, ("cgdiodone: bp %p bcount %ld resid %ld\n",
 	    obp, obp->b_bcount, obp->b_resid));
-	DPRINTF(CGDB_IO, (" dev 0x%x, cbp %p bn %" PRId64 " addr %p bcnt %ld\n",
-	    cbp->cb_buf.b_dev, cbp, cbp->cb_buf.b_blkno, cbp->cb_buf.b_data,
-	    cbp->cb_buf.b_bcount));
-	s = splbio();
+	DPRINTF(CGDB_IO, (" dev 0x%x, nbp %p bn %" PRId64 " addr %p bcnt %ld\n",
+	    nbp->b_dev, nbp, nbp->b_blkno, nbp->b_data,
+	    nbp->b_bcount));
 	if (nbp->b_flags & B_ERROR) {
 		obp->b_flags |= B_ERROR;
 		obp->b_error  = nbp->b_error ? nbp->b_error : EIO;
@@ -409,7 +392,7 @@
 	if (nbp->b_data != obp->b_data)
 		cgd_putdata(dksc, nbp->b_data);
 
-	CGD_PUTBUF(cbp);
+	pool_put(&bufpool, nbp);
 
 	/* Request is complete for whatever reason */
 	obp->b_resid = 0;
@@ -419,7 +402,6 @@
 	    (obp->b_flags & B_READ));
 	biodone(obp);
 	dk_iodone(di, dksc);
-	splx(s);
 }
 
 /* XXX: we should probably put these into dksubr.c, mostly */

--=-=-=--