Re: git: c35f527ed115 - main - sx: unset td_wantedlock around going to sleep

  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