From owner-svn-src-stable-8@FreeBSD.ORG Thu Sep 6 06:23:39 2012 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AA8DD1065670; Thu, 6 Sep 2012 06:23:39 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-lpp01m010-f54.google.com (mail-lpp01m010-f54.google.com [209.85.215.54]) by mx1.freebsd.org (Postfix) with ESMTP id 2506B8FC08; Thu, 6 Sep 2012 06:23:37 +0000 (UTC) Received: by lage12 with SMTP id e12so1130772lag.13 for ; Wed, 05 Sep 2012 23:23:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=BJeTj0zloVerQkRH19rtY1Kjaj+mMRcetkec8o5UCC0=; b=0bWQLj18see/cLs6XBZNE8ozMyo+Lz2ycbHDgMpsND6QXh0/kbRlfjOJeIy1y60YzJ A6MfOB3gOVgTlmT/QwVg5joGo559Ktr86off0ic6Uke2MJGW3LyNKnV5wcge7F3WPKjY IR33wPcKK12BMnSTN00GkJ/rHFXpN6cOV4sQtyY551hzrScQA/EXBRTWWd2s8B2JXZwp h5QwnEKixvnzktdh97oPTKm00uC8Cztym6mKxyc4Zuzzw+ZJ7/8iBGGGKRIwhUz2/JPb kXyC3HU6AQednI82KHcTxp3nrdZNm4GJhv6tjHcCHp/TX8y8ZZjRpu3akuk1u0fVBD/Z Lhsw== Received: by 10.152.130.3 with SMTP id oa3mr864241lab.27.1346912616720; Wed, 05 Sep 2012 23:23:36 -0700 (PDT) Received: from localhost ([188.230.122.226]) by mx.google.com with ESMTPS id bc2sm296531lbb.3.2012.09.05.23.23.34 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 05 Sep 2012 23:23:35 -0700 (PDT) Date: Thu, 6 Sep 2012 09:23:32 +0300 From: Mikolaj Golub To: Robert Watson Message-ID: <20120906062330.GA94205@gmail.com> References: <201209011033.q81AXsGb094283@svn.freebsd.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="h31gzZEtNLTqOjlF" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, "Bjoern A. Zeeb" , src-committers@freebsd.org, svn-src-stable-8@freebsd.org Subject: Re: svn commit: r239983 - stable/8/sys/netinet X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Sep 2012 06:23:40 -0000 --h31gzZEtNLTqOjlF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Sep 05, 2012 at 07:31:19PM +0100, Robert Watson wrote: > On Wed, 5 Sep 2012, Bjoern A. Zeeb wrote: > > >> Log: > >> MFC r239075: > >> > >> In tcp timers, check INP_DROPPED flag a little later, after > >> callout_deactivate(), so if INP_DROPPED is set we return with the > >> timer active flag cleared. > >> > >> For me this fixes negative keep timer values reported by `netstat -x' > >> for connections in CLOSE state. > > > > panic: Lock tcp not read locked @ /w/src/sys/netinet/tcp_timer.c:497 > > > > reproducable on the cluster. Probably wrong all the way up to HEAD? > > This looks like a mis-merge -- in 8.x, read-locking of the inpcbinfo is not > used in the TCP timer code, whereas in 9.x and later it is. There are > important and subtle differences between inpcb/inpcb/inpcbhash locking across > all supported major FreeBSD versions, as we have substantially refined our > approach to improve performance and stability over the years. As a result, it > is generally not safe to MFC TCP/UDP locking changes without extreme care. I > would recommend seeking at least one, if not multiple, reviewers for changes > and MFCs along these lines. Even with you and others doing line-by-line > reviews of much of the original work, we ran into quite a few bugs due to the > complexity and difficulty in reviewing the code. Thanks! I am going to direct commit this patch to fix the mis-merge. Sorry, will be much more careful with such things next time. -- Mikolaj Golub --h31gzZEtNLTqOjlF Content-Type: text/x-diff; charset=us-ascii Content-Disposition: inline; filename="tcp_timer.c.stable8.diff" Index: stable/8/sys/netinet/tcp_timer.c =================================================================== --- stable/8/sys/netinet/tcp_timer.c (revision 240156) +++ stable/8/sys/netinet/tcp_timer.c (working copy) @@ -494,7 +494,7 @@ callout_deactivate(&tp->t_timers->tt_rexmt); if ((inp->inp_flags & INP_DROPPED) != 0) { INP_WUNLOCK(inp); - INP_INFO_RUNLOCK(&V_tcbinfo); + INP_INFO_WUNLOCK(&V_tcbinfo); CURVNET_RESTORE(); return; } --h31gzZEtNLTqOjlF--