Re: [PATCH] bug fix: SCSI async scan sysfs -EEXIST problem

[James Smart <James.Smart@Emulex.Com>]

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