On Mon, Jul 25, 2022 at 06:44:31PM -0500, Scott Cheloha wrote:
> On Mon, Jul 25, 2022 at 01:52:36PM +0200, Mark Kettenis wrote:
> > > Date: Sun, 24 Jul 2022 19:33:57 -0500
> > > From: Scott Cheloha <[email protected]>
> > >
> > > On Sat, Jul 23, 2022 at 08:14:32PM -0500, Scott Cheloha wrote:
> > > >
> > > > [...]
> > > >
> > > > I don't have a powerpc64 machine, so this is untested. [...]
> > >
> > > gkoehler@ has pointed out two dumb typos in the prior patch. My bad.
> > >
> > > Here is a corrected patch that, according to gkoehler@, actually
> > > compiles.
> >
> > Thanks. I already figured that bit out myself. Did some limited
> > testing, but it seems to work correctly. No noticable effect on the
> > timekeeping even when building clang on all the (4) cores.
>
> I wouldn't expect this patch to impact timekeeping. All we're doing
> is calling hardclock(9) a bit sooner than we normally would a few
> times every second.
>
> I would expect to see slightly more distinct interrupts (uvmexp.intrs)
> per second because we aren't actively batching hardclock(9) and
> statclock calls.
>
> ... by the way, uvmexp.intrs should probably be incremented
> atomically, no?
>
> > Regarding the diff, I think it would be better to avoid changing
> > trap.c. That function is complicated enough and splitting the logic
> > for this over three files makes it a bit harder to understand. So you
> > could have:
> >
> > void
> > decr_intr(struct trapframe *frame)
> > {
> > struct cpu_info *ci = curcpu();
> > ...
> > int s;
> >
> > if (ci->ci_cpl >= IPL_CLOCK) {
> > ci->ci_dec_deferred = 1;
> > mtdec(UINT32_MAX >> 1); /* clear DEC exception */
> > return;
> > }
> >
> > ci->ci_dec_deferred = 0;
> >
> > ...
> > }
> >
> > That has the downside of course that it will be slightly less
> > efficient if we're at IPL_CLOCK or above, but that really shouldn't
> > happen often enough for it to matter.
>
> Yep. It's an extra function call, the overhead is small.
>
> Updated patch below.
At what point do we consider the patch safe? Have you seen any hangs?
Wanna run with it another week?
Index: include/cpu.h
===================================================================
RCS file: /cvs/src/sys/arch/powerpc64/include/cpu.h,v
retrieving revision 1.31
diff -u -p -r1.31 cpu.h
--- include/cpu.h 6 Jul 2021 09:34:07 -0000 1.31
+++ include/cpu.h 25 Jul 2022 23:43:47 -0000
@@ -74,9 +74,9 @@ struct cpu_info {
uint64_t ci_lasttb;
uint64_t ci_nexttimerevent;
uint64_t ci_nextstatevent;
- int ci_statspending;
volatile int ci_cpl;
+ volatile int ci_dec_deferred;
uint32_t ci_ipending;
uint32_t ci_idepth;
#ifdef DIAGNOSTIC
Index: powerpc64/clock.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/clock.c,v
retrieving revision 1.3
diff -u -p -r1.3 clock.c
--- powerpc64/clock.c 23 Feb 2021 04:44:31 -0000 1.3
+++ powerpc64/clock.c 25 Jul 2022 23:43:47 -0000
@@ -98,6 +98,17 @@ decr_intr(struct trapframe *frame)
int s;
/*
+ * If the clock interrupt is masked, postpone all work until
+ * it is unmasked in splx(9).
+ */
+ if (ci->ci_cpl >= IPL_CLOCK) {
+ ci->ci_dec_deferred = 1;
+ mtdec(UINT32_MAX >> 1); /* clear DEC exception */
+ return;
+ }
+ ci->ci_dec_deferred = 0;
+
+ /*
* Based on the actual time delay since the last decrementer reload,
* we arrange for earlier interrupt next time.
*/
@@ -130,30 +141,23 @@ decr_intr(struct trapframe *frame)
mtdec(nextevent - tb);
mtdec(nextevent - mftb());
- if (ci->ci_cpl >= IPL_CLOCK) {
- ci->ci_statspending += nstats;
- } else {
- nstats += ci->ci_statspending;
- ci->ci_statspending = 0;
-
- s = splclock();
- intr_enable();
-
- /*
- * Do standard timer interrupt stuff.
- */
- while (ci->ci_lasttb < prevtb) {
- ci->ci_lasttb += tick_increment;
- clock_count.ec_count++;
- hardclock((struct clockframe *)frame);
- }
+ s = splclock();
+ intr_enable();
- while (nstats-- > 0)
- statclock((struct clockframe *)frame);
-
- intr_disable();
- splx(s);
+ /*
+ * Do standard timer interrupt stuff.
+ */
+ while (ci->ci_lasttb < prevtb) {
+ ci->ci_lasttb += tick_increment;
+ clock_count.ec_count++;
+ hardclock((struct clockframe *)frame);
}
+
+ while (nstats-- > 0)
+ statclock((struct clockframe *)frame);
+
+ intr_disable();
+ splx(s);
}
void
Index: powerpc64/intr.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/intr.c,v
retrieving revision 1.9
diff -u -p -r1.9 intr.c
--- powerpc64/intr.c 26 Sep 2020 17:56:54 -0000 1.9
+++ powerpc64/intr.c 25 Jul 2022 23:43:47 -0000
@@ -139,6 +139,11 @@ splx(int new)
{
struct cpu_info *ci = curcpu();
+ if (ci->ci_dec_deferred && new < IPL_CLOCK) {
+ mtdec(0);
+ mtdec(UINT32_MAX); /* raise DEC exception */
+ }
+
if (ci->ci_ipending & intr_smask[new])
intr_do_pending(new);
