vinum panic on -devel

Simon 'corecode' Schubert corecode at fs.ei.tum.de
Fri Jul 7 14:25:28 PDT 2006


Matthew Dillon wrote:
    That should work.  Really the correct way is probably to use getpbuf()
    instead of geteblk(), but I'd like to make as few functional changes
    as possible to get vinum working again.
Of course I tried to find out how the "really correct way" should look like.  Now I don't get any panics and it *seems* that it might work, but some completely different, but seemingly related (happened several times, not neccessarily directly coupled) panic occured:

dev = #ad/0x20003, block = 11896, fs = /var
panic: ffs_blkfree: freeing free block
backtrace I can't because:

(kgdb) bt
#0  0x00000000 in ?? ()
* 39 Thread 0xce2ec500 (PID=-2: syncer)  0x00000000 in ?? ()

[something is wrong with kgdb here, I will look into this later]

/var was fsck -f'ed directly before.  This time this happened when I start'ed vinum (running following patch).

What I wonder is:  is it my patch which is fucking up stuff, is it vinum doing wrong offset calculations (and thus overwriting something in /var) or is it "just" a bug in ffs?

thanks
 simon
diff -r b16b8dd8a0d1 sys/dev/raid/vinum/vinum.c
--- a/sys/dev/raid/vinum/vinum.c	Fri Jul 07 15:03:06 2006 +0200
+++ b/sys/dev/raid/vinum/vinum.c	Fri Jul 07 15:04:51 2006 +0200
@@ -97,6 +97,8 @@ vinumattach(void *dummy)
    dqend = NULL;
    cdevsw_add(&vinum_cdevsw, 0, 0);			    /* add the cdevsw entry */
+
+    vinum_conf.physbufs = nswbuf / 2 + 1;		    /* maximum amount of physical bufs */
    /* allocate space: drives... */
    DRIVE = (struct drive *) Malloc(sizeof(struct drive) * INITIAL_DRIVES);
diff -r b16b8dd8a0d1 sys/dev/raid/vinum/vinumrevive.c
--- a/sys/dev/raid/vinum/vinumrevive.c	Fri Jul 07 15:03:06 2006 +0200
+++ b/sys/dev/raid/vinum/vinumrevive.c	Fri Jul 07 15:46:07 2006 +0200
@@ -140,11 +140,10 @@ revive_block(int sdno)
	if (bp == NULL)					    /* no buffer space */
	    return ENOMEM;				    /* chicken out */
    } else {						    /* data block */
-	crit_enter();
-	bp = geteblk(size);				    /* Get a buffer */
-	crit_exit();
+	bp = trypbuf(&vinum_conf.physbufs);		    /* Get a buffer */
	if (bp == NULL)
	    return ENOMEM;
+	bp->b_data = Malloc(size);
	/*
	 * Amount to transfer: block size, unless it
@@ -164,7 +163,6 @@ revive_block(int sdno)
	else						    /* it's an unattached plex */
	    dev = VINUM_PLEX(sd->plexno);		    /* create the device number */
-	bp->b_flags = B_PAGING;		    /* either way, read it */
	bp->b_cmd = BUF_CMD_READ;
	vinumstart(dev, &bp->b_bio1, 1);
	biowait(bp);
@@ -176,7 +174,7 @@ revive_block(int sdno)
	/* Now write to the subdisk */
    {
	dev = VINUM_SD(sdno);			    /* create the device number */
-	bp->b_flags = B_ORDERED | B_PAGING;    /* and make this an ordered write */
+	bp->b_flags |= B_ORDERED;		    /* and make this an ordered write */
	bp->b_cmd = BUF_CMD_WRITE;
	bp->b_resid = bp->b_bcount;
	bp->b_bio1.bio_offset = (off_t)sd->revived << DEV_BSHIFT;		    /* write it to here */
@@ -219,11 +217,8 @@ revive_block(int sdno)
	    sd->waitlist = sd->waitlist->next;		    /* and move on to the next */
	}
    }
