Print this page
NEX-20098 idm_refcnt_unref_task() fails to hold mutex before calling REFCNT_AUDIT
Reviewed by: Rob Gittins <rob.gittins@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
NEX-9981 Deadman timer panic from idm_refcnt_wait_ref thread while offlining iSCSI targets
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Reviewed by: Sanjay Nadkarni <sanjay.nadkarni@nexenta.com>
Reviewed by: Rob Gittins <rob.gittins@nexenta.com>
NEX-6018 Return of the walking dead idm_refcnt_wait_ref comstar threads
Reviewed by:  Rick McNeal <rick.mcneal@nexenta.com>
Reviewed by:  Evan Layton <evan.layton@nexenta.com>
        
*** 18,27 ****
--- 18,28 ----
   *
   * CDDL HEADER END
   */
  /*
   * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
+  * Copyright 2019 Nexenta Systems, Inc. All rights reserved.
   */
  
  #include <sys/cpuvar.h>
  #include <sys/conf.h>
  #include <sys/file.h>
*** 59,69 ****
  static int _idm_fini(void);
  static void idm_buf_bind_in_locked(idm_task_t *idt, idm_buf_t *buf);
  static void idm_buf_bind_out_locked(idm_task_t *idt, idm_buf_t *buf);
  static void idm_buf_unbind_in_locked(idm_task_t *idt, idm_buf_t *buf);
  static void idm_buf_unbind_out_locked(idm_task_t *idt, idm_buf_t *buf);
! static void idm_task_abort_one(idm_conn_t *ic, idm_task_t *idt,
      idm_abort_type_t abort_type);
  static void idm_task_aborted(idm_task_t *idt, idm_status_t status);
  static idm_pdu_t *idm_pdu_alloc_common(uint_t hdrlen, uint_t datalen,
      int sleepflag);
  
--- 60,70 ----
  static int _idm_fini(void);
  static void idm_buf_bind_in_locked(idm_task_t *idt, idm_buf_t *buf);
  static void idm_buf_bind_out_locked(idm_task_t *idt, idm_buf_t *buf);
  static void idm_buf_unbind_in_locked(idm_task_t *idt, idm_buf_t *buf);
  static void idm_buf_unbind_out_locked(idm_task_t *idt, idm_buf_t *buf);
! static stmf_status_t idm_task_abort_one(idm_conn_t *ic, idm_task_t *idt,
      idm_abort_type_t abort_type);
  static void idm_task_aborted(idm_task_t *idt, idm_status_t status);
  static idm_pdu_t *idm_pdu_alloc_common(uint_t hdrlen, uint_t datalen,
      int sleepflag);
  
*** 1520,1534 ****
  idm_task_rele(idm_task_t *idt)
  {
          idm_refcnt_rele(&idt->idt_refcnt);
  }
  
! void
  idm_task_abort(idm_conn_t *ic, idm_task_t *idt, idm_abort_type_t abort_type)
  {
          idm_task_t      *task;
          int             idx;
  
          /*
           * Passing NULL as the task indicates that all tasks
           * for this connection should be aborted.
           */
--- 1521,1536 ----
  idm_task_rele(idm_task_t *idt)
  {
          idm_refcnt_rele(&idt->idt_refcnt);
  }
  
! stmf_status_t
  idm_task_abort(idm_conn_t *ic, idm_task_t *idt, idm_abort_type_t abort_type)
  {
          idm_task_t      *task;
          int             idx;
+         stmf_status_t   s = STMF_SUCCESS;
  
          /*
           * Passing NULL as the task indicates that all tasks
           * for this connection should be aborted.
           */
*** 1546,1565 ****
                          mutex_enter(&task->idt_mutex);
                          if ((task->idt_state != TASK_IDLE) &&
                              (task->idt_state != TASK_COMPLETE) &&
                              (task->idt_ic == ic)) {
                                  rw_exit(&idm.idm_taskid_table_lock);
!                                 idm_task_abort_one(ic, task, abort_type);
                                  rw_enter(&idm.idm_taskid_table_lock, RW_READER);
                          } else
                                  mutex_exit(&task->idt_mutex);
                  }
                  rw_exit(&idm.idm_taskid_table_lock);
          } else {
                  mutex_enter(&idt->idt_mutex);
!                 idm_task_abort_one(ic, idt, abort_type);
          }
  }
  
  static void
  idm_task_abort_unref_cb(void *ref)
  {
--- 1548,1568 ----
                          mutex_enter(&task->idt_mutex);
                          if ((task->idt_state != TASK_IDLE) &&
                              (task->idt_state != TASK_COMPLETE) &&
                              (task->idt_ic == ic)) {
                                  rw_exit(&idm.idm_taskid_table_lock);
!                                 s = idm_task_abort_one(ic, task, abort_type);
                                  rw_enter(&idm.idm_taskid_table_lock, RW_READER);
                          } else
                                  mutex_exit(&task->idt_mutex);
                  }
                  rw_exit(&idm.idm_taskid_table_lock);
          } else {
                  mutex_enter(&idt->idt_mutex);
!                 s = idm_task_abort_one(ic, idt, abort_type);
          }
