On Fri, 12 Mar 2010, James Bottomley wrote: > > Some time ago I did post a patch adding an new state (STARGET_NEW) in > > order to record whether or not target_alloc had failed. I can't find > > your response in the email archives, but I vaguely recall you saying > > that adding a new state to the state machine wasn't an appropriate > > approach. > > > > What do you think is the best way to handle this? > > Just add documentation ... there's no real need for a new state or a > flag. Okay. I'll put it off for now... in fact, I had more or less reached the conclusion that this patch wasn't worth bothering with any further. The 5/5 patch is more important, though. The race doesn't need to be fixed in the way I did it; another approach would be for scsi_alloc_target() to busy-wait if the old target it finds has not yet gone through the transport_setup_device() and shost->hostt->target_alloc() procedures. Waiting on the scan_mutex seems more straightforward, though. And it looks like there needs to be one more patch in addition. The logic in scsi_alloc_target() is wrong. When deciding whether or not the old target it found is dying, it needs to test reap_ref rather than found_target->state, because the state is changed outside the spinlocked region in scsi_target_reap(). Or the state change could be moved inside the spinlocked region; that would work also. Alan Stern -- 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