-    if (bp->b_qindex == 0) {				    /* not on a queue, */
-	bp->b_flags |= B_INVAL;
-	bp->b_flags &= ~B_ERROR;
-	brelse(bp);					    /* is this kosher? */
-    }
+    Free(bp->b_data);
+    relpbuf(bp, &vinum_conf.physbufs);
    return error;
}
@@ -321,9 +316,8 @@ parityops(struct vinum_ioctl_msg *data)
    }
    if (pbp->b_flags & B_ERROR)
	reply->error = pbp->b_error;
-    pbp->b_flags |= B_INVAL;
-    pbp->b_flags &= ~B_ERROR;
-    brelse(pbp);
+    Free(pbp->b_data);
+    relpbuf(pbp, &vinum_conf.physbufs);
    unlockrange(plexno, lock);
}
@@ -397,18 +391,16 @@ parityrebuild(struct plex *plex,
    for (sdno = 0; sdno < bufcount; sdno++) {		    /* for each subdisk */
	if ((sdno != psd) || (op != rebuildparity)) {
	    /* Get a buffer header and initialize it. */
-	    crit_enter();
-	    bpp[sdno] = geteblk(mysize);		    /* Get a buffer */
+	    bpp[sdno] = trypbuf(&vinum_conf.physbufs);	    /* Get a buffer */
	    if (bpp[sdno] == NULL) {
		while (sdno-- > 0) {			    /* release the ones we got */
-		    bpp[sdno]->b_flags |= B_INVAL;
-		    brelse(bpp[sdno]);			    /* give back our resources */
+		    Free(bpp[sdno]->b_data);
+		    relpbuf(bpp[sdno], &vinum_conf.physbufs); /* give back our resources */
		}
-		crit_exit();
		printf("vinum: can't allocate buffer space for parity op.\n");
		return NULL;				    /* no bpps */
	    }
-	    crit_exit();
+	    bpp[sdno]->b_data = Malloc(mysize);
	    if (sdno == psd)
		parity_buf = (int *) bpp[sdno]->b_data;
	    if (sdno == newpsd)				    /* the new one? */
@@ -416,7 +408,6 @@ parityrebuild(struct plex *plex,
	    else
		bpp[sdno]->b_bio1.bio_driver_info = VINUM_SD(plex->sdnos[sdno]);	/* device number */
	    bpp[sdno]->b_cmd = BUF_CMD_READ;	    /* either way, read it */
-	    bpp[sdno]->b_flags = B_PAGING;
	    bpp[sdno]->b_bcount = mysize;
	    bpp[sdno]->b_resid = bpp[sdno]->b_bcount;
	    bpp[sdno]->b_bio1.bio_offset = (off_t)pstripe << DEV_BSHIFT;	    /* transfer from here */
@@ -468,8 +459,8 @@ parityrebuild(struct plex *plex,
	    }
	}
	if (sdno != psd) {				    /* release all bps except parity */
-	    bpp[sdno]->b_flags |= B_INVAL;
-	    brelse(bpp[sdno]);				    /* give back our resources */
+	    Free(bpp[sdno]->b_data);
+	    relpbuf(bpp[sdno], &vinum_conf.physbufs);	    /* give back our resources */
	}
    }
@@ -489,8 +480,8 @@ parityrebuild(struct plex *plex,
		break;
	    }
	}
-	bpp[psd]->b_flags |= B_INVAL;
-	brelse(bpp[psd]);				    /* give back our resources */
+	Free(bpp[psd]->b_data);
+	relpbuf(bpp[psd], &vinum_conf.physbufs);	    /* give back our resources */
    }
    /* release our resources */
    Free(bpp);
@@ -543,14 +534,13 @@ initsd(int sdno, int verify)
    size = min(sd->init_blocksize >> DEV_BSHIFT, sd->sectors - sd->initialized) << DEV_BSHIFT;

