Re: [PATCH] Separate target visibility from reaped state information

[Bart Van Assche <bart.vanassche@sandisk.com>]

On 01/19/16 17:03, James Bottomley wrote:
> On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote:
>>>>>>> "Bart" == Bart Van Assche <bart.vanassche@sandisk.com>
>>>>>>> writes:
>>
>> Bart> Instead of representing the states "visible in sysfs" and "has
>> Bart> been removed from the target list" by a single state variable,
>> use
>> Bart> two variables to represent this information.
>>
>> James: Are you happy with the latest iteration of this? Should I
>> queue
>> it?
>
> Well, I'm OK with the patch: it's a simple transformation of the
> enumerated state to a two bit state.  What I can't see is how it fixes
> any soft lockup.
>
> The only change from the current workflow is that the DEL transition
> (now the reaped flag) is done before the spin lock is dropped which
> would fix a tiny window for two threads both trying to remove the same
> target, but there's nothing that could possibly fix an iterative soft
> lockup caused by restarting the loop, which is what the changelog says.

Hello James,

scsi_remove_target() doesn't lock the scan_mutex which means that 
concurrent SCSI scanning activity is not prohibited. Such scanning 
activity can postpone the transition of the state of a SCSI target into 
STARGET_DEL. I think if the scheduler decides to run the thread that 
executes scsi_remove_target() on the same CPU as the scanning code after 
the scanning code has obtained a reap ref and before the scanning code 
has released the reap ref again that the soft lockup can be triggered 
that has been reported by Sebastian Herbszt.

Bart.