On Tue, 2016-02-02 at 10:29 -0800, Bart Van Assche wrote:
> On 02/02/2016 03:46 AM, James Bottomley wrote:
> > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> > index 4f18a85..00bc721 100644
> > --- a/drivers/scsi/scsi_sysfs.c
> > +++ b/drivers/scsi/scsi_sysfs.c
> > @@ -1272,16 +1272,18 @@ static void __scsi_remove_target(struct
> > scsi_target *starget)
> > void scsi_remove_target(struct device *dev)
> > {
> > struct Scsi_Host *shost = dev_to_shost(dev->parent);
> > - struct scsi_target *starget;
> > + struct scsi_target *starget, *last_target = NULL;
> > unsigned long flags;
> >
> > restart:
> > spin_lock_irqsave(shost->host_lock, flags);
> > list_for_each_entry(starget, &shost->__targets, siblings) {
> > - if (starget->state == STARGET_DEL)
> > + if (starget->state == STARGET_DEL ||
> > + starget == last_target)
> > continue;
> > if (starget->dev.parent == dev || &starget->dev ==
> > dev) {
> > kref_get(&starget->reap_ref);
> > + last_target = starget;
> > spin_unlock_irqrestore(shost->host_lock,
> > flags);
> > __scsi_remove_target(starget);
> > scsi_target_reap(starget);
>
> Hello James,
>
> Do you think it is a robust approach to store the pointer to the last
> removed target in the last_target variable ?
Well, yes, I think it will work, if that's what you mean.
> What if e.g. scsi_target_reap() frees the memory the last_target
> pointer points at and another thread reallocates a scsi_target data
> structure ? Can that last data structure have the same address as the
> contents of the last_target variable ?
Yes, but it doesn't matter, does it? Add/Remove has always (and will
always) be racy. Under current conditions you can still add to the
target after the list_for_each terminates and have scsi_remove_target()
return with attached devices. The only way to close the race is
basically to forbid scanning as we shut down the host and wait for all
in-progress scans before starting the final removals.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
