kern/95707: PATCH: iostat's transfer rate for cdroms is wrong

[Andre Albsmeier]

>Number:         95707
>Category:       kern
>Synopsis:       PATCH: iostat's transfer rate for cdroms is wrong
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Apr 13 17:40:17 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Andre Albsmeier
>Release:        FreeBSD 5.4-STABLE i386
>Organization:
>Environment:

System: FreeBSD 5.4-STABLE Tue Mar 7 21:56:34 CET 2006

>Description:

The transfer rate of cdroms shown by iostat is wrong (exactly
twice as big as it should be).

>How-To-Repeat:

Copy a big file from cd (cd0 in this case) to an idle disk
(da0 in this case) on an otherwise idle system. In another
window run iostat for the two devices and compare the results:

andre at voyager:~>iostat -d cd0 da0 1
             da0              cd0
  KB/t tps  MB/s   KB/t tps  MB/s
  5.74  23  0.13   0.00   0  0.00
  6.24  17  0.10   0.00   0  0.00
 66.26  35  2.24  64.00  73  4.58
 11.08 247  2.67  64.00  30  1.86
115.63  30  3.35  64.00  93  5.82
123.20  25  2.98  64.00  95  5.94
 77.72  40  3.01  64.00  95  5.94
107.14  29  3.00  64.00  95  5.94
119.70  27  3.12  64.00  97  6.06
 65.98  49  3.13  64.00  97  6.06
107.30  30  3.11  64.00  99  6.19
 86.63  38  3.18  64.00  99  6.19
 58.17  59  3.37  64.00  99  6.19
108.19  31  3.24  64.00 101  6.31
111.45  29  3.13  64.00 101  6.31
 77.29  45  3.36  64.00 103  6.44
 73.47  47  3.34  64.00 103  6.44
 63.23  55  3.42  64.00 103  6.44
...

If the cd is full and the read process reaches the outer
(faster) areas of the cd, the cd0 transfer rate shown
climbs up to 12MB/sec. This would mean my Plextor PX-40
reads at 80x speed :-).

>Fix:

[Don't know if this is correct...]

When reading cdroms, devstat_end_transaction_bio() is called
twice at the end of each transfer. One time from g_disk_done()
in geom_disk.c and another time from biofinish() in vfs_bio.c
which in turn was called from cddone() in scsi_cd.c.

We can avoid accounting in the second case by passing NULL to
biofinish() as devstat*:

--- sys/cam/scsi/scsi_cd.c.ORI	Mon Nov 28 06:57:04 2005
+++ sys/cam/scsi/scsi_cd.c	Thu Apr 13 16:03:49 2006
@@ -1669,7 +1669,7 @@
 		if (softc->flags & CD_FLAG_CHANGER)
 			cdchangerschedule(softc);
 
-		biofinish(bp, softc->disk->d_devstat, 0);
+		biofinish(bp, NULL, 0);
 		break;
 	}
 	case CD_CCB_PROBE:
>Release-Note:
>Audit-Trail:
>Unformatted: