lkml.org 
[lkml]   [2012]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 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


    \
     
     \ /
      Last update: 2012-02-16 17:39    [W:0.041 / U:0.200 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site