+    bp = trypbuf(&vinum_conf.physbufs);		    /* Get a buffer */
+    if (bp == NULL)
+	return ENOMEM;
+    bp->b_data = Malloc(size);
+
    verified = 0;
    while (!verified) {					    /* until we're happy with it, */
-	crit_enter();
-	bp = geteblk(size);				    /* Get a buffer */
-	crit_exit();
-	if (bp == NULL)
-	    return ENOMEM;
-
	bp->b_bcount = size;
	bp->b_resid = bp->b_bcount;
	bp->b_bio1.bio_offset = (off_t)sd->initialized << DEV_BSHIFT;		    /* write it to here */
@@ -561,49 +551,33 @@ initsd(int sdno, int verify)
	biowait(bp);
	if (bp->b_flags & B_ERROR)
	    error = bp->b_error;
-	if (bp->b_qindex == 0) {			    /* not on a queue, */
-	    bp->b_flags |= B_INVAL;
-	    bp->b_flags &= ~B_ERROR;
-	    brelse(bp);					    /* is this kosher? */
-	}
	if ((error == 0) && verify) {			    /* check that it got there */
-	    crit_enter();
-	    bp = geteblk(size);				    /* get a buffer */
-	    if (bp == NULL) {
-		crit_exit();
-		error = ENOMEM;
-	    } else {
-		bp->b_bcount = size;
-		bp->b_resid = bp->b_bcount;
-		bp->b_bio1.bio_offset = (off_t)sd->initialized << DEV_BSHIFT;	    /* read from here */
-		bp->b_bio1.bio_driver_info = VINUM_SD(sdno);
-		bp->b_cmd = BUF_CMD_READ;		    /* read it back */
-		crit_exit();
-		sdio(&bp->b_bio1);
-		biowait(bp);
-		/*
-		 * XXX Bug fix code.  This is hopefully no
-		 * longer needed (21 February 2000).
-		 */
-		if (bp->b_flags & B_ERROR)
-		    error = bp->b_error;
-		else if ((*bp->b_data != 0)		    /* first word spammed */
-		||(bcmp(bp->b_data, &bp->b_data[1], bp->b_bcount - 1))) { /* or one of the others */
-		    printf("vinum: init error on %s, offset 0x%llx sectors\n",
-			sd->name,
-			(long long) sd->initialized);
-		    verified = 0;
-		} else
-		    verified = 1;
-		if (bp->b_qindex == 0) {		    /* not on a queue, */
-		    bp->b_flags |= B_INVAL;
-		    bp->b_flags &= ~B_ERROR;
-		    brelse(bp);				    /* is this kosher? */
-		}
-	    }
+	    bp->b_bcount = size;
+	    bp->b_resid = bp->b_bcount;
+	    bp->b_bio1.bio_offset = (off_t)sd->initialized << DEV_BSHIFT;	    /* read from here */
+	    bp->b_bio1.bio_driver_info = VINUM_SD(sdno);
+	    bp->b_cmd = BUF_CMD_READ;		    /* read it back */
+	    sdio(&bp->b_bio1);
+	    biowait(bp);
+	    /*
+	     * XXX Bug fix code.  This is hopefully no
+	     * longer needed (21 February 2000).
+	     */
+	    if (bp->b_flags & B_ERROR)
+		error = bp->b_error;
+	    else if ((*bp->b_data != 0)		    /* first word spammed */
+	    ||(bcmp(bp->b_data, &bp->b_data[1], bp->b_bcount - 1))) { /* or one of the others */
+		printf("vinum: init error on %s, offset 0x%llx sectors\n",
+		    sd->name,
+		    (long long) sd->initialized);
+		verified = 0;
+	    } else
+		verified = 1;
	} else
	    verified = 1;
    }
+    Free(bp->b_data);
+    relpbuf(bp, &vinum_conf.physbufs);
    if (error == 0) {					    /* did it, */
	sd->initialized += size >> DEV_BSHIFT;		    /* moved this much further down */
	if (sd->initialized >= sd->sectors) {		    /* finished */
diff -r b16b8dd8a0d1 sys/dev/raid/vinum/vinumvar.h
--- a/sys/dev/raid/vinum/vinumvar.h	Fri Jul 07 15:03:06 2006 +0200
+++ b/sys/dev/raid/vinum/vinumvar.h	Fri Jul 07 15:04:51 2006 +0200
@@ -313,6 +313,7 @@ struct _vinum_conf {
    struct request *lastrq;
    struct bio *lastbio;
#endif
+    int physbufs;
};
/* Use these defines to simplify code */

--
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \




More information about the Bugs mailing list