|
Hi! I’ve been making some (mostly cosmetic) changes to pdcraid.c and quite accidentally I came across a very interesting
bug. I’m in the middle of a lot of stuff so it won’t
be easy to produce a patch for the next few days. J The bug will result in any drive which reports a native
number of heads being anything other than 16 (technically any multiple of 8) will
not mount. Maybe this will help all
those people who email the list saying ‘On xxx drive pdcraid
mounts my array but with yyy drive it doesn’t’. Here’s the bug: static int read_disk_sb (int major, int minor, unsigned
char *buffer,int bufsize) { int ret = -EINVAL; struct buffer_head
*bh = NULL; kdev_t dev = MKDEV(major,minor); unsigned long sb_offset; if (blksize_size[major]==NULL) /* device doesn't exist */ return -EINVAL;
/* * Calculate the position of the superblock, * it's at first
sector of the last cylinder */ sb_offset = calc_pdcblock_offset(major,minor)/8; /*
The /8 transforms sectors into 4Kb blocks */ and then a little later… static void __init
probedisk(int devindex,int device, int raidlevel) { int i; int major, minor; struct promise_raid_conf
*prom; static unsigned char block[4096]; if (devlist[devindex].device!=-1)
/* already assigned to another array */ return; major = devlist[devindex].major; minor = devlist[devindex].minor; if (read_disk_sb(major,minor,(unsigned char*)&block,sizeof(block))) return;
prom = (struct promise_raid_conf*)&block[512]; First you calculate the lba
address of the superblock and immediately divide it
by 8 (for bread). The problem is
that in the second snippet of code you assume that the block you’re
looking for is one sector into the buffer. In fact it always is in that location if
the native heads is 16. However if
the native heads is at 15, the buffer you’re looking for is the 4th
sector in the buffer. Oops, now the
superblock won’t be found and you’ll end
up not finding the array. Here’s what I did to remedy the problem: In read_disk_sb: /* * Calculate the position of the superblock, * it's at first
sector of the last cylinder */ sb_offset = calc_pdcblock_offset(major,minor); if (sb_offset==0) return -1; set_blocksize (dev, 4096); bh = bread (dev, sb_offset/8,
8192); /*
The /8 transforms sectors into 4Kb blocks */ if (bh) { memcpy (buffer, bh->b_data +
((sb_offset%8)*512), bufsize); }
else { printk(KERN_ERR
"pdcraid: Error reading superblock.\n"); goto abort; } Notice that I’m reading 16 sectors instead of 8. This is mostly a precaution. I also adjust the memcpy
so that the superblock always gets copied to the
beginning of the buffer. This means that I also changed.
prom = (struct promise_raid_conf*)&block[512]; to: prom = (struct promise_raid_conf*)block; And that’s it.
If you want a patch, I can generate one on Monday. Also, as a side note, I’ve made some other changes
(those cosmetic changes mentioned earlier). I noticed that in the source, you never
check the superblock for PR_MAGIC. Is there any reason why this is not
happening? I would think this would
make probedisk a little more secure against incorrectly
mounting a wrong superblock. And, the one that started my foray into this, I made a
change so that probedisk would only mount the superblock if the drive was running on the pdc202xx
chipset. While it is neat that you
can take a drive set that has been running on the promise controller and
effectively remove the promise controller and everything still works, my
problem was that the drive had long since been removed from the promise
controller and the superblock described a
configuration which was no longer valid.
Maybe this should be a kernel option or maybe a module option? |
BEGIN:VCARD VERSION:2.1 N:Burr;Phillip FN:Phillip Burr ORG:;Development TITLE:Software Engineer TEL;WORK;VOICE:+1-801-437-8317 EMAIL;PREF;INTERNET:Phillip.Burr@xxxxxxxxxxxxxx REV:20020211T183314Z END:VCARD
