Date: Mon, 18 Dec 2017 08:03:08 -0700 From: Warner Losh <imp@bsdimp.com> To: Mark Johnston <markj@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, Peter Holm <pho@freebsd.org> Subject: Re: svn commit: r326643 - head/sys/cam Message-ID: <CANCZdfrOROVz1W2jQ%2BfsPFwPPXJHE1XqocsfaKN6AgYVO3wSjA@mail.gmail.com> In-Reply-To: <20171218145914.GD4235@raichu> References: <201712062305.vB6N57XP065402@repo.freebsd.org> <20171218145914.GD4235@raichu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Dec 18, 2017 at 7:59 AM, Mark Johnston <markj@freebsd.org> wrote: > On Wed, Dec 06, 2017 at 11:05:07PM +0000, Warner Losh wrote: > > Author: imp > > Date: Wed Dec 6 23:05:07 2017 > > New Revision: 326643 > > URL: https://svnweb.freebsd.org/changeset/base/326643 > > > > Log: > > Make cam_periph_runccb be safe to call when we can only do polling. > > > > Sponsored by: Netflix > > Differential Revision: https://reviews.freebsd.org/D13388 > > > > Modified: > > head/sys/cam/cam_periph.c > > head/sys/cam/cam_xpt.c > > head/sys/cam/cam_xpt.h > > > > Modified: head/sys/cam/cam_periph.c > > ============================================================ > ================== > > --- head/sys/cam/cam_periph.c Wed Dec 6 23:03:34 2017 (r326642) > > +++ head/sys/cam/cam_periph.c Wed Dec 6 23:05:07 2017 (r326643) > > @@ -1160,7 +1160,11 @@ cam_periph_runccb(union ccb *ccb, > > struct bintime *starttime; > > struct bintime ltime; > > int error; > > - > > + bool sched_stopped; > > + struct mtx *periph_mtx; > > + struct cam_periph *periph; > > + uint32_t timeout = 1; > > + > > starttime = NULL; > > xpt_path_assert(ccb->ccb_h.path, MA_OWNED); > > KASSERT((ccb->ccb_h.flags & CAM_UNLOCKED) == 0, > > @@ -1180,21 +1184,47 @@ cam_periph_runccb(union ccb *ccb, > > devstat_start_transaction(ds, starttime); > > } > > > > + sched_stopped = SCHEDULER_STOPPED(); > > It looks like this regresses DDB's "dump" command: while > SCHEDULER_STOPPED() will be true after a panic, it is not true after > breaking into DDB from the console. pho@ reported the following > issue: > > db:0:allt> call doadump > Dumping 2234 out of 65426 MB:panic: sleepq_add: td 0xfffff80003a48000 to > sleep on wchan 0xfffffe0000b36ce8 with sleeping prohibited > cpuid = 18 > time = 1513582125 > KDB: stack backtrace: > db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame > 0xfffffe0000b36940 > vpanic() at vpanic+0x19c/frame 0xfffffe0000b369c0 > kassert_panic() at kassert_panic+0x126/frame 0xfffffe0000b36a30 > sleepq_add() at sleepq_add+0x34d/frame 0xfffffe0000b36a80 > _sleep() at _sleep+0x26c/frame 0xfffffe0000b36b20 > cam_periph_runccb() at cam_periph_runccb+0x17d/frame 0xfffffe0000b36c80 > dadump() at dadump+0x12a/frame 0xfffffe0000b36ef0 > dump_append() at dump_append+0xa5/frame 0xfffffe0000b36f10 > blk_write() at blk_write+0x28b/frame 0xfffffe0000b36f50 > minidumpsys() at minidumpsys+0x959/frame 0xfffffe0000b37010 > ... > > Wouldn't it be more correct to predicate on "dumping" rather than > SCHEDULER_STOPPED()? > I debated between a number of different alternatives, but didn't have a use case for when SCHEDUELR_STOPPED() would be wrong. Now I do. I think that you are right. I'll make that change. Sorry for the hassle this may have caused. I'll submit a review and add you as a reviewer today. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrOROVz1W2jQ%2BfsPFwPPXJHE1XqocsfaKN6AgYVO3wSjA>