Hi Ilkka,

I was going through this particular issue reported here, which also seen 
with Oracle Linux. I was curious to see the core for which you have posted 
your below debugging.
Can I get the access to this particular core dump.

Regards,
Ashish

On Tuesday, February 21, 2017 at 11:05:54 PM UTC+5:30, Chris Leech wrote:
>
> There's a rather long standing regression from commit 
> 659743b [SCSI] libiscsi: Reduce locking contention in fast path 
>
> Depending on iSCSI target behavior, it's possible to hit the case in 
> iscsi_complete_task where the task is still on a pending list 
> (!list_empty(&task->running)).  When that happens the task is removed 
> from the list while holding the session back_lock, but other task list 
> modification occur under the frwd_lock.  That leads to linked list 
> corruption and eventually a panicked system. 
>
> Rather than back out the session lock split entirely, in order to try 
> and keep some of the performance gains this patch adds another lock to 
> maintain the task lists integrity. 
>
> Major enterprise supported kernels have been backing out the lock split 
> for while now, thanks to the efforts at IBM where a lab setup has the 
> most reliable reproducer I've seen on this issue.  This patch has been 
> tested there successfully. 
>
> Signed-off-by: Chris Leech <cle...@redhat.com <javascript:>> 
> --- 
>  drivers/scsi/libiscsi.c | 26 +++++++++++++++++++++++++- 
>  include/scsi/libiscsi.h |  1 + 
>  2 files changed, 26 insertions(+), 1 deletion(-) 
>
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c 
> index 834d121..acb5ef3 100644 
> --- a/drivers/scsi/libiscsi.c 
> +++ b/drivers/scsi/libiscsi.c 
> @@ -560,8 +560,12 @@ static void iscsi_complete_task(struct iscsi_task 
> *task, int state) 
>          WARN_ON_ONCE(task->state == ISCSI_TASK_FREE); 
>          task->state = state; 
>   
> -        if (!list_empty(&task->running)) 
> +        spin_lock_bh(&conn->taskqueuelock); 
> +        if (!list_empty(&task->running)) { 
> +                WARN_ONCE(1, "iscsi_complete_task while task on list"); 
>                  list_del_init(&task->running); 
> +        } 
> +        spin_unlock_bh(&conn->taskqueuelock); 
>   
>          if (conn->task == task) 
>                  conn->task = NULL; 
> @@ -783,7 +787,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct 
> iscsi_hdr *hdr, 
>                  if (session->tt->xmit_task(task)) 
>                          goto free_task; 
>          } else { 
> +                spin_lock_bh(&conn->taskqueuelock); 
>                  list_add_tail(&task->running, &conn->mgmtqueue); 
> +                spin_unlock_bh(&conn->taskqueuelock); 
>                  iscsi_conn_queue_work(conn); 
>          } 
>   
> @@ -1474,8 +1480,10 @@ void iscsi_requeue_task(struct iscsi_task *task) 
>           * this may be on the requeue list already if the xmit_task 
> callout 
>           * is handling the r2ts while we are adding new ones 
>           */ 
> +        spin_lock_bh(&conn->taskqueuelock); 
>          if (list_empty(&task->running)) 
>                  list_add_tail(&task->running, &conn->requeue); 
> +        spin_unlock_bh(&conn->taskqueuelock); 
>          iscsi_conn_queue_work(conn); 
>  } 
>  EXPORT_SYMBOL_GPL(iscsi_requeue_task); 
> @@ -1512,22 +1520,26 @@ static int iscsi_data_xmit(struct iscsi_conn 
> *conn) 
>           * only have one nop-out as a ping from us and targets should not 
>           * overflow us with nop-ins 
>           */ 
> +        spin_lock_bh(&conn->taskqueuelock); 
>  check_mgmt: 
>          while (!list_empty(&conn->mgmtqueue)) { 
>                  conn->task = list_entry(conn->mgmtqueue.next, 
>                                           struct iscsi_task, running); 
>                  list_del_init(&conn->task->running); 
> +                spin_unlock_bh(&conn->taskqueuelock); 
>                  if (iscsi_prep_mgmt_task(conn, conn->task)) { 
>                          /* regular RX path uses back_lock */ 
>                          spin_lock_bh(&conn->session->back_lock); 
>                          __iscsi_put_task(conn->task); 
>                          spin_unlock_bh(&conn->session->back_lock); 
>                          conn->task = NULL; 
> +                        spin_lock_bh(&conn->taskqueuelock); 
>                          continue; 
>                  } 
>                  rc = iscsi_xmit_task(conn); 
>                  if (rc) 
>                          goto done; 
> +                spin_lock_bh(&conn->taskqueuelock); 
>          } 
>   
>          /* process pending command queue */ 
> @@ -1535,19 +1547,24 @@ static int iscsi_data_xmit(struct iscsi_conn 
> *conn) 
>                  conn->task = list_entry(conn->cmdqueue.next, struct 
> iscsi_task, 
>                                          running); 
>                  list_del_init(&conn->task->running); 
> +                spin_unlock_bh(&conn->taskqueuelock); 
>                  if (conn->session->state == ISCSI_STATE_LOGGING_OUT) { 
>                          fail_scsi_task(conn->task, DID_IMM_RETRY); 
> +                        spin_lock_bh(&conn->taskqueuelock); 
>                          continue; 
>                  } 
>                  rc = iscsi_prep_scsi_cmd_pdu(conn->task); 
>                  if (rc) { 
>                          if (rc == -ENOMEM || rc == -EACCES) { 
> +                                spin_lock_bh(&conn->taskqueuelock); 
>                                  list_add_tail(&conn->task->running, 
>                                                &conn->cmdqueue); 
>                                  conn->task = NULL; 
> +                                spin_unlock_bh(&conn->taskqueuelock); 
>                                  goto done; 
>                          } else 
>                                  fail_scsi_task(conn->task, DID_ABORT); 
> +                        spin_lock_bh(&conn->taskqueuelock); 
>                          continue; 
>                  } 
>                  rc = iscsi_xmit_task(conn); 
> @@ -1558,6 +1575,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) 
>                   * we need to check the mgmt queue for nops that need to 
>                   * be sent to aviod starvation 
>                   */ 
> +                spin_lock_bh(&conn->taskqueuelock); 
>                  if (!list_empty(&conn->mgmtqueue)) 
>                          goto check_mgmt; 
>          } 
> @@ -1577,12 +1595,15 @@ static int iscsi_data_xmit(struct iscsi_conn 
> *conn) 
>                  conn->task = task; 
>                  list_del_init(&conn->task->running); 
>                  conn->task->state = ISCSI_TASK_RUNNING; 
> +                spin_unlock_bh(&conn->taskqueuelock); 
>                  rc = iscsi_xmit_task(conn); 
>                  if (rc) 
>                          goto done; 
> +                spin_lock_bh(&conn->taskqueuelock); 
>                  if (!list_empty(&conn->mgmtqueue)) 
>                          goto check_mgmt; 
>          } 
> +        spin_unlock_bh(&conn->taskqueuelock); 
>          spin_unlock_bh(&conn->session->frwd_lock); 
>          return -ENODATA; 
>   
> @@ -1738,7 +1759,9 @@ int iscsi_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *sc) 
>                          goto prepd_reject; 
>                  } 
>          } else { 
> +                spin_lock_bh(&conn->taskqueuelock); 
>                  list_add_tail(&task->running, &conn->cmdqueue); 
> +                spin_unlock_bh(&conn->taskqueuelock); 
>                  iscsi_conn_queue_work(conn); 
>          } 
>   
> @@ -2896,6 +2919,7 @@ iscsi_conn_setup(struct iscsi_cls_session 
> *cls_session, int dd_size, 
>          INIT_LIST_HEAD(&conn->mgmtqueue); 
>          INIT_LIST_HEAD(&conn->cmdqueue); 
>          INIT_LIST_HEAD(&conn->requeue); 
> +        spin_lock_init(&conn->taskqueuelock); 
>          INIT_WORK(&conn->xmitwork, iscsi_xmitworker); 
>   
>          /* allocate login_task used for the login/text sequences */ 
> diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h 
> index b0e275d..583875e 100644 
> --- a/include/scsi/libiscsi.h 
> +++ b/include/scsi/libiscsi.h 
> @@ -196,6 +196,7 @@ struct iscsi_conn { 
>          struct iscsi_task        *task;                /* xmit task in 
> progress */ 
>   
>          /* xmit */ 
> +        spinlock_t                taskqueuelock;  /* protects the next 
> three lists */ 
>          struct list_head        mgmtqueue;        /* mgmt (control) xmit 
> queue */ 
>          struct list_head        cmdqueue;        /* data-path cmd queue 
> */ 
>          struct list_head        requeue;        /* tasks needing another 
> run */ 
> -- 
> 2.9.3 
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Reply via email to