Date: Fri, 17 Nov 2000 18:59:12 -0800 From: Jake Burkholder <jburkhol@home.com> To: John Baldwin <jhb@FreeBSD.ORG> Cc: smp@FreeBSD.ORG Subject: Re: cvs commit: src/sys/kern kern_timeout.c Message-ID: <20001118025912.158D4BA7A@io.yi.org> In-Reply-To: Message from John Baldwin <jhb@FreeBSD.ORG> of "Fri, 17 Nov 2000 16:47:30 PST." <XFMail.001117164730.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multipart MIME message. --==_Exmh_18918598640 Content-Type: text/plain; charset=us-ascii > > On 18-Nov-00 John Baldwin wrote: > > > > On 18-Nov-00 John Baldwin wrote: > >> jhb 2000/11/17 16:21:01 PST > >> > >> Modified files: > >> sys/kern kern_timeout.c > >> Log: > >> Release sched_lock very briefly to give interrupts a chance to fire if we > >> are in softclock() for a long time. The old code already did an > >> splx()/slphigh() pair here, I just missed adding in the equivalent mutex > >> operations on sched_lock earlier. > > > > Before this I could get processes to hang (the box was still up, but one > > process was stuck, tying up its pty) though the box was still fine (I could > > login and do stuff) if I ran a buildworld -j X where X > 1. With this I > > built > > a kernel with -j 8 and am currently chugging through a -j 64 world. > > Grrrr. Spoke too soon. :( It took longer, but I have one hung process (though > the rest of the buildworld is still chugging along). I notice the problem > because I have one sh process that has been a zombie for a way too long. It's > parent is a make doing a make cleandir, and the parent is in SSLEEP on > "pipewr". I notice that when the kernel tsleep()'s with "pipewr", it does a > tsleep() with PCATCH, so I am betting that there it is still related to the > CURSIG() stuff in msleep(). Since this doesn't happen very often, I'm guessing > that this comes about when a process is switched out because it blocked on > Giant. Hmm. Do you know if its a timed sleep? If so, its possible that you're missing a timeout because hardclock and softclock are racing. If its not a timeout, and the wakeup happens while we're blocked on Giant, it should be OK because wakeup will set p_wchan to 0 even if the process is in state SMTX and we'll goto resume when the process gets unblocked. Anyway, here's the callout_lock patch I mentioned, it might help. It will conflict with the above commit because I don't have it locally yet, but its small. Before this gets committed I'd like to move the callout initialization to its own sysinit in kern_timeout.c, so we can initialize the lock there instead of in all the machdep.c's. I think the APM_FIXUP_CALLTODO thing has rotted, its based on the old delta queue timeout implementation. Jake > > -- > > John Baldwin <jhb@FreeBSD.org> -- http://www.FreeBSD.org/~jhb/ > PGP Key: http://www.baldwin.cx/~john/pgpkey.asc > "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ > > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-smp" in the body of the message --==_Exmh_18918598640 Content-Type: text/plain ; name="timo2.diff"; charset=us-ascii Content-Description: timo2.diff Content-Disposition: attachment; filename="timo2.diff" Index: sys/callout.h =================================================================== RCS file: /home/ncvs/src/sys/sys/callout.h,v retrieving revision 1.17 diff -u -r1.17 callout.h --- sys/callout.h 2000/05/26 02:06:53 1.17 +++ sys/callout.h 2000/11/18 02:00:18 @@ -61,6 +61,7 @@ #define CALLOUT_LOCAL_ALLOC 0x0001 /* was allocated from callfree */ #define CALLOUT_ACTIVE 0x0002 /* callout is currently active */ #define CALLOUT_PENDING 0x0004 /* callout is waiting for timeout */ +#define CALLOUT_MPSAFE 0x0008 /* callout handler is mp safe */ struct callout_handle { struct callout *callout; @@ -72,6 +73,7 @@ extern int ncallout; extern struct callout_tailq *callwheel; extern int callwheelsize, callwheelbits, callwheelmask, softticks; +extern struct mtx callout_lock; #define callout_active(c) ((c)->c_flags & CALLOUT_ACTIVE) #define callout_deactivate(c) ((c)->c_flags &= ~CALLOUT_ACTIVE) Index: kern/kern_clock.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_clock.c,v retrieving revision 1.115 diff -u -r1.115 kern_clock.c --- kern/kern_clock.c 2000/10/25 05:19:38 1.115 +++ kern/kern_clock.c 2000/11/18 02:05:45 @@ -154,6 +154,7 @@ register struct clockframe *frame; { register struct proc *p; + int need_softclock = 0; p = curproc; if (p != idleproc) { @@ -187,16 +188,25 @@ statclock(frame); tc_windup(); - ticks++; /* * Process callouts at a very low cpu priority, so we don't keep the * relatively high clock interrupt priority any longer than necessary. */ + mtx_enter(&callout_lock, MTX_SPIN); + ticks++; if (TAILQ_FIRST(&callwheel[ticks & callwheelmask]) != NULL) { - sched_swi(softclock_ih, SWI_NOSWITCH); + need_softclock = 1; } else if (softticks + 1 == ticks) ++softticks; + mtx_exit(&callout_lock, MTX_SPIN); + + /* + * sched_swi acquires sched_lock, so we don't want to call it with + * callout_lock held; incorrect locking order. + */ + if (need_softclock) + sched_swi(softclock_ih, SWI_NOSWITCH); } /* Index: kern/kern_intr.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_intr.c,v retrieving revision 1.31 diff -u -r1.31 kern_intr.c --- kern/kern_intr.c 2000/11/15 22:05:23 1.31 +++ kern/kern_intr.c 2000/11/18 02:06:07 @@ -258,7 +258,7 @@ { net_ih = sinthand_add("net", NULL, swi_net, NULL, SWI_NET, 0); softclock_ih = - sinthand_add("clock", &clk_ithd, softclock, NULL, SWI_CLOCK, 0); + sinthand_add("clock", &clk_ithd, softclock, NULL, SWI_CLOCK, INTR_MPSAFE); vm_ih = sinthand_add("vm", NULL, swi_vm, NULL, SWI_VM, 0); } Index: kern/kern_mutex.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_mutex.c,v retrieving revision 1.17 diff -u -r1.17 kern_mutex.c --- kern/kern_mutex.c 2000/11/17 18:09:14 1.17 +++ kern/kern_mutex.c 2000/11/18 02:09:55 @@ -703,6 +703,7 @@ #ifdef __i386__ "clk", #endif + "callout", /* * leaf locks */ Index: kern/kern_timeout.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_timeout.c,v retrieving revision 1.61 diff -u -r1.61 kern_timeout.c --- kern/kern_timeout.c 2000/11/16 21:20:52 1.61 +++ kern/kern_timeout.c 2000/11/18 02:18:10 @@ -56,6 +56,7 @@ int callwheelsize, callwheelbits, callwheelmask; struct callout_tailq *callwheel; int softticks; /* Like ticks, but for softclock(). */ +MUTEX_DECLARE(, callout_lock); static struct callout *nextsoftcheck; /* Next callout to be checked. */ @@ -90,7 +91,7 @@ steps = 0; s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + mtx_enter(&callout_lock, MTX_SPIN); while (softticks != ticks) { softticks++; /* @@ -107,8 +108,10 @@ if (steps >= MAX_SOFTCLOCK_STEPS) { nextsoftcheck = c; /* Give interrupts a chance. */ + mtx_exit(&callout_lock, MTX_SPIN); splx(s); s = splhigh(); + mtx_enter(&callout_lock, MTX_SPIN); c = nextsoftcheck; steps = 0; } @@ -129,18 +132,22 @@ c->c_flags = (c->c_flags & ~CALLOUT_PENDING); } - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); + if (!(c->c_flags & CALLOUT_MPSAFE)) + mtx_enter(&Giant, MTX_DEF); splx(s); c_func(c_arg); s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + if (!(c->c_flags & CALLOUT_MPSAFE)) + mtx_exit(&Giant, MTX_DEF); + mtx_enter(&callout_lock, MTX_SPIN); steps = 0; c = nextsoftcheck; } } } nextsoftcheck = NULL; - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); } @@ -171,7 +178,7 @@ struct callout_handle handle; s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + mtx_enter(&callout_lock, MTX_SPIN); /* Fill in the next free callout structure. */ new = SLIST_FIRST(&callfree); @@ -183,7 +190,7 @@ callout_reset(new, to_ticks, ftn, arg); handle.callout = new; - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); return (handle); } @@ -205,10 +212,10 @@ return; s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + mtx_enter(&callout_lock, MTX_SPIN); if (handle.callout->c_func == ftn && handle.callout->c_arg == arg) callout_stop(handle.callout); - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); } @@ -242,7 +249,7 @@ int s; s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + mtx_enter(&callout_lock, MTX_SPIN); if (c->c_flags & CALLOUT_PENDING) callout_stop(c); @@ -260,7 +267,7 @@ c->c_time = ticks + to_ticks; TAILQ_INSERT_TAIL(&callwheel[c->c_time & callwheelmask], c, c_links.tqe); - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); } @@ -271,13 +278,13 @@ int s; s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + mtx_enter(&callout_lock, MTX_SPIN); /* * Don't attempt to delete a callout that's not on the queue. */ if (!(c->c_flags & CALLOUT_PENDING)) { c->c_flags &= ~CALLOUT_ACTIVE; - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); return; } @@ -292,7 +299,7 @@ if (c->c_flags & CALLOUT_LOCAL_ALLOC) { SLIST_INSERT_HEAD(&callfree, c, c_links.sle); } - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); } @@ -354,7 +361,7 @@ /* don't collide with softclock() */ s = splhigh(); - mtx_enter(&sched_lock, MTX_SPIN); + mtx_enter(&callout_lock, MTX_SPIN); for (p = calltodo.c_next; p != NULL; p = p->c_next) { p->c_time -= delta_ticks; @@ -365,7 +372,7 @@ /* take back the ticks the timer didn't use (p->c_time <= 0) */ delta_ticks = -p->c_time; } - mtx_exit(&sched_lock, MTX_SPIN); + mtx_exit(&callout_lock, MTX_SPIN); splx(s); return; Index: i386/i386/machdep.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/machdep.c,v retrieving revision 1.420 diff -u -r1.420 machdep.c --- i386/i386/machdep.c 2000/10/27 11:45:23 1.420 +++ i386/i386/machdep.c 2000/11/18 01:59:51 @@ -409,6 +409,8 @@ TAILQ_INIT(&callwheel[i]); } + mtx_init(&callout_lock, "callout", MTX_SPIN | MTX_COLD); + #if defined(USERCONFIG) userconfig(); cninit(); /* the preferred console may have changed */ --==_Exmh_18918598640-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001118025912.158D4BA7A>