James Bottomley wrote:
On Mon, 2007-05-07 at 14:57 -0400, James Smart wrote:+ mutex_lock(&shost->scan_mutex); scsi_sysfs_add_devices(shost); + atomic_set(&shost->async_scan, 0); + mutex_unlock(&shost->scan_mutex);It really seems that the only safety here is expanding the scan mutex to cover more of the code. I don't really see any value to using the atomic types ... it's not a simultaneous variable update problem, it's a scan race which the mutex can be used to mediate. James
True. First requirement is that scsi_sysfs_add_devices() must be under the scan_mutex. Additionally, (ignoring the atomic) shost->async_scan, based on its use in scsi_add_lun(), should be changed under the mutex as well, thus the "proper" placement in the snippet above is while under the scan mutex. The real headache was the other code paths that read the value of async_scan and determine whether to enumerate or ignore the sdev. Reorganizing that code to place it under scan_mutex was very ugly. Converting to an atomic simply addressed the synchronicity on sampling, although it essentially makes it independent of the scan_mutex. Are you suggesting you would rather rework all the other code paths for scan_mutex encapsulating async_scan ? -- james - 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
