Hi Slawa, On 10/11/16 2:11 PM, Slawa Olhovchenkov wrote: > On Tue, Oct 11, 2016 at 09:20:17AM +0200, Julien Charbon wrote: >> Then threads are competing for the INP_WLOCK lock. For the example, >> let's say the thread A wants to run tcp_input()/in_pcblookup_mbuf() and >> racing for this INP_WLOCK: >> >> https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/in_pcb.c#L1964 >> >> And thread B wants to run tcp_timer_2msl()/tcp_close()/in_pcbdrop() and >> racing for this INP_WLOCK: >> >> https://github.com/freebsd/freebsd/blob/release/11.0.0/sys/netinet/tcp_timer.c#L323 >> >> That leads to two cases: >> >> o Thread A wins the race: >> >> Thread A will continue tcp_input() as usal and INP_DROPPED flags is >> not set and inp is still in TCP hash table. >> Thread B is waiting on thread A to release INP_WLOCK after finishing >> tcp_input() processing, and thread B will continue >> tcp_timer_2msl()/tcp_close()/in_pcbdrop() processing. >> >> o Thread B wins the race: >> >> Thread B runs tcp_timer_2msl()/tcp_close()/in_pcbdrop() and inp >> INP_DROPPED is set and inp being removed from TCP hash table. >> In parallel, thread A has found the inp in TCP hash before is was >> removed, and waiting on the found inp INP_WLOCK lock. >> Once thread B has released the INP_WLOCK lock, thread A gets this lock >> and sees the INP_DROPPED flag and do "goto findpcb" but here because the >> inp is not more in TCP hash table and it will not be find again by >> in_pcblookup_mbuf(). >> >> Hopefully I am clear enough here. > > Thanks, very clear. > Small qeustion: when both thread run on same CPU core, INP_WLOCK will > be re-schedule?
Hmm, a thread can re-scheduled but not a lock. Thus no sure I understand your question here. :) > As I remeber race created by call tcp_twstart() at time of end > tcp_close(), at path sofree()-tcp_usr_detach() and unexpected > INP_TIMEWAIT state in the tcp_usr_detach(). INP_TIMEWAIT set in tcp_twstart() Exactly, thus the current fix is: If you already have the INP_DROPPED flag set you are not allowed to call tcp_twstart(), actually it is a good candidate for a new INVARIANT. Let me add that. > After check source code I am found invocation of tcp_twstart() in > sys/netinet/tcp_stacks/fastpath.c, sys/netinet/tcp_input.c, > sys/dev/cxgb/ulp/tom/cxgb_cpl_io.c, sys/dev/cxgbe/tom/t4_cpl_io.c. > > Invocation from sys/netinet/tcp_stacks/fastpath.c and > sys/netinet/tcp_input.c guarded by INP_WLOCK in tcp_input(), and now > will be OK. > > Invocation from sys/dev/cxgb/ulp/tom/cxgb_cpl_io.c and > sys/dev/cxgbe/tom/t4_cpl_io.c is not clear to me, I am see independed > INP_WLOCK. Is this OK? > > Can be thread A wants do_peer_close() directed from chelsio IRQ > handler, bypass tcp_input()? If you look carefully INP_WLOCK is used in cxgb_cpl_io.c and t4_cpl_io.c before calling tcp_twstart(). -- Julien
signature.asc
Description: OpenPGP digital signature
