| Message ID | 1436189485-21443-2-git-send-email-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/07/2015 15:31, Andrew Jones wrote: > + if (argc && strcmp(argv[0], "non-atomic") != 0) { You want == here, don't you? The reason I switched was that the non-atomic test didn't work for me. I have now debugged it, and it needs this: @@ -29,12 +29,12 @@ static void gcc_builtin_unlock(int *lock_var) } static void none_lock(int *lock_var) { - while (*lock_var != 0); - *lock_var = 1; + while (*(volatile int *)lock_var != 0); + *(volatile int *)lock_var = 1; } static void none_unlock(int *lock_var) { - *lock_var = 0; + *(volatile int *)lock_var = 0; } static int global_a, global_b; Otherwise the none_lock function does not reload lock_var. Paolo > lock_ops.lock = none_lock; > lock_ops.unlock = none_unlock; > + } else { > + lock_ops.lock = gcc_builtin_lock; > + lock_ops.unlock = gcc_builtin_unlock; > } -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 06, 2015 at 07:51:40PM +0200, Paolo Bonzini wrote: > > > On 06/07/2015 15:31, Andrew Jones wrote: > > + if (argc && strcmp(argv[0], "non-atomic") != 0) { > > You want == here, don't you? Ah, now I see that your default was correct before. Sorry that I didn't looked closely enough to see that you flipped that logic. > > The reason I switched was that the non-atomic test didn't work for me. > I have now debugged it, and it needs this: > > @@ -29,12 +29,12 @@ static void gcc_builtin_unlock(int *lock_var) > } > static void none_lock(int *lock_var) > { > - while (*lock_var != 0); > - *lock_var = 1; > + while (*(volatile int *)lock_var != 0); > + *(volatile int *)lock_var = 1; > } > static void none_unlock(int *lock_var) > { > - *lock_var = 0; > + *(volatile int *)lock_var = 0; > } > > static int global_a, global_b; > > Otherwise the none_lock function does not reload lock_var. Good call. I'll send a v2 of this patch that puts this in. Thanks, drew > > Paolo > > > lock_ops.lock = none_lock; > > lock_ops.unlock = none_unlock; > > + } else { > > + lock_ops.lock = gcc_builtin_lock; > > + lock_ops.unlock = gcc_builtin_unlock; > > } > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/07/2015 13:24, Andrew Jones wrote: >> > The reason I switched was that the non-atomic test didn't work for me. >> > I have now debugged it, and it needs this: >> > >> > @@ -29,12 +29,12 @@ static void gcc_builtin_unlock(int *lock_var) >> > } >> > static void none_lock(int *lock_var) >> > { >> > - while (*lock_var != 0); >> > - *lock_var = 1; >> > + while (*(volatile int *)lock_var != 0); >> > + *(volatile int *)lock_var = 1; >> > } >> > static void none_unlock(int *lock_var) >> > { >> > - *lock_var = 0; >> > + *(volatile int *)lock_var = 0; >> > } >> > >> > static int global_a, global_b; >> > >> > Otherwise the none_lock function does not reload lock_var. > Good call. I'll send a v2 of this patch that puts this in. No need to, I have already applied this patch, and my fixup on top. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arm/spinlock-test.c b/arm/spinlock-test.c index 1dcf2b144a134..1f91edf4116a4 100644 --- a/arm/spinlock-test.c +++ b/arm/spinlock-test.c @@ -1,3 +1,11 @@ +/* + * Port of the spinlock torture test from virtualopensystems + * tcg_baremetal_tests + * + * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com> + * + * This work is licensed under the terms of the GNU LGPL, version 2. + */ #include <libcflat.h> #include <asm/smp.h> #include <asm/cpumask.h> @@ -69,12 +77,12 @@ int main(int argc, char **argv) { int cpu; - if (argc && strcmp(argv[0], "bad") != 0) { - lock_ops.lock = gcc_builtin_lock; - lock_ops.unlock = gcc_builtin_unlock; - } else { + if (argc && strcmp(argv[0], "non-atomic") != 0) { lock_ops.lock = none_lock; lock_ops.unlock = none_unlock; + } else { + lock_ops.lock = gcc_builtin_lock; + lock_ops.unlock = gcc_builtin_unlock; } for_each_present_cpu(cpu) { diff --git a/arm/unittests.cfg b/arm/unittests.cfg index ee655b2678a4e..9993d133bc394 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -35,3 +35,9 @@ file = selftest.flat smp = $(getconf _NPROCESSORS_CONF) extra_params = -append 'smp' groups = selftest + +# Test spinlocks +#[smp::spinlocks] +#file = spinlock-test.flat +#smp = $(getconf _NPROCESSORS_CONF) +#groups = smp
I hadn't plan to post this for kvm-unit-tests, but Paolo picked it up. That's fine, but now we should add its header. Also, Paolo tweaked the original to "use atomics by default", but it actually was using non-atomic by default. Switch to atomics, and rename the parameter from 'bad' (which is a bit ambiguous) to 'non-atomic'. And, let's give it its own place in unittests.cfg, but leave it disabled for now. Signed-off-by: Andrew Jones <drjones@redhat.com> --- arm/spinlock-test.c | 16 ++++++++++++---- arm/unittests.cfg | 6 ++++++ 2 files changed, 18 insertions(+), 4 deletions(-)