From owner-svn-src-all@freebsd.org  Thu Mar 16 02:32:48 2017
Return-Path: <owner-svn-src-all@freebsd.org>
Delivered-To: svn-src-all@mailman.ysv.freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org
 [IPv6:2001:1900:2254:206a::19:1])
 by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4C215D0E82B;
 Thu, 16 Mar 2017 02:32:48 +0000 (UTC) (envelope-from kp@FreeBSD.org)
Received: from venus.codepro.be (venus.codepro.be [IPv6:2a01:4f8:162:1127::2])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits))
 (Client CN "*.codepro.be", Issuer "Gandi Standard SSL CA 2" (verified OK))
 by mx1.freebsd.org (Postfix) with ESMTPS id 170531646;
 Thu, 16 Mar 2017 02:32:48 +0000 (UTC) (envelope-from kp@FreeBSD.org)
Received: from [192.168.8.218] (203.141.139.231.static.zoot.jp
 [203.141.139.231]) (Authenticated sender: kp)
 by venus.codepro.be (Postfix) with ESMTPSA id C87B61E0F6;
 Thu, 16 Mar 2017 03:32:43 +0100 (CET)
From: "Kristof Provost" <kp@FreeBSD.org>
To: "John Baldwin" <jhb@freebsd.org>
Cc: "Gleb Smirnoff" <glebius@freebsd.org>, src-committers@freebsd.org,
 svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject: Re: svn commit: r315136 - head/sys/netpfil/pf
Date: Thu, 16 Mar 2017 11:32:38 +0900
Message-ID: <00566E96-2330-4354-9095-6D8F77C3AB26@FreeBSD.org>
In-Reply-To: <1803226.Igex2bR0P8@ralph.baldwin.cx>
References: <201703120542.v2C5gvM4075391@repo.freebsd.org>
 <20170314215706.GB1072@FreeBSD.org>
 <7B1C8879-E636-4315-99A2-A258AB9AE500@FreeBSD.org>
 <1803226.Igex2bR0P8@ralph.baldwin.cx>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
X-Mailer: MailMate (2.0BETAr6080)
X-BeenThere: svn-src-all@freebsd.org
X-Mailman-Version: 2.1.23
Precedence: list
List-Id: "SVN commit messages for the entire src tree \(except for &quot;
 user&quot; and &quot; projects&quot; \)" <svn-src-all.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>
List-Post: <mailto:svn-src-all@freebsd.org>
List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-all>,
 <mailto:svn-src-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 16 Mar 2017 02:32:48 -0000

On 15 Mar 2017, at 15:45, John Baldwin wrote:
> You are ignoring interrupts and preemption.  Suppose you get an 
> interrupt
> after 'wakeup_one(pf_purge_thread)' and before 'tsleep(..., 0)' in
> pf_unload().  If the interrupt preempts and results in the purge 
> thread
> running and issuing its wakeup before the thread executing pf_unload()
> resumes, then eventually when pf_unload() resumes it will do a 
> tsleep() with
> no timeout that will never be awoken.
>
Thank you for the thorough explanation. I’ll work on a fix and post it 
for
review as soon as I can.

> You obviously didn't test this in a debug kernel since there is a 
> KASSERT
> explicitly to catch obvious tsleep races in _sleep():
>
>         KASSERT(sbt != 0 || mtx_owned(&Giant) || lock != NULL,
>             ("sleeping without a lock"));
>
I’m sure I did test this with both INVARIANTS and WITNESS enabled.
Is Giant held during module load/unload?

Regards,
Kristof