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.

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);
 

Reply via email to