Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Dec 2012 11:12:11 -0500
From:      Karim Fodil-Lemelin <fodillemlinkarim@gmail.com>
To:        Vijay Singh <vijju.singh@gmail.com>
Cc:        Karim Fodil-Lemelin <fodillemlinkarim@gmail.com>, freebsd-net@freebsd.org, Navdeep Parhar <nparhar@gmail.com>
Subject:   Re: use of V_tcbinfo lock for TCP syncache
Message-ID:  <50D338DB.5070907@gmail.com>
In-Reply-To: <CALCNsJQHCk4m=MqXwK-ejfhD31JKHQxtP8Ycx_FyGsiutfvMdA@mail.gmail.com>
References:  <CALCNsJRFyQ9ZmfJ3quX2-cUuFHjs2rXw63Tq5ZH-uP1%2BoQmjLw@mail.gmail.com> <50D218BA.7080301@FreeBSD.org> <50D21C3B.8020803@gmail.com> <CALCNsJS8j0wvv4dc1Q6rLckmr0RE%2BgW0%2BmUDdmSvyfjXzKugsQ@mail.gmail.com> <50D24774.80800@gmail.com> <CALCNsJQHCk4m=MqXwK-ejfhD31JKHQxtP8Ycx_FyGsiutfvMdA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 19/12/2012 6:40 PM, Vijay Singh wrote:
>> Sure but syncache_expand() is entered with the tcbinfo already write locked
>> which also protects the unlocking of the listening connection and the
>> locking of the newly created socket. Around this part:
>>
>>              /*
>>               * Socket is created in state SYN_RECEIVED.
>>               * Unlock the listen socket, lock the newly
>>               * created socket and update the tp variable.
>>               */
>>              INP_WUNLOCK(inp);    /* listen socket */
>>              inp = sotoinpcb(so);
>>              INP_WLOCK(inp);        /* new connection */
>>              tp = intotcpcb(inp);
>>
>>   Without the tcbinfo lock the new socket could be closed (getting a reset)
>> which would put it in INP_TIMEWAIT or INP_DROPPED _after_ the check is made
>> in tcp_usr_accept since there is a period of time where tcbinfo is not
>> locked and the new socket inp is not locked either.
>>
>> I could be wrong but it seems that without the tcbinfo lock a lot could
>> happen between the unlocking of the listen socket and the locking of the new
> Hopefully Robert will chime in.
>
> I am sorry that I was not clear. In my experiment, syncache_expand()
> is still entered with the V_tcbinfo lock held.
>
> In my (limited) view, sonewconn() is overleaded. It
>
> [1] allocates a new socket
> [2] initializes it using the listener socket
> [3] invokes the pru_attach routine, where the inp is allocated
> [4] it inserts the socket in the listeners queue
> [5] it, optionally, notifies the listener of the new connection
>
> When sonewconn returns, we do a bunch of things, [6] such as call
> in_pcbconnect() and set state in tp etc.
>
> What I am experimenting with is to separate out [4] & [5] from the
> list above, and move those to AFTER we do the inp processing in [6].
> At that point I do not think that the pcbinfo lock should be required
> to be held.
>
>
How do you plan to handle the fact that most of tcp_input() and 
tcp_do_segment() require at least a read lock held on the pcbinfo lock?

Is your goal to reduce the amount of code that gets executed under the 
write lock protection of pcbinfo or completely get rid of the lock all 
together?

Karim.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50D338DB.5070907>