Date: Thu, 16 Mar 2017 11:32:38 +0900 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 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?00566E96-2330-4354-9095-6D8F77C3AB26>