Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
On Tue, 2 Aug 2022 19:55:02 +0200 (CEST)
Mark Kettenis wrote:
> > Date: Tue, 2 Aug 2022 11:56:59 -0500
> > From: Scott Cheloha
> >
> > 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.
Yes, it builds. ok gkoehler@
I have been running this version of the diff for a few hours, after
switching from the previous version (which had modified trap.c).
> > 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 - 1.31
> > +++ include/cpu.h 25 Jul 2022 23:43:47 -
> > @@ -74,9 +74,9 @@ struct cpu_info {
> > uint64_tci_lasttb;
> > uint64_tci_nexttimerevent;
> > uint64_tci_nextstatevent;
> > - int ci_statspending;
> >
> > volatile intci_cpl;
> > + volatile intci_dec_deferred;
> > uint32_tci_ipending;
> > uint32_tci_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 - 1.3
> > +++ powerpc64/clock.c 25 Jul 2022 23:43:47 -
> > @@ -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.c26 Sep 2020 17:56:54 - 1.9
> > +++ powerpc64/intr.c25 Jul 2022 23:43:47 -
> > @@ -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);
> >
> >
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
> Date: Tue, 2 Aug 2022 11:56:59 -0500
> From: Scott Cheloha
>
> 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
> > > >
> > > > 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 - 1.31
> +++ include/cpu.h 25 Jul 2022 23:43:47 -
> @@ -74,9 +74,9 @@ struct cpu_info {
> uint64_tci_lasttb;
> uint64_tci_nexttimerevent;
> uint64_tci_nextstatevent;
> - int ci_statspending;
>
> volatile intci_cpl;
> + volatile intci_dec_deferred;
> uint32_tci_ipending;
> uint32_tci_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 - 1.3
> +++ powerpc64/clock.c 25 Jul 2022 23:43:47 -
> @@ -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();
>
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
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
> > >
> > > 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 - 1.31
+++ include/cpu.h 25 Jul 2022 23:43:47 -
@@ -74,9 +74,9 @@ struct cpu_info {
uint64_tci_lasttb;
uint64_tci_nexttimerevent;
uint64_tci_nextstatevent;
- int ci_statspending;
volatile intci_cpl;
+ volatile intci_dec_deferred;
uint32_tci_ipending;
uint32_tci_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 - 1.3
+++ powerpc64/clock.c 25 Jul 2022 23:43:47 -
@@ -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);
+
+
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
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
> >
> > 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 - 1.31
+++ include/cpu.h 25 Jul 2022 23:43:47 -
@@ -74,9 +74,9 @@ struct cpu_info {
uint64_tci_lasttb;
uint64_tci_nexttimerevent;
uint64_tci_nextstatevent;
- int ci_statspending;
volatile intci_cpl;
+ volatile intci_dec_deferred;
uint32_tci_ipending;
uint32_tci_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 - 1.3
+++ powerpc64/clock.c 25 Jul 2022 23:43:47 -
@@ -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
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
> Date: Sun, 24 Jul 2022 19:33:57 -0500
> From: Scott Cheloha
>
> 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.
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.
> 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 - 1.31
> +++ include/cpu.h 25 Jul 2022 00:30:33 -
> @@ -74,9 +74,9 @@ struct cpu_info {
> uint64_tci_lasttb;
> uint64_tci_nexttimerevent;
> uint64_tci_nextstatevent;
> - int ci_statspending;
>
> volatile intci_cpl;
> + volatile intci_dec_deferred;
> uint32_tci_ipending;
> uint32_tci_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 - 1.3
> +++ powerpc64/clock.c 25 Jul 2022 00:30:33 -
> @@ -130,30 +130,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();
>
> - 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);
> - }
> + /*
> + * 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);
> + while (nstats-- > 0)
> + statclock((struct clockframe *)frame);
>
> - intr_disable();
> - splx(s);
> - }
> + 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 - 1.9
> +++ powerpc64/intr.c 25 Jul 2022 00:30:33 -
> @@ -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);
>
> Index: powerpc64/trap.c
> ===
> RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/trap.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 trap.c
> --- powerpc64/trap.c 11 May 2021 18:21:12 - 1.51
> +++ powerpc64/trap.c 25 Jul 2022 00:30:33 -
> @@ -65,9 +65,15 @@ trap(struct trapframe *frame)
> switch (type) {
> case EXC_DECR:
> uvmexp.intrs++;
> - ci->ci_idepth++;
> - decr_intr(frame);
> - ci->ci_idepth--;
> + if (ci->ci_cpl < IPL_CLOCK) {
> + ci->ci_dec_deferred = 0;
> + ci->ci_idepth++;
> +
Re: powerpc64: retrigger deferred DEC interrupts from splx(9)
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.
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 - 1.31
+++ include/cpu.h 25 Jul 2022 00:30:33 -
@@ -74,9 +74,9 @@ struct cpu_info {
uint64_tci_lasttb;
uint64_tci_nexttimerevent;
uint64_tci_nextstatevent;
- int ci_statspending;
volatile intci_cpl;
+ volatile intci_dec_deferred;
uint32_tci_ipending;
uint32_tci_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 - 1.3
+++ powerpc64/clock.c 25 Jul 2022 00:30:33 -
@@ -130,30 +130,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();
- 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);
- }
+ /*
+* 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);
+ while (nstats-- > 0)
+ statclock((struct clockframe *)frame);
- intr_disable();
- splx(s);
- }
+ 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.c26 Sep 2020 17:56:54 - 1.9
+++ powerpc64/intr.c25 Jul 2022 00:30:33 -
@@ -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);
Index: powerpc64/trap.c
===
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/trap.c,v
retrieving revision 1.51
diff -u -p -r1.51 trap.c
--- powerpc64/trap.c11 May 2021 18:21:12 - 1.51
+++ powerpc64/trap.c25 Jul 2022 00:30:33 -
@@ -65,9 +65,15 @@ trap(struct trapframe *frame)
switch (type) {
case EXC_DECR:
uvmexp.intrs++;
- ci->ci_idepth++;
- decr_intr(frame);
- ci->ci_idepth--;
+ if (ci->ci_cpl < IPL_CLOCK) {
+ ci->ci_dec_deferred = 0;
+ ci->ci_idepth++;
+ decr_intr(frame);
+ ci->ci_idepth--;
+ } else {
+ ci->ci_dec_deferred = 1;
+ mtdec(UINT32_MAX >> 1); /* clear DEC exception */
+ }
return;
case EXC_EXI:
uvmexp.intrs++;
powerpc64: retrigger deferred DEC interrupts from splx(9)
Okay, we did this for powerpc/macppc, on to powerpc64.
It's roughly the same problem as before:
- On powerpc64 we need to leave the DEC unmasked at or above
IPL_CLOCK.
- Currently we defer clock interrupt work to the next tick if a DEC
interrupt arrives when the CPU's priority level is at or above
IPL_CLOCK.
- This is a problem because the MD code needs to know about
when the next clock interrupt event is scheduled and I intend
to make that information machine-independent and handle it
in machine-independent code in the future.
- This patch instead defers clock interrupt work to the next splx(9)
call where the CPU's priority level is dropping below IPL_CLOCK.
This requires no knowledge of when the next clock interrupt
event is scheduled.
The code is almost identical to what we did for powerpc/macppc,
except that:
- We can do the ci_dec_deferred handling in trap(), which is a
bit cleaner.
- There is only one splx() function that needs modifying.
Unless I'm missing something, we no longer need the struct member
cpu_info.ci_statspending.
I don't have a powerpc64 machine, so this is untested. I would
appreciate tests and review. If you're copied on this, I'm under the
impression you have a powerpc64 machine or know someone who might.
Thoughts? Test results?
I'm really sorry if this doesn't work out of the box and your machine
hangs.
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 - 1.31
+++ include/cpu.h 24 Jul 2022 01:08:22 -
@@ -74,9 +74,9 @@ struct cpu_info {
uint64_tci_lasttb;
uint64_tci_nexttimerevent;
uint64_tci_nextstatevent;
- int ci_statspending;
volatile intci_cpl;
+ volatile intci_dec_deferred;
uint32_tci_ipending;
uint32_tci_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 - 1.3
+++ powerpc64/clock.c 24 Jul 2022 01:08:22 -
@@ -130,30 +130,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();
- 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);
- }
+ /*
+* 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);
+ while (nstats-- > 0)
+ statclock((struct clockframe *)frame);
- intr_disable();
- splx(s);
- }
+ 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.c26 Sep 2020 17:56:54 - 1.9
+++ powerpc64/intr.c24 Jul 2022 01:08:22 -
@@ -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);
Index: powerpc64/trap.c
===
RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/trap.c,v
retrieving revision 1.51
diff -u -p -r1.51 trap.c
--- powerpc64/trap.c11 May 2021 18:21:12 - 1.51
+++ powerpc64/trap.c24 Jul 2022 01:08:22 -
@@ -65,9 +65,15 @@ trap(struct trapframe *frame)
switch (type) {
case EXC_DECR:
uvmexp.intrs++;
- ci->ci_idepth++;
- decr_intr(frame);
- ci->ci_idepth--;
+ if (ci->ci_cpl < IPL_CLOCK) {
+ ci->ci_decr_deferred = 0;
+
