> On Apr 25, 2017, at 3:17 PM, Bart Van Assche <Bart.VanAssche@xxxxxxxxxxx> wrote:
>
> On Tue, 2017-04-25 at 21:17 +0000, Song Liu wrote:
>> I am not sure I fully understand the problem here. If I understand the logic
>> correctly, when a device is being removed, it will stay in scsi_host->__devices
>> until fully the remove routine is finished. And LUN scanning in parallel will
>> find the device with scsi_device_lookup_by_target(), and thus it would not
>> rescan the device until the device is fully removed? Did I miss anything here?
>
> Hello Song,
>
> The SCSI core is already complicated enough. Please don't complicate it further
> by making subtle changes to the semantics of scan_mutex. Please also note that
> I have proposed an alternative, namely to make the START STOP UNIT command
> asynchronous.
>
Hello Bart,
I total agree with your concern in making SCSI core more complicated. However, I am
not sure whether how asynchronous START STOP UNIT command makes multiple deletes run
in parallel. If I understand correctly, each device delete has the following steps:
1. lock scan_mutex
2. issue async START STOP UNIT
3. wait START STOP UNIT to complete
4. unlock scan_mutex
scan_mutex will prevent multiple device deletes to run in parallel.
On the other hand, maybe the problem is simpler than I thought? Once we ensure all
slave_destroy() holds proper lock to access host level data, something as follows
might just work. In this way, echoing into delete handle is not protected by
scan_mutex. However, when the sysfs entry exists, the device should be fully added
to the system. And before delete operation finishes, future scan should not re-add
the device (as the device can be found by scsi_device_lookup_by_target).
Am I too optimistic here?
Thanks,
Song
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 82dfe07..d1c3338 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -710,7 +710,7 @@ sdev_store_delete(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
if (device_remove_file_self(dev, attr))
- scsi_remove_device(to_scsi_device(dev));
+ __scsi_remove_device(to_scsi_device(dev));
return count;
};
