Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 23 Oct 2023 17:32:44 +0000
From:      Gary Jennejohn <garyj@gmx.de>
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:  <20231023193244.148ddcec@ernst.home>
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
On Mon, 23 Oct 2023 17:23:28 GMT
Mateusz Guzik <mjg@FreeBSD.org> wrote:

> The branch main has been updated by mjg:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=3Dc35f527ed115b792463d30c7d=
3c8904e435caead
>
> commit c35f527ed115b792463d30c7d3c8904e435caead
> Author:     Mateusz Guzik <mjg@FreeBSD.org>
> AuthorDate: 2023-10-23 17:22:12 +0000
> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
> CommitDate: 2023-10-23 17:22:12 +0000
>
>     sx: unset td_wantedlock around going to sleep
>
>     Otherwise it can crash in sleepq_wait_sig -> sleepq_catch_signals ->
>     sig_ast_checksusp -> thread_suspend_check due to a mutex acquire.
>
>     Reported by:    pho
>     Sponsored by:   Rubicon Communications, LLC ("Netgate")
> ---
>  sys/kern/kern_sx.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/sys/kern/kern_sx.c b/sys/kern/kern_sx.c
> index bc8a1214689f..455ae165e328 100644
> --- a/sys/kern/kern_sx.c
> +++ b/sys/kern/kern_sx.c
> @@ -854,10 +854,17 @@ retry_sleepq:
>  		sleepq_add(&sx->lock_object, NULL, sx->lock_object.lo_name,
>  		    SLEEPQ_SX | ((opts & SX_INTERRUPTIBLE) ?
>  		    SLEEPQ_INTERRUPTIBLE : 0), SQ_EXCLUSIVE_QUEUE);
> +		/*
> +		 * Hack: this can land in thread_suspend_check which will
> +		 * conditionally take a mutex, tripping over an assert if a
> +		 * lock we are waiting for is set.
> +		 */
> +		THREAD_CONTENTION_DONE(&sx->lock_object);
>  		if (!(opts & SX_INTERRUPTIBLE))
>  			sleepq_wait(&sx->lock_object, 0);
>  		else
>  			error =3D sleepq_wait_sig(&sx->lock_object, 0);
> +		THREAD_CONTENTION_DONE(&sx->lock_object);

		should this be THREAD_CONTENDS_ON_LOCK, like below?

>  #ifdef KDTRACE_HOOKS
>  		sleep_time +=3D lockstat_nsecs(&sx->lock_object);
>  		sleep_cnt++;
> @@ -1209,10 +1216,17 @@ retry_sleepq:
>  		sleepq_add(&sx->lock_object, NULL, sx->lock_object.lo_name,
>  		    SLEEPQ_SX | ((opts & SX_INTERRUPTIBLE) ?
>  		    SLEEPQ_INTERRUPTIBLE : 0), SQ_SHARED_QUEUE);
> +		/*
> +		 * Hack: this can land in thread_suspend_check which will
> +		 * conditionally take a mutex, tripping over an assert if a
> +		 * lock we are waiting for is set.
> +		 */
> +		THREAD_CONTENTION_DONE(&sx->lock_object);
>  		if (!(opts & SX_INTERRUPTIBLE))
>  			sleepq_wait(&sx->lock_object, 0);
>  		else
>  			error =3D sleepq_wait_sig(&sx->lock_object, 0);
> +		THREAD_CONTENDS_ON_LOCK(&sx->lock_object);
>  #ifdef KDTRACE_HOOKS
>  		sleep_time +=3D lockstat_nsecs(&sx->lock_object);
>  		sleep_cnt++;
>


=2D-
Gary Jennejohn



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20231023193244.148ddcec>