Date: Thu, 22 Dec 2016 10:39:12 -0800 From: John Baldwin <jhb@freebsd.org> To: Mark Johnston <markj@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r310423 - head/sys/kern Message-ID: <6562460.a4qdZuDa0s@ralph.baldwin.cx> In-Reply-To: <201612221751.uBMHpim4062786@repo.freebsd.org> References: <201612221751.uBMHpim4062786@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, December 22, 2016 05:51:44 PM Mark Johnston wrote: > Author: markj > Date: Thu Dec 22 17:51:44 2016 > New Revision: 310423 > URL: https://svnweb.freebsd.org/changeset/base/310423 > > Log: > Revert part of r300109. > > The removal of TAILQ_FOREACH_SAFE introduced a small race: when the last > thread on a sleepqueue is awoken, it reclaims the sleepqueue and may begin > executing on a different CPU before sleepq_resume_thread() returns. This > leaves a window during which it may go back to sleep and incorrectly be > awoken again by the caller of sleepq_broadcast(). This is very subtle. The issue is that the last sleepq_resume_thread transfers ownership of 'sq' from the wait channel that the sleepq_broadcast has locked, to the thread being resumed. I thought about using a local TAILQ_HEAD and using TAILQ_CONCAT to move the list of threads out of the sleep queue and then walking that list. However, a comment explaining this transfer of ownership (and that we can't safely access 'sq' after the last thread is resumed) is probably sufficient (but necessary I think). Do you feel like adding one? -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6562460.a4qdZuDa0s>