On Fri, May 25, 2012 at 9:52 PM, Mike Christie <michaelc@xxxxxxxxxxx> wrote:
> On 05/25/2012 10:34 AM, Dan Williams wrote:
>>> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
>>> index f7565fc..3afb38d 100644
>>> --- a/drivers/scsi/scsi_transport_sas.c
>>> +++ b/drivers/scsi/scsi_transport_sas.c
>>> @@ -1592,7 +1592,7 @@ int sas_rphy_add(struct sas_rphy *rphy)
>>> else
>>> lun = 0;
>>>
>>> - scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id, lun, 0);
>>> + rphy->starget = scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id, lun, 0);
>>> }
>>>
>>> return 0;
>>> @@ -1669,7 +1669,8 @@ sas_rphy_remove(struct sas_rphy *rphy)
>>>
>>> switch (rphy->identify.device_type) {
>>> case SAS_END_DEVICE:
>>> - scsi_remove_target(dev);
>>> + __scsi_remove_target(rphy->starget);
>>> + rphy->starget = NULL;
>>> break;
>
> I do not think it is safe to have the starget pointer on the phy and
> then pass it to scsi_remove_target. If you did
>
> for each device
> echo 1 > /sys/..../remove
>
> then the target would get reaped/freed from under the transport class right?
Where does that 'remove' attribute come from? I can't seem to find it.
In any event I agree it's a bad precedent for the transport class to
start remembering scsi_targets especially without reference counting
them. The problem is really that device_for_each_child() can't help
us in the async remove case. So maybe a brute force "is the passed in
dev an ancestor for a scsi_target (registered or not) on
shost->__targets" is needed. I'll give that a shot.
> It looks like the fc class needs a similar fix though too.
>
> iSCSI is ok since no iscsi driver does scsi_scan_host and does async
> scanning from userspace.
Ok, iscsi can never set shost->async_scan in that case.
--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
