Messages in this thread | | | Date | Thu, 16 Feb 2012 08:36:30 -0800 | From | Tejun Heo <> | Subject | Re: Kernel crashing on eject SD card |
| |
Hello,
On Thu, Feb 16, 2012 at 10:26:38AM +0900, Jun'ichi Nomura wrote: > >> +int invalidate_partitions(struct gendisk *disk, struct block_device *bdev) > >> +{ > >> + int res; > >> + > >> + res = drop_partitions(disk, bdev); > >> + if (res) > >> + return res; > >> + > > > > Hmmm... shouldn't we have set_capacity(disk, 0) here? > > Added. > I wasn't sure whether I should leave it to drivers.
The problem is that we shouldn't call into drivers without first opening the device, so....
> But it seems capacity 0 for ENOMEDIUM device is reasonable.
Yeah, I *think* it should be okay.
> >> + check_disk_size_change(disk, bdev); > >> + bdev->bd_invalidated = 0; > >> + /* tell userspace that the media / partition table may have changed */ > >> + kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE); > > > > Also, we really shouldn't be generating KOBJ_CHANGE after every > > -ENOMEDIUM open. This can easily lead to infinite loop. We should > > generate this iff we actually dropped partitions && modified the size. > > invalidate_partitions() is called only when bd_invalidated is set. > So KOBJ_CHANGE is not raised for every ENOMEDIUM open.
Ah, okay.
> I put it explicit in the function to make it safer for > possible misuse. > > How about this? > > --------------------------------------------------------- > Do not call drivers when invalidating partitions for -ENOMEDIUM > > When a scsi driver returns -ENOMEDIUM for open(), > __blkdev_get() calls rescan_partitions(), which ends up calling > sd_revalidate_disk() without getting a refcount of scsi_device. > > That could lead to oops like this: > > process A process B > ---------------------------------------------- > sys_open > __blkdev_get > sd_open > returns -ENOMEDIUM > scsi_remove_device > <scsi_device torn down> > rescan_partitions > sd_revalidate_disk > <oops> > > Oopses are reported here: > http://marc.info/?l=linux-scsi&m=132388619710052 > > This patch separates the partition invalidation from rescan_partitions() > and use it for -ENOMEDIUM case.
Yeah, this looks good to me.
Thank you.
-- tejun
| |