Date: Mon, 23 Oct 2023 10:28:04 -0700 From: Gleb Smirnoff <glebius@freebsd.org> To: Mateusz Guzik <mjg@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: c35f527ed115 - main - sx: unset td_wantedlock around going to sleep Message-ID: <ZTatJO0Zcj_1R4UU@FreeBSD.org> In-Reply-To: <202310231723.39NHNSn8063744@gitrepo.freebsd.org> References: <202310231723.39NHNSn8063744@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Mateusz, On Mon, Oct 23, 2023 at 05:23:28PM +0000, Mateusz Guzik wrote: M> The branch main has been updated by mjg: M> M> URL: https://cgit.FreeBSD.org/src/commit/?id=c35f527ed115b792463d30c7d3c8904e435caead M> M> commit c35f527ed115b792463d30c7d3c8904e435caead M> Author: Mateusz Guzik <mjg@FreeBSD.org> M> AuthorDate: 2023-10-23 17:22:12 +0000 M> Commit: Mateusz Guzik <mjg@FreeBSD.org> M> CommitDate: 2023-10-23 17:22:12 +0000 M> M> sx: unset td_wantedlock around going to sleep M> M> Otherwise it can crash in sleepq_wait_sig -> sleepq_catch_signals -> M> sig_ast_checksusp -> thread_suspend_check due to a mutex acquire. M> M> Reported by: pho M> Sponsored by: Rubicon Communications, LLC ("Netgate") M> --- M> sys/kern/kern_sx.c | 14 ++++++++++++++ M> 1 file changed, 14 insertions(+) M> M> diff --git a/sys/kern/kern_sx.c b/sys/kern/kern_sx.c M> index bc8a1214689f..455ae165e328 100644 M> --- a/sys/kern/kern_sx.c M> +++ b/sys/kern/kern_sx.c M> @@ -854,10 +854,17 @@ retry_sleepq: M> sleepq_add(&sx->lock_object, NULL, sx->lock_object.lo_name, M> SLEEPQ_SX | ((opts & SX_INTERRUPTIBLE) ? M> SLEEPQ_INTERRUPTIBLE : 0), SQ_EXCLUSIVE_QUEUE); M> + /* M> + * Hack: this can land in thread_suspend_check which will M> + * conditionally take a mutex, tripping over an assert if a M> + * lock we are waiting for is set. M> + */ M> + THREAD_CONTENTION_DONE(&sx->lock_object); M> if (!(opts & SX_INTERRUPTIBLE)) M> sleepq_wait(&sx->lock_object, 0); M> else M> error = sleepq_wait_sig(&sx->lock_object, 0); M> + THREAD_CONTENTION_DONE(&sx->lock_object); Shouldn't the second one be THREAD_CONTENDS_ON_LOCK? M> #ifdef KDTRACE_HOOKS M> sleep_time += lockstat_nsecs(&sx->lock_object); M> sleep_cnt++; M> @@ -1209,10 +1216,17 @@ retry_sleepq: M> sleepq_add(&sx->lock_object, NULL, sx->lock_object.lo_name, M> SLEEPQ_SX | ((opts & SX_INTERRUPTIBLE) ? M> SLEEPQ_INTERRUPTIBLE : 0), SQ_SHARED_QUEUE); M> + /* M> + * Hack: this can land in thread_suspend_check which will M> + * conditionally take a mutex, tripping over an assert if a M> + * lock we are waiting for is set. M> + */ M> + THREAD_CONTENTION_DONE(&sx->lock_object); M> if (!(opts & SX_INTERRUPTIBLE)) M> sleepq_wait(&sx->lock_object, 0); M> else M> error = sleepq_wait_sig(&sx->lock_object, 0); M> + THREAD_CONTENDS_ON_LOCK(&sx->lock_object); M> #ifdef KDTRACE_HOOKS M> sleep_time += lockstat_nsecs(&sx->lock_object); M> sleep_cnt++; -- Gleb Smirnoff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ZTatJO0Zcj_1R4UU>