| Message ID | 1525941957-9140-1-git-send-email-akshay.adiga@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SLW: Remove stop1_lite and stop2_lite | expand |
On Thu, May 10, 2018 at 02:15:57PM +0530, Akshay Adiga wrote: > Lite states don't give up the SMT resources, can potentially have a > performance impact on sibling threads. At latencies of stop1_lite and > stop2_lite , lite states dont make sense and can have performance > impact. > > Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> > --- > hw/slw.c | 29 +---------------------------- > 1 file changed, 1 insertion(+), 28 deletions(-) > > diff --git a/hw/slw.c b/hw/slw.c > index c059d9c..513be7b 100644 > --- a/hw/slw.c > +++ b/hw/slw.c > @@ -529,20 +529,7 @@ static struct cpu_idle_states power9_cpu_idle_states[] = { > | OPAL_PM_PSSCR_ESL \ > | OPAL_PM_PSSCR_EC, > .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > - { > - .name = "stop1_lite", /* Enter stop1 with no state loss */ > - .latency_ns = 4900, > - .residency_ns = 49000, > - .flags = 0*OPAL_PM_DEC_STOP \ > - | 0*OPAL_PM_TIMEBASE_STOP \ > - | 0*OPAL_PM_LOSE_USER_CONTEXT \ > - | 0*OPAL_PM_LOSE_HYP_CONTEXT \ > - | 0*OPAL_PM_LOSE_FULL_CONTEXT \ > - | 1*OPAL_PM_STOP_INST_FAST, > - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(1) \ > - | OPAL_PM_PSSCR_MTL(3) \ > - | OPAL_PM_PSSCR_TR(3), > - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > + > { > .name = "stop1", > .latency_ns = 5000, > @@ -560,20 +547,6 @@ static struct cpu_idle_states power9_cpu_idle_states[] = { > | OPAL_PM_PSSCR_EC, > .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > { > - .name = "stop2_lite", /* Enter stop2 with no state loss */ > - .latency_ns = 9900, > - .residency_ns = 99000, > - .flags = 0*OPAL_PM_DEC_STOP \ > - | 0*OPAL_PM_TIMEBASE_STOP \ > - | 0*OPAL_PM_LOSE_USER_CONTEXT \ > - | 0*OPAL_PM_LOSE_HYP_CONTEXT \ > - | 0*OPAL_PM_LOSE_FULL_CONTEXT \ > - | 1*OPAL_PM_STOP_INST_FAST, > - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(2) \ > - | OPAL_PM_PSSCR_MTL(3) \ > - | OPAL_PM_PSSCR_TR(3), > - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > - { > .name = "stop2", > .latency_ns = 10000, > .residency_ns = 100000, > -- > 2.5.5 > This is a followup on the previous patch : https://patchwork.ozlabs.org/patch/906582/ Nick,Stewart does this look good ?
On Tue, 15 May 2018 12:08:31 +0530 Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> wrote: > On Thu, May 10, 2018 at 02:15:57PM +0530, Akshay Adiga wrote: > > Lite states don't give up the SMT resources, can potentially have a > > performance impact on sibling threads. At latencies of stop1_lite > > and stop2_lite , lite states dont make sense and can have > > performance impact. > > > > Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> > > --- > > hw/slw.c | 29 +---------------------------- > > 1 file changed, 1 insertion(+), 28 deletions(-) > > > > diff --git a/hw/slw.c b/hw/slw.c > > index c059d9c..513be7b 100644 > > --- a/hw/slw.c > > +++ b/hw/slw.c > > @@ -529,20 +529,7 @@ static struct cpu_idle_states > > power9_cpu_idle_states[] = { | OPAL_PM_PSSCR_ESL \ > > | OPAL_PM_PSSCR_EC, > > .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > > - { > > - .name = "stop1_lite", /* Enter stop1 with no state > > loss */ > > - .latency_ns = 4900, > > - .residency_ns = 49000, > > - .flags = 0*OPAL_PM_DEC_STOP \ > > - | 0*OPAL_PM_TIMEBASE_STOP \ > > - | 0*OPAL_PM_LOSE_USER_CONTEXT \ > > - | 0*OPAL_PM_LOSE_HYP_CONTEXT \ > > - | 0*OPAL_PM_LOSE_FULL_CONTEXT \ > > - | 1*OPAL_PM_STOP_INST_FAST, > > - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(1) \ > > - | OPAL_PM_PSSCR_MTL(3) \ > > - | OPAL_PM_PSSCR_TR(3), > > - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > > + > > { > > .name = "stop1", > > .latency_ns = 5000, > > @@ -560,20 +547,6 @@ static struct cpu_idle_states > > power9_cpu_idle_states[] = { | OPAL_PM_PSSCR_EC, > > .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > > { > > - .name = "stop2_lite", /* Enter stop2 with no state > > loss */ > > - .latency_ns = 9900, > > - .residency_ns = 99000, > > - .flags = 0*OPAL_PM_DEC_STOP \ > > - | 0*OPAL_PM_TIMEBASE_STOP \ > > - | 0*OPAL_PM_LOSE_USER_CONTEXT \ > > - | 0*OPAL_PM_LOSE_HYP_CONTEXT \ > > - | 0*OPAL_PM_LOSE_FULL_CONTEXT \ > > - | 1*OPAL_PM_STOP_INST_FAST, > > - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(2) \ > > - | OPAL_PM_PSSCR_MTL(3) \ > > - | OPAL_PM_PSSCR_TR(3), > > - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, > > - { > > .name = "stop2", > > .latency_ns = 10000, > > .residency_ns = 100000, > > -- > > 2.5.5 > > > > This is a followup on the previous patch : > https://patchwork.ozlabs.org/patch/906582/ > > Nick,Stewart does this look good ? > Yes I think so. The rest we can look at tuning with Linux and dt parameters, but I think this should avoid most of the pathological cases and work with existing kernels, without overly hard coding policy. I would add a comment as to why we're leaving those states out though. Thanks, Nick
On Tue, May 15, 2018 at 05:21:04PM +1000, Nicholas Piggin wrote: > On Tue, 15 May 2018 12:08:31 +0530 > Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> wrote: > > > On Thu, May 10, 2018 at 02:15:57PM +0530, Akshay Adiga wrote: > > > Lite states don't give up the SMT resources, can potentially have a > > > performance impact on sibling threads. At latencies of stop1_lite > > > and stop2_lite , lite states dont make sense and can have > > > performance impact. > > > > > > Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> > > > > > > > This is a followup on the previous patch : > > https://patchwork.ozlabs.org/patch/906582/ > > > > Nick,Stewart does this look good ? > > > > Yes I think so. The rest we can look at tuning with Linux and dt > parameters, but I think this should avoid most of the pathological > cases and work with existing kernels, without overly hard coding > policy. > > I would add a comment as to why we're leaving those states out > though. I dont have problem adding a comment. But it will be like adding a comment for the code that does removed code. Anyways, we have the the reason mentioned in git log. > > Thanks, > Nick >
diff --git a/hw/slw.c b/hw/slw.c index c059d9c..513be7b 100644 --- a/hw/slw.c +++ b/hw/slw.c @@ -529,20 +529,7 @@ static struct cpu_idle_states power9_cpu_idle_states[] = { | OPAL_PM_PSSCR_ESL \ | OPAL_PM_PSSCR_EC, .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, - { - .name = "stop1_lite", /* Enter stop1 with no state loss */ - .latency_ns = 4900, - .residency_ns = 49000, - .flags = 0*OPAL_PM_DEC_STOP \ - | 0*OPAL_PM_TIMEBASE_STOP \ - | 0*OPAL_PM_LOSE_USER_CONTEXT \ - | 0*OPAL_PM_LOSE_HYP_CONTEXT \ - | 0*OPAL_PM_LOSE_FULL_CONTEXT \ - | 1*OPAL_PM_STOP_INST_FAST, - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(1) \ - | OPAL_PM_PSSCR_MTL(3) \ - | OPAL_PM_PSSCR_TR(3), - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, + { .name = "stop1", .latency_ns = 5000, @@ -560,20 +547,6 @@ static struct cpu_idle_states power9_cpu_idle_states[] = { | OPAL_PM_PSSCR_EC, .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, { - .name = "stop2_lite", /* Enter stop2 with no state loss */ - .latency_ns = 9900, - .residency_ns = 99000, - .flags = 0*OPAL_PM_DEC_STOP \ - | 0*OPAL_PM_TIMEBASE_STOP \ - | 0*OPAL_PM_LOSE_USER_CONTEXT \ - | 0*OPAL_PM_LOSE_HYP_CONTEXT \ - | 0*OPAL_PM_LOSE_FULL_CONTEXT \ - | 1*OPAL_PM_STOP_INST_FAST, - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(2) \ - | OPAL_PM_PSSCR_MTL(3) \ - | OPAL_PM_PSSCR_TR(3), - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK }, - { .name = "stop2", .latency_ns = 10000, .residency_ns = 100000,
Lite states don't give up the SMT resources, can potentially have a performance impact on sibling threads. At latencies of stop1_lite and stop2_lite , lite states dont make sense and can have performance impact. Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> --- hw/slw.c | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-)