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,10 +18,11 @@
  *
  * 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,11 +60,11 @@
 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,
+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,15 +1521,16 @@
 idm_task_rele(idm_task_t *idt)
 {
         idm_refcnt_rele(&idt->idt_refcnt);
 }
 
-void
+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,20 +1548,21 @@
                         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);
+                                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);
-                idm_task_abort_one(ic, idt, abort_type);
+                s = idm_task_abort_one(ic, idt, abort_type);
         }
+        return (s);
 }
 
 static void
 idm_task_abort_unref_cb(void *ref)
 {

@@ -1586,13 +1589,15 @@
 
 /*
  * Abort the idm task.
  *    Caller must hold the task mutex, which will be released before return
  */
-static void
+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,11 +1612,11 @@
                          * references are released the callback will call
                          * idm_task_aborted().
                          */
                         idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                             &idm_task_abort_unref_cb);
-                        return;
+                        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,11 +1626,11 @@
                          * references are released the callback will call
                          * idm_task_aborted().
                          */
                         idm_refcnt_async_wait_ref(&idt->idt_refcnt,
                             &idm_task_abort_unref_cb);
-                        return;
+                        return (s);
                 default:
                         ASSERT(0);
                 }
                 break;
         case TASK_SUSPENDING:

@@ -1661,11 +1666,11 @@
                          * 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;
+                        return (s);
                 default:
                         ASSERT(0);
                 }
                 break;
         case TASK_ABORTING:

@@ -1680,21 +1685,19 @@
                 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.
-                 */
+                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,11 +2156,13 @@
 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,10 +2266,33 @@
                 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);
 }