Date: Mon, 10 Oct 2005 13:31:39 +0200 From: =?ISO-8859-1?Q?S=F8ren_Schmidt?= <sos@FreeBSD.ORG> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: src-committers@FreeBSD.ORG, Pawel Jakub Dawidek <pjd@FreeBSD.ORG>, Peter Edwards <peadar.edwards@gmail.com>, Peter Edwards <peadar@FreeBSD.ORG>, cvs-src@FreeBSD.ORG, cvs-all@FreeBSD.ORG Subject: Re: cvs commit: src/sys/dev/ata atapi-cd.c Message-ID: <6962DB1A-370D-45CC-9FD2-AB44B7745B20@FreeBSD.ORG> In-Reply-To: <13986.1128942110@critter.freebsd.dk> References: <13986.1128942110@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/10/2005, at 13:01, Poul-Henning Kamp wrote:
> In message =20
> <34cb7c840510100356t1b93e679v6479afda16277afa@mail.gmail.com>, Peter
> Edwards writes:
>
>>> +> Please see geom_disk.c
>> I think an advantage to Poul-Henning's approach is that it reduces
>> latency: the I/O can start immediately, rather than requiring all the
>> bio's to be allocated first. The very fact that the race condition =20=
>> was
>> triggered indicates that the IO devices can overtake the CPU. You
>> might waste some time in the failure case, but that's obviously a
>> small price to pay for improved performance in the normal run of
>> things.
>
> Agreed.
OK, see the attached patch, if that does it for you I'll commit it to =20=
-current and keep maintainership there, then re@ can decide what they =20=
want with 6.0 ;)
> I've started wondering if I should actually put this code in a
> function so we don't have to reinvent it all over the place.
Good idea!
The patch is against version 1.180 of atapi-cd.c
retrieving revision 1.180
diff -u -r1.180 atapi-cd.c
--- atapi-cd.c 17 Aug 2005 14:50:18 -0000 1.180
+++ atapi-cd.c 10 Oct 2005 11:28:52 -0000
@@ -760,20 +760,28 @@
}
else {
u_int pos, size =3D cdp->iomax - cdp->iomax % bp->bio_to-=20
>sectorsize;
- struct bio *bp2;
+ struct bio *bp2, *bp3;
- for (pos =3D 0; pos < bp->bio_length; pos +=3D size) {
- if (!(bp2 =3D g_clone_bio(bp))) {
- bp->bio_error =3D ENOMEM;
- break;
- }
+ if (!(bp2 =3D g_clone_bio(bp))) {
+ g_io_deliver(bp, EIO);
+ return;
+ }
+
+ for (pos =3D 0; bp2; pos +=3D size) {
+ bp3 =3D NULL;
bp2->bio_done =3D g_std_done;
bp2->bio_to =3D bp->bio_to;
bp2->bio_offset +=3D pos;
bp2->bio_data +=3D pos;
- bp2->bio_length =3D MIN(size, bp->bio_length - pos);
+ bp2->bio_length =3D bp->bio_length - pos;
+ if (bp2->bio_length > size) {
+ bp2->bio_length =3D size;
+ if (!(bp3 =3D g_clone_bio(bp)))
+ bp->bio_error =3D ENOMEM;
+ }
bp2->bio_pblkno =3D bp2->bio_offset / bp2->bio_to-=20
>sectorsize;
acd_strategy(bp2);
+ bp2 =3D bp3;
}
}
}
S=F8ren Schmidt
sos@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6962DB1A-370D-45CC-9FD2-AB44B7745B20>