Re: list_for_each_entry_safe() regarded as unsafe

[Brian King <brking@us.ibm.com>]

Alan Stern wrote:
> On Thu, 9 Jun 2005, Mike Anderson wrote:
> 
> 
>>Well we need a updated scsi_host state model that would prevent scanning
>>while we are removing the host. I would believe that if the oopses in
>>__scsi_remove_target where prevent there maybe some other oopses showing
>>up as the host started going away.
> 
> 
> More than that is needed -- you have to guarantee that two threads won't 
> try to add or remove a target or device to the same host at the same time.
> 
> 
>>>I don't know what the best way is fix this.  Even if scsi_forget_host() 
>>>acquired the host's scan_mutex, that wouldn't be enough to guarantee the 
>>>__targets and __devices lists won't change, would it?  And it might cause 
>>>interference with other pathways.
>>>
>>
>>Yes if scsi_forget_host acquired the scan_mutex it would deadlock when
>>scsi_remove_device acquired it later on in the call stack.
> 
> 
> How about not acquiring the scan_mutex in scsi_remove_device, and 
> insisting that the caller hold it instead?  There aren't that many places 
> where it gets called.  In fact, one of those places (an error pathway in 
> scsi_sysfs_add_sdev) looks like it already will cause a deadlock.

scsi_remove_device is an exported symbol, so requiring the caller to obtain
the scan_mutex prior to calling it would not work. A __scsi_remove_device
could be created, however, which would not grab the scan_mutex so that scsi
core could do the right thing.


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center