svn commit: r209067 - stable/8/sys/netinet
Michael Tuexen
tuexen at FreeBSD.org
Fri Jun 11 20:38:22 UTC 2010
Author: tuexen
Date: Fri Jun 11 20:38:20 2010
New Revision: 209067
URL: http://svn.freebsd.org/changeset/base/209067
Log:
MFC 209029
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.
Approved by: re (bz)
Modified:
stable/8/sys/netinet/sctp_pcb.c
stable/8/sys/netinet/sctp_usrreq.c
stable/8/sys/netinet/sctputil.c
Directory Properties:
stable/8/sys/ (props changed)
stable/8/sys/amd64/include/xen/ (props changed)
stable/8/sys/cddl/contrib/opensolaris/ (props changed)
stable/8/sys/contrib/dev/acpica/ (props changed)
stable/8/sys/contrib/pf/ (props changed)
stable/8/sys/dev/xen/xenpci/ (props changed)
stable/8/sys/geom/sched/ (props changed)
Modified: stable/8/sys/netinet/sctp_pcb.c
==============================================================================
--- stable/8/sys/netinet/sctp_pcb.c Fri Jun 11 20:08:20 2010 (r209066)
+++ stable/8/sys/netinet/sctp_pcb.c Fri Jun 11 20:38:20 2010 (r209067)
@@ -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: stable/8/sys/netinet/sctp_usrreq.c
==============================================================================
--- stable/8/sys/netinet/sctp_usrreq.c Fri Jun 11 20:08:20 2010 (r209066)
+++ stable/8/sys/netinet/sctp_usrreq.c Fri Jun 11 20:38:20 2010 (r209067)
@@ -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: stable/8/sys/netinet/sctputil.c
==============================================================================
--- stable/8/sys/netinet/sctputil.c Fri Jun 11 20:08:20 2010 (r209066)
+++ stable/8/sys/netinet/sctputil.c Fri Jun 11 20:38:20 2010 (r209067)
@@ -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;
More information about the svn-src-all
mailing list