+         return (s);
  }
  
  static void
  idm_task_abort_unref_cb(void *ref)
  {
*** 1586,1598 ****
  
  /*
   * Abort the idm task.
   *    Caller must hold the task mutex, which will be released before return
   */
! static void
  idm_task_abort_one(idm_conn_t *ic, idm_task_t *idt, idm_abort_type_t abort_type)
  {
          /* Caller must hold connection mutex */
          ASSERT(mutex_owned(&idt->idt_mutex));
          switch (idt->idt_state) {
          case TASK_ACTIVE:
                  switch (abort_type) {
--- 1589,1603 ----
  
  /*
   * Abort the idm task.
   *    Caller must hold the task mutex, which will be released before return
   */
! static stmf_status_t
  idm_task_abort_one(idm_conn_t *ic, idm_task_t *idt, idm_abort_type_t abort_type)
  {
+         stmf_status_t   s = STMF_SUCCESS;
+ 
          /* Caller must hold connection mutex */
          ASSERT(mutex_owned(&idt->idt_mutex));
          switch (idt->idt_state) {
          case TASK_ACTIVE:
                  switch (abort_type) {
*** 1607,1617 ****
                           * references are released the callback will call
                           * idm_task_aborted().
                           */
                          idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                              &idm_task_abort_unref_cb);
!                         return;
                  case AT_INTERNAL_ABORT:
                  case AT_TASK_MGMT_ABORT:
                          idt->idt_state = TASK_ABORTING;
                          mutex_exit(&idt->idt_mutex);
                          ic->ic_transport_ops->it_free_task_rsrc(idt);
--- 1612,1622 ----
                           * references are released the callback will call
                           * idm_task_aborted().
                           */
                          idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                              &idm_task_abort_unref_cb);
!                         return (s);
                  case AT_INTERNAL_ABORT:
                  case AT_TASK_MGMT_ABORT:
                          idt->idt_state = TASK_ABORTING;
                          mutex_exit(&idt->idt_mutex);
                          ic->ic_transport_ops->it_free_task_rsrc(idt);
*** 1621,1631 ****
                           * references are released the callback will call
                           * idm_task_aborted().
                           */
                          idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                              &idm_task_abort_unref_cb);
!                         return;
                  default:
                          ASSERT(0);
                  }
                  break;
          case TASK_SUSPENDING:
--- 1626,1636 ----
                           * references are released the callback will call
                           * idm_task_aborted().
                           */
                          idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                              &idm_task_abort_unref_cb);
!                         return (s);
                  default:
                          ASSERT(0);
                  }
                  break;
          case TASK_SUSPENDING:
*** 1661,1671 ****
                           * set the state to TASK_ABORTING instead of
                           * TASK_ABORTED so we can use the same code path.
                           */
                          idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                              &idm_task_abort_unref_cb);
!                         return;
                  default:
                          ASSERT(0);
                  }
                  break;
          case TASK_ABORTING:
--- 1666,1676 ----
                           * set the state to TASK_ABORTING instead of
                           * TASK_ABORTED so we can use the same code path.
                           */
                          idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                              &idm_task_abort_unref_cb);
!                         return (s);
                  default:
                          ASSERT(0);
                  }
                  break;
          case TASK_ABORTING:
*** 1680,1700 ****
                  default:
                          ASSERT(0);
                  }
                  break;
          case TASK_COMPLETE:
!                 /*
!                  * In this case, let it go.  The status has already been
!                  * sent (which may or may not get successfully transmitted)
!                  * and we don't want to end up in a race between completing
!                  * the status PDU and marking the task suspended.
!                  */
                  break;
          default:
                  ASSERT(0);
          }
          mutex_exit(&idt->idt_mutex);
  }
  
  static void
  idm_task_aborted(idm_task_t *idt, idm_status_t status)
  {
--- 1685,1703 ----
                  default:
                          ASSERT(0);
                  }
                  break;
          case TASK_COMPLETE:
!                 idm_refcnt_wait_ref(&idt->idt_refcnt);
!                 s = STMF_ABORT_SUCCESS;
                  break;
          default:
                  ASSERT(0);
          }
          mutex_exit(&idt->idt_mutex);
+ 
+         return (s);
  }
  
  static void
  idm_task_aborted(idm_task_t *idt, idm_status_t status)
  {
*** 2153,2163 ****
--- 2156,2168 ----
  static void
  idm_refcnt_unref_task(void *refcnt_void)
  {
          idm_refcnt_t *refcnt = refcnt_void;
  
+         mutex_enter(&refcnt->ir_mutex);
          REFCNT_AUDIT(refcnt);
+         mutex_exit(&refcnt->ir_mutex);
          (*refcnt->ir_cb)(refcnt->ir_referenced_obj);
  }
  
  void
  idm_refcnt_rele(idm_refcnt_t *refcnt)
*** 2261,2270 ****
--- 2266,2298 ----
                  return;
          }
          mutex_exit(&refcnt->ir_mutex);
  }
  
+ /*
+  * used to determine the status of the refcnt.
+  *
+  * if refcnt is 0 return is 0
+  * if refcnt is negative return is -1
+  * if refcnt > 0 and no waiters return is 1
+  * if refcnt > 0 and waiters return is 2
+  */
+ int
+ idm_refcnt_is_held(idm_refcnt_t *refcnt)
+ {
+         if (refcnt->ir_refcnt < 0)
+                 return (-1);
+ 
+         if (refcnt->ir_refcnt == 0)
+                 return (0);
+ 
+         if (refcnt->ir_waiting == REF_NOWAIT && refcnt->ir_refcnt > 0)
+                 return (1);
+ 
+         return (2);
+ }
+ 
  void
  idm_conn_hold(idm_conn_t *ic)
  {
          idm_refcnt_hold(&ic->ic_refcnt);
  }