Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 5 Oct 2005 14:13:59 +0100
From:      Peter Edwards <peadar.edwards@gmail.com>
To:        freebsd-current@freebsd.org
Cc:        jkim@freebsd.org
Subject:   Re: biodone panics
Message-ID:  <34cb7c840510050613u36e76be1pef82c41130da8804@mail.gmail.com>
In-Reply-To: <200509291303.41181.jhb@FreeBSD.org>
References:  <34cb7c8405092815247dc89bf6@mail.gmail.com> <1127978233.3383.12.camel@berloga.shadowland> <200509291303.41181.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
------=_Part_12820_8414373.1128518039151
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

Take 2 for the biodone() panics:

acd_geom_start() implements request limiting by breaking up a large
request in the passed bio into a sequence of smaller ones.  As each
request is created,  acd_strategy is invoked to start the IO.

However, I think this IO can complete while still issuing the child
requests, leading to the parent being retired early. (ie, when a child
operation completes, it checks if its the last operation to complete,
and, if so, retires the parent: see g_std_done.)

The attached patch makes my qemu box much more reliable (I could crash
a qemu hosted system 100% with a "tar fc" of the 6.0-BETA bootonly ISO
without the patch, and it's gone through many iterations fine with it)

Any opinions/testing results welcome.

------=_Part_12820_8414373.1128518039151
Content-Type: text/plain; name=ata-geom.txt; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="ata-geom.txt"

Index: sys/dev/ata/atapi-cd.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ata/atapi-cd.c,v
retrieving revision 1.179.2.1
diff -u -r1.179.2.1 atapi-cd.c
--- sys/dev/ata/atapi-cd.c	25 Aug 2005 16:21:05 -0000	1.179.2.1
+++ sys/dev/ata/atapi-cd.c	5 Oct 2005 12:01:33 -0000
@@ -760,7 +760,7 @@
     }
     else {
 	u_int pos, size = cdp->iomax - cdp->iomax % bp->bio_to->sectorsize;
-	struct bio *bp2;
+	struct bio *bp2, *first, *cur, **next = &first;
 
 	for (pos = 0; pos < bp->bio_length; pos += size) {
 	    if (!(bp2 = g_clone_bio(bp))) {
@@ -773,7 +773,13 @@
 	    bp2->bio_data += pos;
 	    bp2->bio_length = MIN(size, bp->bio_length - pos);
 	    bp2->bio_pblkno = bp2->bio_offset / bp2->bio_to->sectorsize;
-	    acd_strategy(bp2);
+	    *next = bp2;
+	    next = (struct bio **)&bp2->bio_driver1;
+	}
+	*next = 0;
+	while ((cur = first) != 0) {
+	    first = (struct bio *)cur->bio_driver1;
+	    acd_strategy(cur);
 	}
     }
 }

------=_Part_12820_8414373.1128518039151--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?34cb7c840510050613u36e76be1pef82c41130da8804>