cvs commit: src/sys/dev/ata atapi-cd.c

[Søren Schmidt]

On 10/10/2005, at 13:01, Poul-Henning Kamp wrote:

> In message  
> <34cb7c840510100356t1b93e679v6479afda16277afa at 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  
>> 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  
-current and keep maintainership there, then re@ can decide what they  
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 = cdp->iomax - cdp->iomax % bp->bio_to- 
 >sectorsize;
-       struct bio *bp2;
+       struct bio *bp2, *bp3;
-       for (pos = 0; pos < bp->bio_length; pos += size) {
-           if (!(bp2 = g_clone_bio(bp))) {
-               bp->bio_error = ENOMEM;
-               break;
-           }
+       if (!(bp2 = g_clone_bio(bp))) {
+           g_io_deliver(bp, EIO);
+           return;
+       }
+
+       for (pos = 0; bp2; pos += size) {
+           bp3 = NULL;
             bp2->bio_done = g_std_done;
             bp2->bio_to = bp->bio_to;
             bp2->bio_offset += pos;
             bp2->bio_data += pos;
-           bp2->bio_length = MIN(size, bp->bio_length - pos);
+           bp2->bio_length = bp->bio_length - pos;
+           if (bp2->bio_length > size) {
+               bp2->bio_length = size;
+               if (!(bp3 = g_clone_bio(bp)))
+                   bp->bio_error = ENOMEM;
+           }
             bp2->bio_pblkno = bp2->bio_offset / bp2->bio_to- 
 >sectorsize;
             acd_strategy(bp2);
+           bp2 = bp3;
         }
      }
}


Søren Schmidt
sos at FreeBSD.org