lkml.org 
[lkml]   [2005]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] Fix reference counting for failed SCSI devices
    James Bottomley wrote:
    > On Tue, 2005-05-24 at 11:38 +0200, Hannes Reinecke wrote:
    >>whenever the scsi-ml tries to scan non-existent devices the reference
    >>count in scsi_alloc_sdev() and scsi_probe_and_add_lun() is not adjusted
    >>properly. Every call to XXX_initialize in the driver core sets the
    >>reference count to 1, so for a proper deallocation an explicit XXX_put()
    >>has to be done.
    >
    > That's true, but I don't see what the problem is if the device has never
    > been made visible.
    >
    It's not visible but it's still allocated and referenced. So on doing a
    rmmod these class_devices are being deallocated which crashes as the
    class device is not connected properly.

    >>+ put_device(&starget->dev);
    >
    > this would amount to a double put, since the parent put method is called
    > in the device release.
    >
    Oops. Correct.

    >>+ class_device_put(&sdev->sdev_classdev);
    >
    > This is unnecessary since the class device is simply occupying a private
    > area in the scsi_device. As long as its never made visible to the
    > system, its refcount is irrelevant
    >
    It's not. Whenever you try to rmmod the adapter it becomes highly
    relevant. If it doesn't crash you've at least generated a memleak as the
    class device is never freed.
    (And these are quite a few for Wide-SCSI Double-channel adapters ...)

    >> put_device(&sdev->sdev_gendev);
    >> out:
    >> if (display_failure_msg)
    >>@@ -855,6 +857,8 @@ static int scsi_probe_and_add_lun(struct
    >> if (sdev->host->hostt->slave_destroy)
    >> sdev->host->hostt->slave_destroy(sdev);
    >> transport_destroy_device(&sdev->sdev_gendev);
    >>+ class_device_put(&sdev->sdev_classdev);
    >>+ put_device(sdev->sdev_gendev.parent);
    >
    > same should apply here. As long as this cascade occurs before
    > scsi_add_lun() (which calls scsi_sysfs_add_sdev()), which is what makes
    > the whole set of devices and classes visible.
    >
    Correct for the parent device. The class device has to be deallocated
    properly if a rmmod should work properly.

    New patch attached. Please apply.

    Cheers,

    Hannes
    --
    Dr. Hannes Reinecke hare@suse.de
    SuSE Linux AG S390 & zSeries
    Maxfeldstraße 5 +49 911 74053 688
    90409 Nürnberg http://www.suse.de
    From: Hannes Reinecke <hare@suse.de>
    Subject: Fix refcount for failed devices

    When a non-present device is scanned it is not properly deregistered
    from the driver core. Calling XXX_initialize() functions from the driver
    core sets the reference count to 1, so for proper deallocation a
    XXX_put() has to be issued.

    --- linux-2.6.12-rc4/drivers/scsi/scsi_scan.c.orig 2005-05-24 10:26:46.000000000 +0200
    +++ linux-2.6.12-rc4/drivers/scsi/scsi_scan.c 2005-05-24 10:55:52.000000000 +0200
    @@ -282,6 +282,7 @@ static struct scsi_device *scsi_alloc_sd
    out_device_destroy:
    transport_destroy_device(&sdev->sdev_gendev);
    scsi_free_queue(sdev->request_queue);
    + class_device_put(&sdev->sdev_classdev);
    put_device(&sdev->sdev_gendev);
    out:
    if (display_failure_msg)
    @@ -855,6 +856,7 @@ static int scsi_probe_and_add_lun(struct
    if (sdev->host->hostt->slave_destroy)
    sdev->host->hostt->slave_destroy(sdev);
    transport_destroy_device(&sdev->sdev_gendev);
    + class_device_put(&sdev->sdev_classdev);
    put_device(&sdev->sdev_gendev);
    }
    out:
    \
     
     \ /
      Last update: 2005-05-25 09:07    [W:2.218 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site