* [kvm-unit-tests 0/2] s390x/smp fix and enhancement
@ 2020-03-30 8:49 Christian Borntraeger
2020-03-30 8:49 ` [kvm-unit-tests 1/2] s390x/smp: fix detection of "running" Christian Borntraeger
2020-03-30 8:49 ` [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status Christian Borntraeger
0 siblings, 2 replies; 9+ messages in thread
From: Christian Borntraeger @ 2020-03-30 8:49 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: KVM, David Hildenbrand, linux-s390, Cornelia Huck, Thomas Huth
Christian Borntraeger (2):
s390x/smp: fix detection of "running"
s390x/smp: add minimal test for sigp sense running status
lib/s390x/smp.c | 4 ++--
lib/s390x/smp.h | 2 +-
s390x/smp.c | 11 +++++++++++
3 files changed, 14 insertions(+), 3 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [kvm-unit-tests 1/2] s390x/smp: fix detection of "running"
2020-03-30 8:49 [kvm-unit-tests 0/2] s390x/smp fix and enhancement Christian Borntraeger
@ 2020-03-30 8:49 ` Christian Borntraeger
2020-03-30 9:03 ` David Hildenbrand
` (2 more replies)
2020-03-30 8:49 ` [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status Christian Borntraeger
1 sibling, 3 replies; 9+ messages in thread
From: Christian Borntraeger @ 2020-03-30 8:49 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: KVM, David Hildenbrand, linux-s390, Cornelia Huck, Thomas Huth
On s390x hosts with a single CPU, the smp test case hangs (loops).
The check is our restart has finished is wrong.
Sigp sense running status checks if the CPU is currently backed by a
real CPU. This means that on single CPU hosts a sigp sense running
will never claim that a target is running. We need to check for not
being stopped instead.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
lib/s390x/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index 2555bf4..5ed8b7b 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -128,7 +128,7 @@ static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
* The order has been accepted, but the actual restart may not
* have been performed yet, so wait until the cpu is running.
*/
- while (!smp_cpu_running(addr))
+ while (smp_cpu_stopped(addr))
mb();
cpu->active = true;
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status
2020-03-30 8:49 [kvm-unit-tests 0/2] s390x/smp fix and enhancement Christian Borntraeger
2020-03-30 8:49 ` [kvm-unit-tests 1/2] s390x/smp: fix detection of "running" Christian Borntraeger
@ 2020-03-30 8:49 ` Christian Borntraeger
2020-03-30 10:58 ` David Hildenbrand
1 sibling, 1 reply; 9+ messages in thread
From: Christian Borntraeger @ 2020-03-30 8:49 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: KVM, David Hildenbrand, linux-s390, Cornelia Huck, Thomas Huth
make sure that sigp sense running status returns a sane value for
stopped CPUs.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
lib/s390x/smp.c | 2 +-
lib/s390x/smp.h | 2 +-
s390x/smp.c | 11 +++++++++++
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index 5ed8b7b..492cb05 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -58,7 +58,7 @@ bool smp_cpu_stopped(uint16_t addr)
return !!(status & (SIGP_STATUS_CHECK_STOP|SIGP_STATUS_STOPPED));
}
-bool smp_cpu_running(uint16_t addr)
+bool smp_sense_running_status(uint16_t addr)
{
if (sigp(addr, SIGP_SENSE_RUNNING, 0, NULL) != SIGP_CC_STATUS_STORED)
return true;
diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
index a8b98c0..639ec92 100644
--- a/lib/s390x/smp.h
+++ b/lib/s390x/smp.h
@@ -40,7 +40,7 @@ struct cpu_status {
int smp_query_num_cpus(void);
struct cpu *smp_cpu_from_addr(uint16_t addr);
bool smp_cpu_stopped(uint16_t addr);
-bool smp_cpu_running(uint16_t addr);
+bool smp_sense_running_status(uint16_t addr);
int smp_cpu_restart(uint16_t addr);
int smp_cpu_start(uint16_t addr, struct psw psw);
int smp_cpu_stop(uint16_t addr);
diff --git a/s390x/smp.c b/s390x/smp.c
index 79cdc1f..f9f143d 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -210,6 +210,16 @@ static void test_emcall(void)
report_prefix_pop();
}
+static void test_sense_running(void)
+{
+ report_prefix_push("sense_running");
+ /* make sure CPU is stopped */
+ smp_cpu_stop(1);
+ report(!smp_sense_running_status(1), "CPU1 sense claims not running");
+ report_prefix_pop();
+}
+
+
/* Used to dirty registers of cpu #1 before it is reset */
static void test_func_initial(void)
{
@@ -319,6 +329,7 @@ int main(void)
test_store_status();
test_ecall();
test_emcall();
+ test_sense_running();
test_reset();
test_reset_initial();
smp_cpu_destroy(1);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests 1/2] s390x/smp: fix detection of "running"
2020-03-30 8:49 ` [kvm-unit-tests 1/2] s390x/smp: fix detection of "running" Christian Borntraeger
@ 2020-03-30 9:03 ` David Hildenbrand
2020-03-30 9:17 ` Janosch Frank
2020-03-31 6:49 ` Cornelia Huck
2 siblings, 0 replies; 9+ messages in thread
From: David Hildenbrand @ 2020-03-30 9:03 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: KVM, linux-s390, Cornelia Huck, Thomas Huth
On 30.03.20 10:49, Christian Borntraeger wrote:
> On s390x hosts with a single CPU, the smp test case hangs (loops).
> The check is our restart has finished is wrong.
> Sigp sense running status checks if the CPU is currently backed by a
> real CPU. This means that on single CPU hosts a sigp sense running
> will never claim that a target is running. We need to check for not
> being stopped instead.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> lib/s390x/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 2555bf4..5ed8b7b 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -128,7 +128,7 @@ static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
> * The order has been accepted, but the actual restart may not
> * have been performed yet, so wait until the cpu is running.
> */
> - while (!smp_cpu_running(addr))
> + while (smp_cpu_stopped(addr))
> mb();
> cpu->active = true;
> return 0;
>
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests 1/2] s390x/smp: fix detection of "running"
2020-03-30 8:49 ` [kvm-unit-tests 1/2] s390x/smp: fix detection of "running" Christian Borntraeger
2020-03-30 9:03 ` David Hildenbrand
@ 2020-03-30 9:17 ` Janosch Frank
2020-03-31 6:49 ` Cornelia Huck
2 siblings, 0 replies; 9+ messages in thread
From: Janosch Frank @ 2020-03-30 9:17 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: KVM, David Hildenbrand, linux-s390, Cornelia Huck, Thomas Huth
[-- Attachment #1.1: Type: text/plain, Size: 1126 bytes --]
On 3/30/20 10:49 AM, Christian Borntraeger wrote:
> On s390x hosts with a single CPU, the smp test case hangs (loops).
> The check is our restart has finished is wrong.
> Sigp sense running status checks if the CPU is currently backed by a
> real CPU. This means that on single CPU hosts a sigp sense running
> will never claim that a target is running. We need to check for not
> being stopped instead.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> lib/s390x/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 2555bf4..5ed8b7b 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -128,7 +128,7 @@ static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
> * The order has been accepted, but the actual restart may not
> * have been performed yet, so wait until the cpu is running.
> */
> - while (!smp_cpu_running(addr))
> + while (smp_cpu_stopped(addr))
> mb();
> cpu->active = true;
> return 0;
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status
2020-03-30 8:49 ` [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status Christian Borntraeger
@ 2020-03-30 10:58 ` David Hildenbrand
2020-04-02 10:44 ` Christian Borntraeger
0 siblings, 1 reply; 9+ messages in thread
From: David Hildenbrand @ 2020-03-30 10:58 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank
Cc: KVM, linux-s390, Cornelia Huck, Thomas Huth
On 30.03.20 10:49, Christian Borntraeger wrote:
> make sure that sigp sense running status returns a sane value for
> stopped CPUs.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> lib/s390x/smp.c | 2 +-
> lib/s390x/smp.h | 2 +-
> s390x/smp.c | 11 +++++++++++
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 5ed8b7b..492cb05 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -58,7 +58,7 @@ bool smp_cpu_stopped(uint16_t addr)
> return !!(status & (SIGP_STATUS_CHECK_STOP|SIGP_STATUS_STOPPED));
> }
>
> -bool smp_cpu_running(uint16_t addr)
> +bool smp_sense_running_status(uint16_t addr)
> {
> if (sigp(addr, SIGP_SENSE_RUNNING, 0, NULL) != SIGP_CC_STATUS_STORED)
> return true;
> diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
> index a8b98c0..639ec92 100644
> --- a/lib/s390x/smp.h
> +++ b/lib/s390x/smp.h
> @@ -40,7 +40,7 @@ struct cpu_status {
> int smp_query_num_cpus(void);
> struct cpu *smp_cpu_from_addr(uint16_t addr);
> bool smp_cpu_stopped(uint16_t addr);
> -bool smp_cpu_running(uint16_t addr);
> +bool smp_sense_running_status(uint16_t addr);
> int smp_cpu_restart(uint16_t addr);
> int smp_cpu_start(uint16_t addr, struct psw psw);
> int smp_cpu_stop(uint16_t addr);
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 79cdc1f..f9f143d 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -210,6 +210,16 @@ static void test_emcall(void)
> report_prefix_pop();
> }
>
> +static void test_sense_running(void)
> +{
> + report_prefix_push("sense_running");
> + /* make sure CPU is stopped */
> + smp_cpu_stop(1);
> + report(!smp_sense_running_status(1), "CPU1 sense claims not running");
> + report_prefix_pop();
> +}
> +
> +
> /* Used to dirty registers of cpu #1 before it is reset */
> static void test_func_initial(void)
> {
> @@ -319,6 +329,7 @@ int main(void)
> test_store_status();
> test_ecall();
> test_emcall();
> + test_sense_running();
> test_reset();
> test_reset_initial();
> smp_cpu_destroy(1);
>
In kvm, we set/clear via kvm_arch_vcpu_load/kvm_arch_vcpu_put. This
means, that a VCPU will also be indicated as running, in case we perform
certain VCPU IOCTLs, while the VCPU is already stopped.
Especially, there is a theoretical race between stopping a VCPU, and it
still being in the kernel, and the other thread sensing the running
status. This is the case with !kvm_s390_user_cpu_state_ctrl(), when
leaving handle_stop() but also with kvm_s390_user_cpu_state_ctrl(), when
setting the MP state via kvm_arch_vcpu_ioctl_set_mpstate().
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests 1/2] s390x/smp: fix detection of "running"
2020-03-30 8:49 ` [kvm-unit-tests 1/2] s390x/smp: fix detection of "running" Christian Borntraeger
2020-03-30 9:03 ` David Hildenbrand
2020-03-30 9:17 ` Janosch Frank
@ 2020-03-31 6:49 ` Cornelia Huck
2020-03-31 8:50 ` David Hildenbrand
2 siblings, 1 reply; 9+ messages in thread
From: Cornelia Huck @ 2020-03-31 6:49 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Janosch Frank, KVM, David Hildenbrand, linux-s390, Thomas Huth
On Mon, 30 Mar 2020 04:49:10 -0400
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On s390x hosts with a single CPU, the smp test case hangs (loops).
> The check is our restart has finished is wrong.
s/is/if/
> Sigp sense running status checks if the CPU is currently backed by a
> real CPU. This means that on single CPU hosts a sigp sense running
> will never claim that a target is running. We need to check for not
> being stopped instead.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> lib/s390x/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 2555bf4..5ed8b7b 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -128,7 +128,7 @@ static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
> * The order has been accepted, but the actual restart may not
> * have been performed yet, so wait until the cpu is running.
> */
> - while (!smp_cpu_running(addr))
> + while (smp_cpu_stopped(addr))
> mb();
> cpu->active = true;
> return 0;
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests 1/2] s390x/smp: fix detection of "running"
2020-03-31 6:49 ` Cornelia Huck
@ 2020-03-31 8:50 ` David Hildenbrand
0 siblings, 0 replies; 9+ messages in thread
From: David Hildenbrand @ 2020-03-31 8:50 UTC (permalink / raw)
To: Cornelia Huck, Christian Borntraeger
Cc: Janosch Frank, KVM, linux-s390, Thomas Huth
On 31.03.20 08:49, Cornelia Huck wrote:
> On Mon, 30 Mar 2020 04:49:10 -0400
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> On s390x hosts with a single CPU, the smp test case hangs (loops).
>> The check is our restart has finished is wrong.
>
> s/is/if/
With that fixed up queued to
https://github.com/davidhildenbrand/kvm-unit-tests.git s390x-next
I'll wait with the other patch until we have a consent there regarding
possible races.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status
2020-03-30 10:58 ` David Hildenbrand
@ 2020-04-02 10:44 ` Christian Borntraeger
0 siblings, 0 replies; 9+ messages in thread
From: Christian Borntraeger @ 2020-04-02 10:44 UTC (permalink / raw)
To: David Hildenbrand, Janosch Frank
Cc: KVM, linux-s390, Cornelia Huck, Thomas Huth
On 30.03.20 12:58, David Hildenbrand wrote:
> On 30.03.20 10:49, Christian Borntraeger wrote:
>> make sure that sigp sense running status returns a sane value for
>> stopped CPUs.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> lib/s390x/smp.c | 2 +-
>> lib/s390x/smp.h | 2 +-
>> s390x/smp.c | 11 +++++++++++
>> 3 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>> index 5ed8b7b..492cb05 100644
>> --- a/lib/s390x/smp.c
>> +++ b/lib/s390x/smp.c
>> @@ -58,7 +58,7 @@ bool smp_cpu_stopped(uint16_t addr)
>> return !!(status & (SIGP_STATUS_CHECK_STOP|SIGP_STATUS_STOPPED));
>> }
>>
>> -bool smp_cpu_running(uint16_t addr)
>> +bool smp_sense_running_status(uint16_t addr)
>> {
>> if (sigp(addr, SIGP_SENSE_RUNNING, 0, NULL) != SIGP_CC_STATUS_STORED)
>> return true;
>> diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
>> index a8b98c0..639ec92 100644
>> --- a/lib/s390x/smp.h
>> +++ b/lib/s390x/smp.h
>> @@ -40,7 +40,7 @@ struct cpu_status {
>> int smp_query_num_cpus(void);
>> struct cpu *smp_cpu_from_addr(uint16_t addr);
>> bool smp_cpu_stopped(uint16_t addr);
>> -bool smp_cpu_running(uint16_t addr);
>> +bool smp_sense_running_status(uint16_t addr);
>> int smp_cpu_restart(uint16_t addr);
>> int smp_cpu_start(uint16_t addr, struct psw psw);
>> int smp_cpu_stop(uint16_t addr);
>> diff --git a/s390x/smp.c b/s390x/smp.c
>> index 79cdc1f..f9f143d 100644
>> --- a/s390x/smp.c
>> +++ b/s390x/smp.c
>> @@ -210,6 +210,16 @@ static void test_emcall(void)
>> report_prefix_pop();
>> }
>>
>> +static void test_sense_running(void)
>> +{
>> + report_prefix_push("sense_running");
>> + /* make sure CPU is stopped */
>> + smp_cpu_stop(1);
>> + report(!smp_sense_running_status(1), "CPU1 sense claims not running");
>> + report_prefix_pop();
>> +}
>> +
>> +
>> /* Used to dirty registers of cpu #1 before it is reset */
>> static void test_func_initial(void)
>> {
>> @@ -319,6 +329,7 @@ int main(void)
>> test_store_status();
>> test_ecall();
>> test_emcall();
>> + test_sense_running();
>> test_reset();
>> test_reset_initial();
>> smp_cpu_destroy(1);
>>
>
> In kvm, we set/clear via kvm_arch_vcpu_load/kvm_arch_vcpu_put. This
> means, that a VCPU will also be indicated as running, in case we perform
> certain VCPU IOCTLs, while the VCPU is already stopped.
>
> Especially, there is a theoretical race between stopping a VCPU, and it
> still being in the kernel, and the other thread sensing the running
> status. This is the case with !kvm_s390_user_cpu_state_ctrl(), when
> leaving handle_stop() but also with kvm_s390_user_cpu_state_ctrl(), when
> setting the MP state via kvm_arch_vcpu_ioctl_set_mpstate().
I think I will just busy wait until the target is not running. This must happen and a timeout indicates a failure.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-04-02 10:44 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 8:49 [kvm-unit-tests 0/2] s390x/smp fix and enhancement Christian Borntraeger
2020-03-30 8:49 ` [kvm-unit-tests 1/2] s390x/smp: fix detection of "running" Christian Borntraeger
2020-03-30 9:03 ` David Hildenbrand
2020-03-30 9:17 ` Janosch Frank
2020-03-31 6:49 ` Cornelia Huck
2020-03-31 8:50 ` David Hildenbrand
2020-03-30 8:49 ` [kvm-unit-tests 2/2] s390x/smp: add minimal test for sigp sense running status Christian Borntraeger
2020-03-30 10:58 ` David Hildenbrand
2020-04-02 10:44 ` Christian Borntraeger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).