Author: rrs
Date: Fri Jun 11 03:54:00 2010
New Revision: 209029
URL: http://svn.freebsd.org/changeset/base/209029

Log:
  3 Fixes -
  a) There was a case where a ICMP message could cause
     us to return leaving a stuck lock on an stcb.
  b) The iterator needed some tweaks to fix its lock
     ordering.
  c) The ITERATOR_LOCK is no longer needed in the freeing
     of a stcb. Now that the timer based one is gone we don't
     have a multiple resume situation. Add to that that there
     was somewhere a path out of the freeing of an assoc that
     did NOT release the iterator_lock.. it was time to clean
     this old code up and in the process fix the lock bug.
  
  MFC after:    1 week

Modified:
  head/sys/netinet/sctp_pcb.c
  head/sys/netinet/sctp_usrreq.c
  head/sys/netinet/sctputil.c

Modified: head/sys/netinet/sctp_pcb.c
==============================================================================
--- head/sys/netinet/sctp_pcb.c Fri Jun 11 03:13:19 2010        (r209028)
+++ head/sys/netinet/sctp_pcb.c Fri Jun 11 03:54:00 2010        (r209029)
@@ -3075,6 +3075,11 @@ sctp_iterator_inp_being_freed(struct sct
                        } else {
                                it->inp = LIST_NEXT(it->inp, sctp_list);
                        }
+                       /*
+                        * When its put in the refcnt is incremented so decr
+                        * it
+                        */
+                       SCTP_INP_DECR_REF(inp);
                }
                it = nit;
        }
@@ -4428,38 +4433,6 @@ sctp_add_vtag_to_timewait(uint32_t tag, 
 }
 
 
-static void
-sctp_iterator_asoc_being_freed(struct sctp_inpcb *inp, struct sctp_tcb *stcb)
-{
-       struct sctp_iterator *it;
-
-       /*
-        * Unlock the tcb lock we do this so we avoid a dead lock scenario
-        * where the iterator is waiting on the TCB lock and the TCB lock is
-        * waiting on the iterator lock.
-        */
-       it = stcb->asoc.stcb_starting_point_for_iterator;
-       if (it == NULL) {
-               return;
-       }
-       if (it->inp != stcb->sctp_ep) {
-               /* hmm, focused on the wrong one? */
-               return;
-       }
-       if (it->stcb != stcb) {
-               return;
-       }
-       it->stcb = LIST_NEXT(stcb, sctp_tcblist);
-       if (it->stcb == NULL) {
-               /* done with all asoc's in this assoc */
-               if (it->iterator_flags & SCTP_ITERATOR_DO_SINGLE_INP) {
-                       it->inp = NULL;
-               } else {
-                       it->inp = LIST_NEXT(inp, sctp_list);
-               }
-       }
-}
-
 
 /*-
  * Free the association after un-hashing the remote port. This
@@ -4665,7 +4638,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, 
 
                SCTP_TCB_UNLOCK(stcb);
 
-               SCTP_ITERATOR_LOCK();
                SCTP_INP_INFO_WLOCK();
                SCTP_INP_WLOCK(inp);
                SCTP_TCB_LOCK(stcb);
@@ -4704,7 +4676,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, 
         * Make it invalid too, that way if its about to run it will abort
         * and return.
         */
