On Mon, Nov 16, 2015 at 7:30 PM, Michael Christie <michaelc@xxxxxxxxxxx> wrote: >> On Nov 15, 2015, at 4:10 AM, Or Gerlitz <gerlitz.or@xxxxxxxxx> wrote: >> After the locking change, adding a task to any of the connection >> mgmtqueue, cmdqueue, or requeue lists is under the session forward lock. >> >> Removing tasks from any of these lists in iscsi_data_xmit is under >> the session forward lock and **before** calling down to the transport >> to handle the task. >> >> The iscsi_complete_task helper was added by Mike's commit >> 3bbaaad95fd38ded "[SCSI] libiscsi: handle cleanup task races" >> and is indeed typically called under the backward lock && has this section >> >> + if (!list_empty(&task->running)) >> + list_del_init(&task->running); >> which per my reading of the code never comes into play, can you comment? >> The bug occurs when a target completes a command while we are still >> processing it. If we are doing a WRITE and the iscsi_task >> is on the cmdqueue because we are handling a R2T. The target shouldn't >> send a Check Condition at this time, but some do. If that happens, then >> iscsi_queuecommand could be adding a new task to the cmdqueue, while the >> recv path is handling the CC for the task with the outsanding R2T. The >> recv path iscsi_complete_task call sees that task it on the cmdqueue and >> deletes it from the list at the same time iscsi_queuecommand is adding a new >> task. So we're now a bit beyond trivial bug and >> This should not happen per the iscsi spec. There is some wording about >> waiting to finish the sequence in progress, but targets goof this up. we have target/s that violate the spec, this is life, but can explain why it took us 18m to get bug report. Can you provide few point pointers into the relevant code pieces that one need to look at to realize what's going on there? was this code added before or after the patch? Or. -- 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
