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: >> On Fri, Nov 13, 2015 at 6:51 PM, Mike Christie <michaelc@xxxxxxxxxxx> wrote: >>> On 11/13/2015 09:06 AM, Or Gerlitz 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? > I had sent this to Sagi and your mellanox email the other day: > 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. Mike, I failed to find how an iscsi_task can be added again to the cmdqueue list, nor how it can be added to the requeue list without the right locking, nor how can an iscsi_task be on either of these lists when iscsi_complete_task is invoked. Specifically, my reading of the code says that these are the events over time: t1. queuecommand :: we put the task on the cmdqueue list (libiscsi.c:1741) - under fwd lock t2. iscsi_data_xmit :: we remove the task from the cmdqueue list (libiscsi.c:1537) - under fwd lock when the R2T flow happens, we do the following t3. iscsi_tcp_hdr_dissect --> iscsi_tcp_r2t_rsp --> iscsi_requeue_task :: put the task on the requeue list -- the call to iscsi_tcp_r2t_rsp is under the fwd lock t4. iscsi_data_xmit :: we remove the task from the requeue list (libiscsi.c: 1578) - under fwd lock Do you agree to t1...t4 being what's possible for a given task? or I missed something? >> 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. >> 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. -- 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
