[lkml]   [2005]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Subject[PATCH scsi-misc-2.6 00/08] scsi: small fixes & cleanups
     Hello, James.
    Hello, Jens.

    These are series of small fixes & cleanups. The last two patches
    deal with reference counting and hot unplugging oops. Patches are
    against scsi-misc-2.6 tree (this is the devel tree, right?).

    Jens, please try #08 and tell me if you still get oops. AFAICT,
    reference counting isn't the issue here. We're over-counting not
    under-counting (see description of #07).

    All compile cleanly and I haven't found any problem yet. USB
    hot-unplugging doesn't create oops or offline device anymore for me.

    [ Start of patch descriptions ]

    : remove unused bounce-buffer release path

    Buffer bouncing hasn't been done inside the scsi midlayer for
    quite sometime now, but bounce-buffer release paths are still
    around. This patch removes these unused paths.

    : don't use blk_insert_request() for requeueing

    blk_insert_request() has 'reinsert' argument, which, when set,
    turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the
    request. SCSI midlayer was the only user of this feature and
    all requeued requests become special requests defeating
    quiesce state. This patch makes scsi midlayer use
    blk_requeue_request() for requeueing and removes 'reinsert'
    feature from blk_insert_request().

    Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and
    scsi_run_queue() are moved upward unchanged.

    : remove unused scsi_cmnd->internal_timeout field

    scsi_cmnd->internal_timeout field doesn't have any meaning
    anymore. Kill the field.

    : remove meaningless volatile qualifiers from structure definitions

    scsi_device->device_busy, Scsi_Host->host_busy and
    ->host_failed have volatile qualifiers, but the qualifiers
    don't serve any purpose. Kill them. While at it, protect
    ->host_failed update in scsi_error for consistency and clarity.

    : remove a timer race from scsi_queue_insert() and cleanup timer

    scsi_queue_insert() has four callers. Three callers call with
    timer disabled and one (the second invocation in
    scsi_dispatch_cmd()) calls with timer activated.
    scsi_queue_insert() used to always call scsi_delete_timer()
    and ignore the return value. This results in race with timer
    expiration. Remove scsi_delete_timer() call from
    scsi_queue_insert() and make the caller delete timer and check
    the return value.

    While at it, as, once a scsi timer is added, it should expire
    or be deleted before reused, make scsi_add_timer() strict
    about timer reuses. Now timer expiration function clears
    ->function and all timer deletion should go through
    scsi_delete_timer(). Also, remove bogus ->eh_action tests
    from scsi_eh_{done|times_out} functions. The condition is
    always true and the test is somewhat misleading.

    : remove meaningless scsi_cmnd->serial_number_at_timeout field

    scsi_cmnd->serial_number_at_timeout doesn't serve any purpose
    anymore. All serial_number == serial_number_at_timeout tests
    are always true in abort callbacks. Kill the field. Also, as
    ->pid always equals ->serial_number and ->serial_number
    doesn't have any special meaning anymore, update comments
    above ->serial_number accordingly. Once we remove all uses of
    this field from all lldd's, this field should go.

    : remove bogus {get|put}_device() calls

    SCSI request submission paths can be categorized like the

    * through high-level driver (sd, st, sg...)
    + requests (fs / pc)
    + ioctls
    + flushes (issue_flush / barrier rqs)
    + backing dev (unplug fn / field referencing)
    + high-level specific (init / revalidation...)
    * through scsi-midlayer
    + midlevel specific (init...)
    + transport specific (domain validations...)

    All accesses either

    * open high-level driver before submitting requests and
    closes with no request left.
    * get_device() before submitting requests and put_device()
    with no request left.

    So, basically, SCSI high-level object (scsi_disk) and
    mid-level object (scsi_device) are reference counted by users,
    not the requests they submit. Reference count cannot go zero
    with active users and users cannot access the object once the
    reference count reaches zero.

    So, the {get/put}_device() calls in scsi_get_command() and
    scsi_request_fn() are bogus and misleading. In addition,
    get_device() cannot synchronize 1->0 and 0->1 transitions and
    always returns the device pointer given as the argument. The
    == NULL tests are just misleading.

    : fix hot unplug sequence

    When hot-unplugging using scsi_remove_host() function (as usb
    does), scsi_forget_host() used to be called before
    scsi_host_cancel(). So, the device gets removed first without
    request cleanup and scsi_host_cancel() never gets to call
    scsi_device_cancel() on the removed devices. This results in
    premature completion of hot-unplugging process with active
    requests left in queue, eventually leading to hang/offlined
    device or oops when the active command times out.

    This patch makes scsi_remove_host() call scsi_host_cancel()
    first such that the host is first transited into cancel state
    and all requests of all devices are killed, and then, the
    devices are removed. This patch fixes the oops in eh after
    hot-unplugging bug.

    [ End of patch descriptions ]

    Thanks a lot.


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-04-06 13:30    [W:0.026 / U:56.816 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site