From mboxrd@z Thu Jan  1 00:00:00 1970
Message-ID: <10e606b8715d8e2c9fda5768466036ca@proxima.alt.za>
To: 9fans@9fans.net
From: Lucio De Re <lucio@proxima.alt.za>
Date: Wed, 17 Nov 2010 06:08:45 +0200
In-Reply-To: <afd7754e0d66d4350bc55a9fcddac3ef@proxima.alt.za>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Subject: Re: [9fans] That deadlock, again
Topicbox-Message-UUID: 835187c4-ead6-11e9-9d60-3106f5b1d025

> Well, here is an acid dump, I'll inspect it in detail, but I'm hoping
> someone will beat me to it (not hard at all, I have to confess):
>
> rumble# acid /sys/src/9/pc/9pccpuf
> /sys/src/9/pc/9pccpuf:386 plan 9 boot image
> /sys/lib/acid/port
> /sys/lib/acid/386
>
[ ... ]

This bit looks suspicious to me, but I'm really not an authority and I
may easily be missing something:

> acid: src(0xf0148c8a)
> /sys/src/9/ip/tcp.c:2096
>  2091		if(waserror()){
>  2092			qunlock(s);
>  2093			nexterror();
>  2094		}
>  2095		qlock(s);
>>2096		qunlock(tcp);
>  2097
>  2098		/* fix up window */
>  2099		seg.wnd <<= tcb->rcv.scale;
>  2100
>  2101		/* every input packet in puts off the keep alive time out */

The source actually says (to be pedantic):

	/* The rest of the input state machine is run with the control block
	 * locked and implements the state machine directly out of the RFC.
	 * Out-of-band data is ignored - it was always a bad idea.
	 */
	tcb = (Tcpctl*)s->ptcl;
	if(waserror()){
		qunlock(s);
		nexterror();
	}
	qlock(s);
	qunlock(tcp);

Now, the qunlock(s) should not precede the qlock(s), this is the first
case in this procedure:

/sys/src/9/ip/tcp.c:1941,2424
void
tcpiput(Proto *tcp, Ipifc*, Block *bp)
{
...
}

and unlocking an unlocked item should not be permitted, right?  If s
(the conversation) is returned locked by either of

/sys/src/9/ip/tcp.c:2048
	s = iphtlook(&tpriv->ht, source, seg.source, dest, seg.dest);

or

/sys/src/9/ip/tcp.c:2081
		s = tcpincoming(s, &seg, source, dest, version);

then the qlock(s) is an issue, but I'm sure that is not the case here.

Finally, locking an item ahead of unlocking another is often cause for
a deadlock, although I understand the possible necessity.

	qlock(s);
	qunlock(tcp);

Even though acid points to a different location, I'd be curious to
have the details above expanded on by somebody familiar with the code.

++L