-       sctp_iterator_asoc_being_freed(inp, stcb);
        /* re-increment the lock */
        if (from_inpcbfree == SCTP_NORMAL_PROC) {
                atomic_add_int(&stcb->asoc.refcnt, -1);
@@ -4721,7 +4692,6 @@ sctp_free_assoc(struct sctp_inpcb *inp, 
        if (from_inpcbfree == SCTP_NORMAL_PROC) {
                SCTP_INP_INCR_REF(inp);
                SCTP_INP_WUNLOCK(inp);
-               SCTP_ITERATOR_UNLOCK();
        }
        /* pull from vtag hash */
        LIST_REMOVE(stcb, sctp_asocs);
@@ -6694,20 +6664,22 @@ sctp_initiate_iterator(inp_func inpf,
        it->no_chunk_output = chunk_output_off;
        it->vn = curvnet;
        if (s_inp) {
+               /* Assume lock is held here */
                it->inp = s_inp;
+               SCTP_INP_INCR_REF(it->inp);
                it->iterator_flags = SCTP_ITERATOR_DO_SINGLE_INP;
        } else {
                SCTP_INP_INFO_RLOCK();
                it->inp = LIST_FIRST(&SCTP_BASE_INFO(listhead));
-
+               if (it->inp) {
+                       SCTP_INP_INCR_REF(it->inp);
+               }
                SCTP_INP_INFO_RUNLOCK();
                it->iterator_flags = SCTP_ITERATOR_DO_ALL_INP;
 
        }
        SCTP_IPI_ITERATOR_WQ_LOCK();
-       if (it->inp) {
-               SCTP_INP_INCR_REF(it->inp);
-       }
+
        TAILQ_INSERT_TAIL(&sctp_it_ctl.iteratorhead, it, sctp_nxt_itr);
        if (sctp_it_ctl.iterator_running == 0) {
                sctp_wakeup_iterator();

Modified: head/sys/netinet/sctp_usrreq.c
==============================================================================
--- head/sys/netinet/sctp_usrreq.c      Fri Jun 11 03:13:19 2010        
(r209028)
+++ head/sys/netinet/sctp_usrreq.c      Fri Jun 11 03:54:00 2010        
(r209029)
@@ -402,6 +402,9 @@ sctp_ctlinput(cmd, sa, vip)
                                SCTP_INP_DECR_REF(inp);
                                SCTP_INP_WUNLOCK(inp);
                        }
+                       if (stcb) {
+                               SCTP_TCB_UNLOCK(stcb);
+                       }
                }
        }
        return;

Modified: head/sys/netinet/sctputil.c
==============================================================================
--- head/sys/netinet/sctputil.c Fri Jun 11 03:13:19 2010        (r209028)
+++ head/sys/netinet/sctputil.c Fri Jun 11 03:54:00 2010        (r209029)
@@ -1255,15 +1255,20 @@ sctp_iterator_work(struct sctp_iterator 
 {
        int iteration_count = 0;
        int inp_skip = 0;
+       int first_in = 1;
+       struct sctp_inpcb *tinp;
 
+       SCTP_INP_INFO_RLOCK();
        SCTP_ITERATOR_LOCK();
        if (it->inp) {
+               SCTP_INP_RLOCK(it->inp);
                SCTP_INP_DECR_REF(it->inp);
        }
        if (it->inp == NULL) {
                /* iterator is complete */
 done_with_iterator:
                SCTP_ITERATOR_UNLOCK();
+               SCTP_INP_INFO_RUNLOCK();
                if (it->function_atend != NULL) {
                        (*it->function_atend) (it->pointer, it->val);
                }
@@ -1271,7 +1276,11 @@ done_with_iterator:
                return;
        }
 select_a_new_ep:
-       SCTP_INP_RLOCK(it->inp);
+       if (first_in) {
+               first_in = 0;
+       } else {
+               SCTP_INP_RLOCK(it->inp);
+       }
        while (((it->pcb_flags) &&
            ((it->inp->sctp_flags & it->pcb_flags) != it->pcb_flags)) ||
            ((it->pcb_features) &&
@@ -1281,8 +1290,9 @@ select_a_new_ep:
                        SCTP_INP_RUNLOCK(it->inp);
                        goto done_with_iterator;
                }
-               SCTP_INP_RUNLOCK(it->inp);
+               tinp = it->inp;
                it->inp = LIST_NEXT(it->inp, sctp_list);
+               SCTP_INP_RUNLOCK(tinp);
                if (it->inp == NULL) {
                        goto done_with_iterator;
                }
@@ -1323,6 +1333,8 @@ select_a_new_ep:
                        SCTP_INP_INCR_REF(it->inp);
                        SCTP_INP_RUNLOCK(it->inp);
                        SCTP_ITERATOR_UNLOCK();
+                       SCTP_INP_INFO_RUNLOCK();
+                       SCTP_INP_INFO_RLOCK();
                        SCTP_ITERATOR_LOCK();
                        if (sctp_it_ctl.iterator_flags) {
                                /* We won't be staying here */
@@ -1382,9 +1394,7 @@ no_stcb:
        if (it->iterator_flags & SCTP_ITERATOR_DO_SINGLE_INP) {
                it->inp = NULL;
        } else {
-               SCTP_INP_INFO_RLOCK();
                it->inp = LIST_NEXT(it->inp, sctp_list);
-               SCTP_INP_INFO_RUNLOCK();
        }
        if (it->inp == NULL) {
                goto done_with_iterator;
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to