On Fri, 13 Dec 2013, James Bottomley wrote:
> This patch eliminates the reap_ref and replaces it with a proper kref.
> On last put of this kref, the target is removed from visibility in
> sysfs. The final call to scsi_target_reap() for the device is done from
> __scsi_remove_device() and only if the device was made visible. This
> ensures that the target disappears as soon as the last device is gone
> rather than waiting until final release of the device (which is often
> too long).
> @@ -474,28 +496,11 @@ static void scsi_target_reap_usercontext(struct
> work_struct *work)
> */
> void scsi_target_reap(struct scsi_target *starget)
> {
> - struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
> - unsigned long flags;
> - enum scsi_target_state state;
> - int empty = 0;
> -
> - spin_lock_irqsave(shost->host_lock, flags);
> - state = starget->state;
> - if (--starget->reap_ref == 0 && list_empty(&starget->devices)) {
> - empty = 1;
> - starget->state = STARGET_DEL;
> - }
> - spin_unlock_irqrestore(shost->host_lock, flags);
> -
> - if (!empty)
> - return;
> -
> - BUG_ON(state == STARGET_DEL);
> - if (state == STARGET_CREATED)
> + BUG_ON(starget->state == STARGET_DEL);
> + if (starget->state == STARGET_CREATED)
> scsi_target_destroy(starget);
> else
> - execute_in_process_context(scsi_target_reap_usercontext,
> - &starget->ew);
> + scsi_target_reap_ref_put(starget);
The refcount test and state change race with scsi_alloc_target().
Maybe the race won't occur in practice, but to be safe you should hold
shost->host_lock throughout that time interval, as the original code
here does.
This means the kref approach won't work so easily. You might as well
leave reap_ref as an ordinary int.
> @@ -393,7 +393,6 @@ static void scsi_device_dev_release_usercontext(struct
> work_struct *work)
> starget = to_scsi_target(parent);
>
> spin_lock_irqsave(sdev->host->host_lock, flags);
> - starget->reap_ref++;
> list_del(&sdev->siblings);
> list_del(&sdev->same_target_siblings);
> list_del(&sdev->starved_entry);
starget is now an unused local variable. It can be eliminated.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
