Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Jan 2018 15:24:42 -0500
From:      Mark Johnston <markj@FreeBSD.org>
To:        Warner Losh <imp@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r326964 - head/sys/cam
Message-ID:  <20180110202442.GC78524@raichu>
In-Reply-To: <201712190413.vBJ4DMY9090982@repo.freebsd.org>
References:  <201712190413.vBJ4DMY9090982@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Dec 19, 2017 at 04:13:22AM +0000, Warner Losh wrote:
> Author: imp
> Date: Tue Dec 19 04:13:22 2017
> New Revision: 326964
> URL: https://svnweb.freebsd.org/changeset/base/326964
> 
> Log:
>   When doing a dump, the scheduler is normally not running, so this
>   changed worked to capture dumps for me. However, the test for
>   SCHEDULER_STOPPED() isn't right. We can also call the dump routine
>   from ddb, in which case the scheduler is still running. This leads to
>   an assertion panic that we're sleeping when we shouldn't. Instead, use
>   the proper test for dumping or not. This brings us in line with other
>   places that do special things while we're doing polled I/O like this.
>   
>   Noticed by: pho@
>   Differential Revision: https://reviews.freebsd.org/D13531
> 
> Modified:
>   head/sys/cam/cam_periph.c
> 
> Modified: head/sys/cam/cam_periph.c
> ==============================================================================
> --- head/sys/cam/cam_periph.c	Tue Dec 19 04:06:07 2017	(r326963)
> +++ head/sys/cam/cam_periph.c	Tue Dec 19 04:13:22 2017	(r326964)
> @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$");
>  #include <sys/malloc.h>
>  #include <sys/kernel.h>
>  #include <sys/bio.h>
> +#include <sys/conf.h>
>  #include <sys/lock.h>
>  #include <sys/mutex.h>
>  #include <sys/buf.h>
> @@ -1158,7 +1159,7 @@ cam_periph_runccb(union ccb *ccb,
>  	struct bintime *starttime;
>  	struct bintime ltime;
>  	int error;
> -	bool sched_stopped;
> +	bool must_poll;
>  	struct mtx *periph_mtx;
>  	struct cam_periph *periph;
>  	uint32_t timeout = 1;
> @@ -1182,7 +1183,13 @@ cam_periph_runccb(union ccb *ccb,
>  		devstat_start_transaction(ds, starttime);
>  	}
>  
> -	sched_stopped = SCHEDULER_STOPPED();
> +	/*
> +	 * We must poll the I/O while we're dumping. The scheduler is normally
> +	 * stopped for dumping, except when we call doadump from ddb. While the
> +	 * scheduler is running in this case, we still need to poll the I/O to
> +	 * avoid sleeping waiting for the ccb to complete.
> +	 */
> +	must_poll = dumping;

Hmm, unfortunately this introduces a new problem: after a panic we call
adashutdown() as part of the shutdown_post_sync event. This occurs
before we call doadump(), so dumping == 0 and we don't poll for the
spindown command's completion. But since we've panicked, the scheduler
is stopped, and so we just hang.



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