Subject: kern/495: Non-512 Byte per sector disks don't work
To: None <gnats-admin@sun-lamp.cs.berkeley.edu>
From: Mark P. Gooderum <mark@nirvana.good.com>
List: netbsd-bugs
Date: 09/23/1994 05:20:03
>Number: 495
>Category: kern
>Synopsis: Non-512 Byte per sector disks
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: gnats-admin (Kernel Bug People)
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Sep 23 05:20:02 1994
>Originator: Mark P. Gooderum
>Organization:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Mark P. Gooderum USSnail: Good Creations
Senior Consultant - Operating Systems Group 3029 Blackstone Ave. So.
"Working hard to be hardly working..." St. Louis Park, MN 55416
EMail: mark@Good.com Voice: (612) 922-3953
Interactive: mark@nirvana.Good.com Fax: (612) 922-2676
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>Release: NetBSD 1.0 Beta
>Environment:
486DLC/40
aic6360 or AHA-1542CF
Maxtor 7620S SCSI Drive
System: NetBSD nirvana 1.0_BETA NetBSD 1.0_BETA (NIRVANA_GATEWAY.1024) #12: Mon Sep 19 21:12:17 CDT 1994 mark@nirvana:/export/usr/src/sys/arch/i386/compile/NIRVANA_GATEWAY.1024 i386
>Description:
A disk that doesn't have 512 byte sectors doesn't work, for many reasons:
sd.c assumes 512 byte sector sizes in all I/O requests and converts that
to device sectors. The raw r/w stuff (physio) calls sd.c in this way,
but ufs and the cd9660 drivers do their own conversion so sd.c trashes
this.
The disklabel code also gets munged by this.
bounds_check_with_label() assumes device's byte sector requests so it shows
an overrun with physio() calls.
>How-To-Repeat:
Try to label a 1024 or non-512 byte per sector disk.
If you fudge the label, then try to newfs it, you'll get errors when more
than 1/2 way past the disk.
>Fix:
Fix sd.c to only adjust block number and count if B_RAW is set (physio()
sets this, fs code doesn't).
Fix bounds_check_with_label() to take this into account as well during
it's calculations.
Fix write/readdisklabel() to set the B_RAW flag to get sd.c to DTRT.
The attached patches do that. With them you can newfs, fsck, and
label all disks until the cows come home, successfully. I've also
tested the patches with 512 byte sector drives, and with IDE drive/controller,
no problems.
There is one more problem. When writing files, the data within the file
gets corrupted, always on the first byte of a new cluster, ie: byte 8193,
etc. The first cluster is always written okay, but subsequent clusters
usually show errors, usually the 2nd one. I haven't had a chance to
track this down in more detail. I'll be writing an I/O test program to
give the exact nature of the problem. Note that Micheal Hitch did
similar fixes to the Amiga port (these patches are either his (the sd one)
or inspired by his) and didn't have any UFS problems...byte sexing dependancy
in all the FS/Block/Cluster macros?? Just a guess, but maybe a good one.
I'm willing (and am) continueing to attack this myself, but pointers on
where to look would help.
--
Mark Gooderum
mark@good.com
*** sd.c.orig Mon Sep 12 20:56:31 1994
--- sd.c Wed Sep 7 19:02:16 1994
***************
*** 491,499 ****
* We have a buf, now we know we are going to go through
* With this thing..
*
! * First, translate the block to absolute
*/
! blkno = bp->b_blkno / (sd->params.blksize / DEV_BSIZE);
if (SDPART(bp->b_dev) != RAW_PART) {
p = &sd->sc_dk.dk_label.d_partitions[SDPART(bp->b_dev)];
blkno += p->p_offset;
--- 491,501 ----
* We have a buf, now we know we are going to go through
* With this thing..
*
! * First, translate the block to absolute if B_RAW
*/
! blkno = bp->b_blkno;
! if (bp->b_flags & B_RAW)
! blkno /= sd->params.blksize / DEV_BSIZE;
if (SDPART(bp->b_dev) != RAW_PART) {
p = &sd->sc_dk.dk_label.d_partitions[SDPART(bp->b_dev)];
blkno += p->p_offset;
***************
*** 645,655 ****
*/
sd->sc_dk.dk_label.d_partitions[0].p_offset = 0;
sd->sc_dk.dk_label.d_partitions[0].p_size =
! sd->params.disksize * (sd->params.blksize / DEV_BSIZE);
sd->sc_dk.dk_label.d_partitions[0].p_fstype = 9; /* XXXX */
sd->sc_dk.dk_label.d_partitions[RAW_PART].p_offset = 0;
sd->sc_dk.dk_label.d_partitions[RAW_PART].p_size =
! sd->params.disksize * (sd->params.blksize / DEV_BSIZE);
sd->sc_dk.dk_label.d_npartitions = MAXPARTITIONS;
sd->sc_dk.dk_label.d_secsize = sd->params.blksize;
--- 647,657 ----
*/
sd->sc_dk.dk_label.d_partitions[0].p_offset = 0;
sd->sc_dk.dk_label.d_partitions[0].p_size =
! sd->params.disksize;
sd->sc_dk.dk_label.d_partitions[0].p_fstype = 9; /* XXXX */
sd->sc_dk.dk_label.d_partitions[RAW_PART].p_offset = 0;
sd->sc_dk.dk_label.d_partitions[RAW_PART].p_size =
! sd->params.disksize;
sd->sc_dk.dk_label.d_npartitions = MAXPARTITIONS;
sd->sc_dk.dk_label.d_secsize = sd->params.blksize;
*** disksubr.c.orig Mon Sep 12 20:57:10 1994
--- disksubr.c Fri Sep 23 07:02:10 1994
***************
*** 106,112 ****
/* read master boot record */
bp->b_blkno = DOSBBSECTOR;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_BUSY | B_READ;
bp->b_cylin = DOSBBSECTOR / lp->d_secpercyl;
(*strat)(bp);
--- 106,112 ----
/* read master boot record */
bp->b_blkno = DOSBBSECTOR;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_BUSY | B_READ | B_RAW;
bp->b_cylin = DOSBBSECTOR / lp->d_secpercyl;
(*strat)(bp);
***************
*** 149,155 ****
bp->b_blkno = dospartoff + LABELSECTOR;
bp->b_cylin = cyl;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_BUSY | B_READ;
(*strat)(bp);
/* if successful, locate disk label within block and validate */
--- 149,155 ----
bp->b_blkno = dospartoff + LABELSECTOR;
bp->b_cylin = cyl;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_BUSY | B_READ | B_RAW;
(*strat)(bp);
/* if successful, locate disk label within block and validate */
***************
*** 157,163 ****
msg = "disk label I/O error";
goto done;
} else for (dlp = (struct disklabel *)bp->b_data;
! dlp <= (struct disklabel *)(bp->b_data + DEV_BSIZE - sizeof(*dlp));
dlp = (struct disklabel *)((char *)dlp + sizeof(long))) {
if (dlp->d_magic != DISKMAGIC || dlp->d_magic2 != DISKMAGIC) {
if (msg == NULL)
--- 157,164 ----
msg = "disk label I/O error";
goto done;
} else for (dlp = (struct disklabel *)bp->b_data;
! dlp <= (struct disklabel *)(bp->b_data +
! lp->d_secsize - sizeof(*dlp));
dlp = (struct disklabel *)((char *)dlp + sizeof(long))) {
if (dlp->d_magic != DISKMAGIC || dlp->d_magic2 != DISKMAGIC) {
if (msg == NULL)
***************
*** 182,193 ****
i = 0;
do {
/* read a bad sector table */
! bp->b_flags = B_BUSY | B_READ;
bp->b_blkno = lp->d_secperunit - lp->d_nsectors + i;
- if (lp->d_secsize > DEV_BSIZE)
- bp->b_blkno *= lp->d_secsize / DEV_BSIZE;
- else
- bp->b_blkno /= DEV_BSIZE / lp->d_secsize;
bp->b_bcount = lp->d_secsize;
bp->b_cylin = lp->d_ncylinders - 1;
(*strat)(bp);
--- 183,190 ----
i = 0;
do {
/* read a bad sector table */
! bp->b_flags = B_BUSY | B_READ | B_RAW;
bp->b_blkno = lp->d_secperunit - lp->d_nsectors + i;
bp->b_bcount = lp->d_secsize;
bp->b_cylin = lp->d_ncylinders - 1;
(*strat)(bp);
***************
*** 308,314 ****
if (dp) {
bp->b_blkno = DOSBBSECTOR;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_BUSY | B_READ;
bp->b_cylin = DOSBBSECTOR / lp->d_secpercyl;
(*strat)(bp);
if ((error = biowait(bp)) == 0) {
--- 305,311 ----
if (dp) {
bp->b_blkno = DOSBBSECTOR;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_BUSY | B_READ | B_RAW;
bp->b_cylin = DOSBBSECTOR / lp->d_secpercyl;
(*strat)(bp);
if ((error = biowait(bp)) == 0) {
***************
*** 339,345 ****
bp->b_blkno = dospartoff + LABELSECTOR;
bp->b_cylin = cyl;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_READ;
(*strat)(bp);
if (error = biowait(bp))
goto done;
--- 336,342 ----
bp->b_blkno = dospartoff + LABELSECTOR;
bp->b_cylin = cyl;
bp->b_bcount = lp->d_secsize;
! bp->b_flags = B_READ | B_RAW;
(*strat)(bp);
if (error = biowait(bp))
goto done;
***************
*** 349,355 ****
if (dlp->d_magic == DISKMAGIC && dlp->d_magic2 == DISKMAGIC &&
dkcksum(dlp) == 0) {
*dlp = *lp;
! bp->b_flags = B_WRITE;
(*strat)(bp);
error = biowait(bp);
goto done;
--- 346,352 ----
if (dlp->d_magic == DISKMAGIC && dlp->d_magic2 == DISKMAGIC &&
dkcksum(dlp) == 0) {
*dlp = *lp;
! bp->b_flags = B_WRITE | B_RAW;
(*strat)(bp);
error = biowait(bp);
goto done;
***************
*** 369,387 ****
int
bounds_check_with_label(struct buf *bp, struct disklabel *lp, int wlabel)
{
! struct partition *p = lp->d_partitions + dkpart(bp->b_dev);
! int labelsect = lp->d_partitions[0].p_offset;
! int maxsz = p->p_size,
! sz = (bp->b_bcount + DEV_BSIZE - 1) >> DEV_BSHIFT;
/* overwriting disk label ? */
/* XXX should also protect bootstrap in first 8K */
if (bp->b_blkno + p->p_offset <= LABELSECTOR + labelsect &&
#if LABELSECTOR != 0
! bp->b_blkno + p->p_offset + sz > LABELSECTOR + labelsect &&
#endif
! (bp->b_flags & B_READ) == 0 && wlabel == 0) {
! bp->b_error = EROFS;
goto bad;
}
--- 366,394 ----
int
bounds_check_with_label(struct buf *bp, struct disklabel *lp, int wlabel)
{
! struct partition *p;
! int labelsect;
! int maxsz;
! int sz;
!
! p = lp->d_partitions + dkpart(bp->b_dev);
! labelsect = lp->d_partitions[0].p_offset;
! if (bp->b_flags & B_RAW) {
! maxsz = p->p_size * (lp->d_secsize / DEV_BSIZE);
! sz = (bp->b_bcount + DEV_BSIZE) / DEV_BSIZE;
! } else {
! maxsz = p->p_size;
! sz = (bp->b_bcount + lp->d_secsize - 1) / lp->d_secsize;
! }
/* overwriting disk label ? */
/* XXX should also protect bootstrap in first 8K */
if (bp->b_blkno + p->p_offset <= LABELSECTOR + labelsect &&
#if LABELSECTOR != 0
! bp->b_blkno + p->p_offset + sz > LABELSECTOR + labelsect &&
#endif
! (bp->b_flags & B_READ) == 0 && wlabel == 0) {
! bp->b_error = EROFS;
goto bad;
}
>Audit-Trail:
>Unformatted: