Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Feb 2017 10:08:27 +0100
From:      Hans Petter Selasky <hps@selasky.org>
To:        Mateusz Guzik <mjg@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r313859 - in head/sys: kern sys
Message-ID:  <2c01557a-3412-8b46-ef4d-77214566b74a@selasky.org>
In-Reply-To: <201702170645.v1H6j4l6060548@repo.freebsd.org>
References:  <201702170645.v1H6j4l6060548@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 02/17/17 07:45, Mateusz Guzik wrote:
> Author: mjg
> Date: Fri Feb 17 06:45:04 2017
> New Revision: 313859
> URL: https://svnweb.freebsd.org/changeset/base/313859
>
> Log:
>   Introduce SCHEDULER_STOPPED_TD for use when the thread pointer was already read
>
>   Sprinkle in few places.
>
> Modified:
>   head/sys/kern/kern_condvar.c
>   head/sys/kern/kern_synch.c
>   head/sys/sys/systm.h
>
> Modified: head/sys/kern/kern_condvar.c
> ==============================================================================
> --- head/sys/kern/kern_condvar.c	Fri Feb 17 06:22:05 2017	(r313858)
> +++ head/sys/kern/kern_condvar.c	Fri Feb 17 06:45:04 2017	(r313859)
> @@ -122,7 +122,7 @@ _cv_wait(struct cv *cvp, struct lock_obj
>  	    "Waiting on \"%s\"", cvp->cv_description);
>  	class = LOCK_CLASS(lock);
>
> -	if (SCHEDULER_STOPPED())
> +	if (SCHEDULER_STOPPED_TD(td))
>  		return;
>
>  	sleepq_lock(cvp);
> @@ -176,7 +176,7 @@ _cv_wait_unlock(struct cv *cvp, struct l
>  	    ("cv_wait_unlock cannot be used with Giant"));
>  	class = LOCK_CLASS(lock);
>
> -	if (SCHEDULER_STOPPED()) {
> +	if (SCHEDULER_STOPPED_TD(td)) {
>  		class->lc_unlock(lock);
>  		return;
>  	}
> @@ -227,7 +227,7 @@ _cv_wait_sig(struct cv *cvp, struct lock
>  	    "Waiting on \"%s\"", cvp->cv_description);
>  	class = LOCK_CLASS(lock);
>
> -	if (SCHEDULER_STOPPED())
> +	if (SCHEDULER_STOPPED_TD(td))
>  		return (0);
>
>  	sleepq_lock(cvp);
> @@ -287,7 +287,7 @@ _cv_timedwait_sbt(struct cv *cvp, struct
>  	    "Waiting on \"%s\"", cvp->cv_description);
>  	class = LOCK_CLASS(lock);
>
> -	if (SCHEDULER_STOPPED())
> +	if (SCHEDULER_STOPPED_TD(td))
>  		return (0);
>
>  	sleepq_lock(cvp);
> @@ -349,7 +349,7 @@ _cv_timedwait_sig_sbt(struct cv *cvp, st
>  	    "Waiting on \"%s\"", cvp->cv_description);
>  	class = LOCK_CLASS(lock);
>
> -	if (SCHEDULER_STOPPED())
> +	if (SCHEDULER_STOPPED_TD(td))
>  		return (0);
>
>  	sleepq_lock(cvp);
>
> Modified: head/sys/kern/kern_synch.c
> ==============================================================================
> --- head/sys/kern/kern_synch.c	Fri Feb 17 06:22:05 2017	(r313858)
> +++ head/sys/kern/kern_synch.c	Fri Feb 17 06:45:04 2017	(r313859)
> @@ -162,7 +162,7 @@ _sleep(void *ident, struct lock_object *
>  	else
>  		class = NULL;
>
> -	if (SCHEDULER_STOPPED()) {
> +	if (SCHEDULER_STOPPED_TD(td)) {
>  		if (lock != NULL && priority & PDROP)
>  			class->lc_unlock(lock);
>  		return (0);
> @@ -250,7 +250,7 @@ msleep_spin_sbt(void *ident, struct mtx
>  	KASSERT(ident != NULL, ("msleep_spin_sbt: NULL ident"));
>  	KASSERT(TD_IS_RUNNING(td), ("msleep_spin_sbt: curthread not running"));
>
> -	if (SCHEDULER_STOPPED())
> +	if (SCHEDULER_STOPPED_TD(td))
>  		return (0);
>
>  	sleepq_lock(ident);
> @@ -411,7 +411,7 @@ mi_switch(int flags, struct thread *newt
>  	 */
>  	if (kdb_active)
>  		kdb_switch();
> -	if (SCHEDULER_STOPPED())
> +	if (SCHEDULER_STOPPED_TD(td))
>  		return;
>  	if (flags & SW_VOL) {
>  		td->td_ru.ru_nvcsw++;
>
> Modified: head/sys/sys/systm.h
> ==============================================================================
> --- head/sys/sys/systm.h	Fri Feb 17 06:22:05 2017	(r313858)
> +++ head/sys/sys/systm.h	Fri Feb 17 06:45:04 2017	(r313859)
> @@ -128,7 +128,11 @@ void	kassert_panic(const char *fmt, ...)
>   * Otherwise, the kernel will deadlock since the scheduler isn't
>   * going to run the thread that holds any lock we need.
>   */
> -#define	SCHEDULER_STOPPED() __predict_false(curthread->td_stopsched)
> +#define	SCHEDULER_STOPPED_TD(td)  ({					\
> +	MPASS((td) == curthread);					\
> +	__predict_false((td)->td_stopsched);				\
> +})
> +#define	SCHEDULER_STOPPED() SCHEDULER_STOPPED_TD(curthread)
>
>  /*
>   * Align variables.
>
>

Hi,

Do these checks also cover panics? Or only shutdown?

Should you also add a check for kdb_active // panicstr ? Then I could 
factor out some checks I've sprinkled in the USB code.

--HPS



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2c01557a-3412-8b46-ef4d-77214566b74a>