| Message ID | 20220303210425.1693486-3-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: SIGP fixes | expand |
On 3/3/22 22:04, Eric Farman wrote: > test_restart() makes two smp_cpu_restart() calls against CPU 1. > It claims to perform both of them against running (operating) CPUs, > but the first invocation tries to achieve this by calling > smp_cpu_stop() to CPU 0. This will be rejected by the library. I played myself there :) > > Let's fix this by making the first restart operate on a stopped CPU, > to ensure it gets test coverage instead of relying on other callers. > > Fixes: 166da884d ("s390x: smp: Add restart when running test") > Signed-off-by: Eric Farman <farman@linux.ibm.com> If you want to you can add a report_pass() after the first wait flag. Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/smp.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/s390x/smp.c b/s390x/smp.c > index 068ac74d..2f4af820 100644 > --- a/s390x/smp.c > +++ b/s390x/smp.c > @@ -50,10 +50,6 @@ static void test_start(void) > report_pass("start"); > } > > -/* > - * Does only test restart when the target is running. > - * The other tests do restarts when stopped multiple times already. > - */ > static void test_restart(void) > { > struct cpu *cpu = smp_cpu_from_idx(1); > @@ -62,8 +58,8 @@ static void test_restart(void) > lc->restart_new_psw.mask = extract_psw_mask(); > lc->restart_new_psw.addr = (unsigned long)test_func; > > - /* Make sure cpu is running */ > - smp_cpu_stop(0); > + /* Make sure cpu is stopped */ > + smp_cpu_stop(1); > set_flag(0); > smp_cpu_restart(1); > wait_for_flag();
On Fri, 2022-03-04 at 11:43 +0100, Janosch Frank wrote: > On 3/3/22 22:04, Eric Farman wrote: > > test_restart() makes two smp_cpu_restart() calls against CPU 1. > > It claims to perform both of them against running (operating) CPUs, > > but the first invocation tries to achieve this by calling > > smp_cpu_stop() to CPU 0. This will be rejected by the library. > > I played myself there :) :) > > > Let's fix this by making the first restart operate on a stopped > > CPU, > > to ensure it gets test coverage instead of relying on other > > callers. > > > > Fixes: 166da884d ("s390x: smp: Add restart when running test") > > Signed-off-by: Eric Farman <farman@linux.ibm.com> > > If you want to you can add a report_pass() after the first wait flag. I did in patch 5. I can move that here, but that patch has some additional prefix changes for those reports, so didn't want to move TOO much of that to this fix. > > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Thank you! > > > --- > > s390x/smp.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/s390x/smp.c b/s390x/smp.c > > index 068ac74d..2f4af820 100644 > > --- a/s390x/smp.c > > +++ b/s390x/smp.c > > @@ -50,10 +50,6 @@ static void test_start(void) > > report_pass("start"); > > } > > > > -/* > > - * Does only test restart when the target is running. > > - * The other tests do restarts when stopped multiple times > > already. > > - */ > > static void test_restart(void) > > { > > struct cpu *cpu = smp_cpu_from_idx(1); > > @@ -62,8 +58,8 @@ static void test_restart(void) > > lc->restart_new_psw.mask = extract_psw_mask(); > > lc->restart_new_psw.addr = (unsigned long)test_func; > > > > - /* Make sure cpu is running */ > > - smp_cpu_stop(0); > > + /* Make sure cpu is stopped */ > > + smp_cpu_stop(1); > > set_flag(0); > > smp_cpu_restart(1); > > wait_for_flag();
On Thu, 2022-03-03 at 22:04 +0100, Eric Farman wrote: > test_restart() makes two smp_cpu_restart() calls against CPU 1. > It claims to perform both of them against running (operating) CPUs, > but the first invocation tries to achieve this by calling > smp_cpu_stop() to CPU 0. This will be rejected by the library. > > Let's fix this by making the first restart operate on a stopped CPU, > to ensure it gets test coverage instead of relying on other callers. > > Fixes: 166da884d ("s390x: smp: Add restart when running test") > Signed-off-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
On Thu, 3 Mar 2022 22:04:21 +0100 Eric Farman <farman@linux.ibm.com> wrote: > test_restart() makes two smp_cpu_restart() calls against CPU 1. > It claims to perform both of them against running (operating) CPUs, > but the first invocation tries to achieve this by calling > smp_cpu_stop() to CPU 0. This will be rejected by the library. > > Let's fix this by making the first restart operate on a stopped CPU, > to ensure it gets test coverage instead of relying on other callers. > > Fixes: 166da884d ("s390x: smp: Add restart when running test") > Signed-off-by: Eric Farman <farman@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > s390x/smp.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/s390x/smp.c b/s390x/smp.c > index 068ac74d..2f4af820 100644 > --- a/s390x/smp.c > +++ b/s390x/smp.c > @@ -50,10 +50,6 @@ static void test_start(void) > report_pass("start"); > } > > -/* > - * Does only test restart when the target is running. > - * The other tests do restarts when stopped multiple times already. > - */ > static void test_restart(void) > { > struct cpu *cpu = smp_cpu_from_idx(1); > @@ -62,8 +58,8 @@ static void test_restart(void) > lc->restart_new_psw.mask = extract_psw_mask(); > lc->restart_new_psw.addr = (unsigned long)test_func; > > - /* Make sure cpu is running */ > - smp_cpu_stop(0); > + /* Make sure cpu is stopped */ > + smp_cpu_stop(1); > set_flag(0); > smp_cpu_restart(1); > wait_for_flag();
diff --git a/s390x/smp.c b/s390x/smp.c index 068ac74d..2f4af820 100644 --- a/s390x/smp.c +++ b/s390x/smp.c @@ -50,10 +50,6 @@ static void test_start(void) report_pass("start"); } -/* - * Does only test restart when the target is running. - * The other tests do restarts when stopped multiple times already. - */ static void test_restart(void) { struct cpu *cpu = smp_cpu_from_idx(1); @@ -62,8 +58,8 @@ static void test_restart(void) lc->restart_new_psw.mask = extract_psw_mask(); lc->restart_new_psw.addr = (unsigned long)test_func; - /* Make sure cpu is running */ - smp_cpu_stop(0); + /* Make sure cpu is stopped */ + smp_cpu_stop(1); set_flag(0); smp_cpu_restart(1); wait_for_flag();
test_restart() makes two smp_cpu_restart() calls against CPU 1. It claims to perform both of them against running (operating) CPUs, but the first invocation tries to achieve this by calling smp_cpu_stop() to CPU 0. This will be rejected by the library. Let's fix this by making the first restart operate on a stopped CPU, to ensure it gets test coverage instead of relying on other callers. Fixes: 166da884d ("s390x: smp: Add restart when running test") Signed-off-by: Eric Farman <farman@linux.ibm.com> --- s390x/smp.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)