> Date: Tue, 2 Aug 2022 11:56:59 -0500
> From: Scott Cheloha <scottchel...@gmail.com>
>
> 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 <scottchel...@gmail.com>
> > > >
> > > > 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?
Sorry. I'm not set up to run my powerpc64 machine for extended
periods of time. But I'm fine with this going in if George can
confirm that this diff builds.
> 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);
>
>
