Bug 24220

Summary:	sa driver increments blkno twice on short reads
Product:	Base System	Reporter:	jean-francois.dockes <jean-francois.dockes>
Component:	kern	Assignee:	Matt Jacob <mjacob>
Status:	Closed FIXED
Severity:	Affects Only Me
Priority:	Normal
Version:	Unspecified
Hardware:	Any
OS:	Any

Description jean-francois.dockes 2001-01-10 13:30:01 UTC

When a read() call longer than the next block results in a residual (on 
a device in variable block size mode), the current block number is 
incremented twice in the sa driver, once in saerror(), and once in 
sadone().
sadone() should not increment the block number in the case where 'error'
is 0, but resid is not because this resulted in a check condition 
with ILI bit set in the sense data, and saerror() already 
incremented the blkno.

Fix: 

I think it's all in the description, in variable block size mode.
don't bump blkno if resid is set in sadone(),it was done by saerror()
(or an equivalent fix to saerror).
How-To-Repeat: mouton$ cat /tmp/testscript 
set -x
  export TAPE=/dev/nrsa0
  mt rew
  dd if=/dev/zero of=$TAPE bs=512 count=100
  mt rew
  mt status
  dd if=$TAPE bs=1024 count=1
  mt status
  dd if=$TAPE bs=1024 count=1
  mt status

## Script output slightly edited for clarity
mouton$ sh /tmp/testscript           
+ export TAPE=/dev/nrsa0

+ mt rew

+ dd if=/dev/zero of=/dev/nrsa0 bs=512 count=100
100+0 records in
100+0 records out
51200 bytes transferred in 0.164832 secs (310619 bytes/sec)

+ mt rew

+ mt status
Mode      Density              Blocksize      bpi      Compression
Current:  0x25:DDS-3           variable       97000    DCLZ
File Number: 0  Record Number: 0

+ dd if=/dev/nrsa0 bs=1024 count=1
0+1 records in
0+1 records out
512 bytes transferred in 0.025843 secs (19812 bytes/sec)


+ mt status
Mode      Density              Blocksize      bpi      Compression
Current:  0x25:DDS-3           variable       97000    DCLZ
File Number: 0  Record Number: 2

+ dd if=/dev/nrsa0 bs=1024 count=1
0+1 records in
0+1 records out
512 bytes transferred in 0.015038 secs (34047 bytes/sec)

+ mt status
Mode      Density              Blocksize      bpi      Compression
Current:  0x25:DDS-3           variable       97000    DCLZ
File Number: 0  Record Number: 4

Comment 1 mjacob 2001-01-10 19:25:37 UTC

Hey, thanks for the test cases! I'll fix this next week!


On Wed, 10 Jan 2001 jean-francois.dockes@musicmaker.com wrote:

> 
> >Number:         24220
> >Category:       kern
> >Synopsis:       sa driver increments blkno twice on short reads
> >Confidential:   no
> >Severity:       non-critical
> >Priority:       low
> >Responsible:    freebsd-bugs
> >State:          open
> >Quarter:        
> >Keywords:       
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Wed Jan 10 05:30:01 PST 2001
> >Closed-Date:
> >Last-Modified:
> >Originator:     Jean-Francois Dockes
> >Release:        4.2 STABLE, Nov 30 2000
> >Organization:
> >Environment:
> FreeBSD mouton 4.2-STABLE FreeBSD 4.2-STABLE #0: Fri Dec  1 21:47:38 CET 2000     dockes@mouton:/usr/obj/hautmedoc/u/src/sys/HAUTMEDOC  i386
> 
> >Description:
> When a read() call longer than the next block results in a residual (on 
> a device in variable block size mode), the current block number is 
> incremented twice in the sa driver, once in saerror(), and once in 
> sadone().
> sadone() should not increment the block number in the case where 'error'
> is 0, but resid is not because this resulted in a check condition 
> with ILI bit set in the sense data, and saerror() already 
> incremented the blkno.
> >How-To-Repeat:
> mouton$ cat /tmp/testscript 
> set -x
>   export TAPE=/dev/nrsa0
>   mt rew
>   dd if=/dev/zero of=$TAPE bs=512 count=100
>   mt rew
>   mt status
>   dd if=$TAPE bs=1024 count=1
>   mt status
>   dd if=$TAPE bs=1024 count=1
>   mt status
> 
> ## Script output slightly edited for clarity
> mouton$ sh /tmp/testscript           
> + export TAPE=/dev/nrsa0
> 
> + mt rew
> 
> + dd if=/dev/zero of=/dev/nrsa0 bs=512 count=100
> 100+0 records in
> 100+0 records out
> 51200 bytes transferred in 0.164832 secs (310619 bytes/sec)
> 
> + mt rew
> 
> + mt status
> Mode      Density              Blocksize      bpi      Compression
> Current:  0x25:DDS-3           variable       97000    DCLZ
> File Number: 0  Record Number: 0
> 
> + dd if=/dev/nrsa0 bs=1024 count=1
> 0+1 records in
> 0+1 records out
> 512 bytes transferred in 0.025843 secs (19812 bytes/sec)
> 
> 
> + mt status
> Mode      Density              Blocksize      bpi      Compression
> Current:  0x25:DDS-3           variable       97000    DCLZ
> File Number: 0  Record Number: 2
> 
> + dd if=/dev/nrsa0 bs=1024 count=1
> 0+1 records in
> 0+1 records out
> 512 bytes transferred in 0.015038 secs (34047 bytes/sec)
> 
> + mt status
> Mode      Density              Blocksize      bpi      Compression
> Current:  0x25:DDS-3           variable       97000    DCLZ
> File Number: 0  Record Number: 4
> 
> >Fix:
> I think it's all in the description, in variable block size mode.
> don't bump blkno if resid is set in sadone(),it was done by saerror()
> (or an equivalent fix to saerror).
> 
> >Release-Note:
> >Audit-Trail:
> >Unformatted:
> 
> 
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-bugs" in the body of the message
>

Comment 2 Matt Jacob freebsd_committer

2001-01-10 19:27:15 UTC

Responsible Changed
From-To: freebsd-bugs->mjacob

my driver 
.

Comment 3 Matt Jacob freebsd_committer

2001-01-15 22:28:53 UTC

State Changed
From-To: open->closed

Problem fixed in revision 1.61 scsi_sa.c.