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>