Re: [2.6.13-rc6-latest] SCSI disk registration msgs repeat themselves
On Tue, Aug 16, 2005 at 10:51:13PM -0700, Pete Zaitcev wrote: > On Tue, 16 Aug 2005 21:39:33 -0700, Patrick Mansfield <[EMAIL PROTECTED]> > wrote: > > On Tue, Aug 16, 2005 at 11:01:30PM -0400, Chuck Ebbert wrote: > > > > I just added some usb-storage devices to my system and got the below. > > > > Why do the first four lines repeat for each device? (Not sure if > > > this is a SCSI or USB problem.) > > > > It is in the partition code. I posted twice before about this with no > > response. > > It's not an important problem, presumably. I observe dual revalidations > as well, but they are not that bothersome. Add to it that your patch Yes. Does this *only* happens for sd (scsi) devices? > appears wrong (see below). If you offered an acceptable solution, I would > expect a warmer welcome... But even then getting a reply from linux-scsi > folks is like pulling a tooth (if my own little CD-ROM sizing patch is > any indication). So, steel yourself for challenges of this life, Patrick! ;-) > Here's what it was in 2.6.9, as documented in drivers/block/ub.c: > > + /* > + * This is a workaround for a specific problem in our block layer. > + * In 2.6.9, register_disk duplicates the code from rescan_partitions. > + * However, if we do add_disk with a device which persistently reports > + * a changed media, add_disk calls register_disk, which does do_open, > + * which will call rescan_paritions for changed media. After that, > + * register_disk attempts to do it all again and causes double kobject > + * registration and a eventually an oops on module removal. > + * > + * The bottom line is, Al Viro says that we should not allow > + * bdev->bd_invalidated to be set when doing add_disk no matter what. > + */ > + if (sc->first_open) { > + if (sc->changed) { > + sc->first_open = 0; > + rc = -ENOMEDIUM; > + goto err_open; > + } > + } > > Users were hitting it with oopses like these: > http://www.ussg.iu.edu/hypermail/linux/kernel/0409.2/0011.html > > The ub alone was not suffient to motivate Al for the fix, so I added > this silly "first_open" thingie, which papered over it. It was thought > that sd was miraclously immune. > > However, over time users hit it with usb-storage and sd, like this: > http://lkml.org/lkml/2004/2/21/19 > This prompted Al's action. He simply dropped all the extra code like > this: > > --- linux-2.6.9-11.5.EL/fs/partitions/check.c 2004-10-18 14:55:07.0 > -0700 > +++ linux-2.6.12/fs/partitions/check.c2005-06-17 12:48:29.0 > -0700 > @@ -358,24 +357,9 @@ void register_disk(struct gendisk *disk) > if (!bdev) > return; > > + bdev->bd_invalidated = 1; > if (blkdev_get(bdev, FMODE_READ, 0) < 0) > return; > - state = check_partition(disk, bdev); > - if (state) { > - for (j = 1; j < state->limit; j++) { > - sector_t size = state->parts[j].size; > - sector_t from = state->parts[j].from; > - if (!size) > - continue; > - add_partition(disk, j, from, size); > -#ifdef CONFIG_BLK_DEV_MD > - if (!state->parts[j].flags) > - continue; > - md_autodetect_dev(bdev->bd_dev+j); > -#endif > - } > - kfree(state); > - } > blkdev_put(bdev); > } OK, thanks for posting those links and information. > > --- linux-2.6.11-rc1/fs/partitions/check.c Fri Dec 24 13:35:28 2004 > > +++ no-double-sd-linux-2.6.11-rc1/fs/partitions/check.c Fri Jan 21 > > 11:19:00 2005 > > @@ -375,8 +375,6 @@ int rescan_partitions(struct gendisk *di > > bdev->bd_invalidated = 0; > > for (p = 1; p < disk->minors; p++) > > delete_partition(disk, p); > > - if (disk->fops->revalidate_disk) > > - disk->fops->revalidate_disk(disk); > > As for your proposed fix, it may be problematic. The ->revalidate > method has to be called at least once for a new device, because > that's when drivers fetch the capacities. But ->open only calls > check_disk_change() for removable devices. Who is going to call > ->revalidate inside add_disk() for non-removable devices? sd.c always calls its revalidate_disk method (sd_revalidate_disk) when the device is attached, so for scsi, we definitely do not miss anything. I thought revalidate_disk was not called prior to Al's patch, so why do we need to call it on the first open now? You already have to call set_capacity() before add_disk(), else register_disk thinks there is no media present, and won't set bd_invalidated. So drivers must already get the capacity (or fake it) prior to calling add_disk. -- Patrick Mansfield - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PR
Re: [2.6.13-rc6-latest] SCSI disk registration msgs repeat themselves
On Tue, 16 Aug 2005 21:39:33 -0700, Patrick Mansfield <[EMAIL PROTECTED]> wrote: > On Tue, Aug 16, 2005 at 11:01:30PM -0400, Chuck Ebbert wrote: > > I just added some usb-storage devices to my system and got the below. > > Why do the first four lines repeat for each device? (Not sure if > > this is a SCSI or USB problem.) > > It is in the partition code. I posted twice before about this with no > response. It's not an important problem, presumably. I observe dual revalidations as well, but they are not that bothersome. Add to it that your patch appears wrong (see below). If you offered an acceptable solution, I would expect a warmer welcome... But even then getting a reply from linux-scsi folks is like pulling a tooth (if my own little CD-ROM sizing patch is any indication). So, steel yourself for challenges of this life, Patrick! > The changelog said it was a workaround for a borken device, but not what > device it was or any other details. Here's what it was in 2.6.9, as documented in drivers/block/ub.c: + /* +* This is a workaround for a specific problem in our block layer. +* In 2.6.9, register_disk duplicates the code from rescan_partitions. +* However, if we do add_disk with a device which persistently reports +* a changed media, add_disk calls register_disk, which does do_open, +* which will call rescan_paritions for changed media. After that, +* register_disk attempts to do it all again and causes double kobject +* registration and a eventually an oops on module removal. +* +* The bottom line is, Al Viro says that we should not allow +* bdev->bd_invalidated to be set when doing add_disk no matter what. +*/ + if (sc->first_open) { + if (sc->changed) { + sc->first_open = 0; + rc = -ENOMEDIUM; + goto err_open; + } + } Users were hitting it with oopses like these: http://www.ussg.iu.edu/hypermail/linux/kernel/0409.2/0011.html The ub alone was not suffient to motivate Al for the fix, so I added this silly "first_open" thingie, which papered over it. It was thought that sd was miraclously immune. However, over time users hit it with usb-storage and sd, like this: http://lkml.org/lkml/2004/2/21/19 This prompted Al's action. He simply dropped all the extra code like this: --- linux-2.6.9-11.5.EL/fs/partitions/check.c 2004-10-18 14:55:07.0 -0700 +++ linux-2.6.12/fs/partitions/check.c 2005-06-17 12:48:29.0 -0700 @@ -358,24 +357,9 @@ void register_disk(struct gendisk *disk) if (!bdev) return; + bdev->bd_invalidated = 1; if (blkdev_get(bdev, FMODE_READ, 0) < 0) return; - state = check_partition(disk, bdev); - if (state) { - for (j = 1; j < state->limit; j++) { - sector_t size = state->parts[j].size; - sector_t from = state->parts[j].from; - if (!size) - continue; - add_partition(disk, j, from, size); -#ifdef CONFIG_BLK_DEV_MD - if (!state->parts[j].flags) - continue; - md_autodetect_dev(bdev->bd_dev+j); -#endif - } - kfree(state); - } blkdev_put(bdev); } I was just about to remove "first_open" from ub, because it's unnecessary with Al's fix and it was getting on my nerves. > --- linux-2.6.11-rc1/fs/partitions/check.cFri Dec 24 13:35:28 2004 > +++ no-double-sd-linux-2.6.11-rc1/fs/partitions/check.c Fri Jan 21 > 11:19:00 2005 > @@ -375,8 +375,6 @@ int rescan_partitions(struct gendisk *di > bdev->bd_invalidated = 0; > for (p = 1; p < disk->minors; p++) > delete_partition(disk, p); > - if (disk->fops->revalidate_disk) > - disk->fops->revalidate_disk(disk); As for your proposed fix, it may be problematic. The ->revalidate method has to be called at least once for a new device, because that's when drivers fetch the capacities. But ->open only calls check_disk_change() for removable devices. Who is going to call ->revalidate inside add_disk() for non-removable devices? -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6.13-rc6-latest] SCSI disk registration msgs repeat themselves
On Tue, Aug 16, 2005 at 11:01:30PM -0400, Chuck Ebbert wrote: > I just added some usb-storage devices to my system and got the below. > Why do the first four lines repeat for each device? (Not sure if > this is a SCSI or USB problem.) It is in the partition code. I posted twice before about this with no response. The changelog said it was a workaround for a borken device, but not what device it was or any other details. There is a patch (against 2.6.11) in the below, I haven't tried it with recent kernels: http://marc.theaimsgroup.com/?l=linux-scsi&m=110719123625593&w=2 http://marc.theaimsgroup.com/?l=linux-scsi&m=110848617107098&w=2 -- Patrick Mansfield - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.6.13-rc6-latest] SCSI disk registration msgs repeat themselves
I just added some usb-storage devices to my system and got the below. Why do the first four lines repeat for each device? (Not sure if this is a SCSI or USB problem.) [ 23.433725] SCSI device sda: 63424 512-byte hdwr sectors (32 MB) [ 23.560564] sda: Write Protect is off [ 23.560581] sda: Mode Sense: 02 00 00 00 [ 23.560593] sda: assuming drive cache: write through [ 23.572525] SCSI device sda: 63424 512-byte hdwr sectors (32 MB) [ 23.576504] sda: Write Protect is off [ 23.576519] sda: Mode Sense: 02 00 00 00 [ 23.576530] sda: assuming drive cache: write through [ 23.576545] sda: sda1 [ 23.583752] Attached scsi removable disk sda at scsi1, channel 0, id 0, lun 0 [ 23.583847] Attached scsi generic sg1 at scsi1, channel 0, id 0, lun 0, type 0 [ 23.584701] usb-storage: device scan complete [ 32.116248] SCSI device sdb: 196608 512-byte hdwr sectors (101 MB) [ 32.141310] sdb: Write Protect is off [ 32.153477] sdb: Mode Sense: 45 00 00 08 [ 32.153490] sdb: assuming drive cache: write through [ 32.178386] SCSI device sdb: 196608 512-byte hdwr sectors (101 MB) [ 32.204473] sdb: Write Protect is off [ 32.216654] sdb: Mode Sense: 45 00 00 08 [ 32.216735] sdb: assuming drive cache: write through [ 32.233259] sdb: sdb4 [ 32.246595] Attached scsi removable disk sdb at scsi2, channel 0, id 0, lun 0 [ 32.270348] Attached scsi generic sg2 at scsi2, channel 0, id 0, lun 0, type 0 [ 32.295843] usb-storage: device scan complete __ Chuck - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/