Marko Kohtala wrote: > On Nov 21, 2007 8:56 AM, Avi Kivity <[EMAIL PROTECTED]> wrote: > >> Marko Kohtala wrote: >> >>> Wait for right amount of tlb flushes. Completed can be larger than >>> needed and therefore the loop waiting them to match never ends. >>> >>> Signed-off-by: Marko Kohtala <[EMAIL PROTECTED]> >>> >>> --- >>> >>> This solves kernel lockup in KVM_SET_MEMORY_REGION ioctl with Linux >>> 2.6.23.8 and before at kvm-52 start. Not needed in 2.6.24. >>> >>> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c >>> index cd05579..b148aff 100644 >>> --- a/drivers/kvm/kvm_main.c >>> +++ b/drivers/kvm/kvm_main.c >>> @@ -279,7 +279,8 @@ void kvm_flush_remote_tlbs(struct kvm *kvm) >>> * to complete. >>> */ >>> for (cpu = first_cpu(cpus); cpu != NR_CPUS; cpu = next_cpu(cpu, cpus)) >>> - smp_call_function_single(cpu, ack_flush, &completed, 1, 0); >>> + if (cpu_isset(cpu, cpus)) >>> + smp_call_function_single(cpu, ack_flush, &completed, >>> 1, 0); >>> >>> >> Can you explain how this makes a difference? Aren't first_cpu() and >> next_cpu() designed to iterate over all cpus which have cpu_isset(cpus)? >> > > Look in linux/cpumask.h: > > 215 #ifdef CONFIG_SMP > 216 int __first_cpu(const cpumask_t *srcp); > 217 #define first_cpu(src) __first_cpu(&(src)) > 218 int __next_cpu(int n, const cpumask_t *srcp); > 219 #define next_cpu(n, src) __next_cpu((n), &(src)) > 220 #else > 221 #define first_cpu(src) 0 > 222 #define next_cpu(n, src) 1 > 223 #endif > > I have uniprocessor kernel, so first_cpu always gives 0, never NR_CPUS. > > I quickly looked through all uses of first_cpu, and in all other > places there is at least one CPU to iterate over. So cpumask.h is not > really wrong. It is just confusing. But being confusing is wrong, so > there may be some room for discussion. > > Another fix to this issue could be to not enter the loop at all if > cpus_empty(cpus). > >
Or maybe, change first_cpu() to return !!cpus_empty() on uniprocessor. Though that's not for 2.6.23. I think terminating early on cpus_empty() in kvm_flush_remote_tlb() is fine. -- Any sufficiently difficult bug is indistinguishable from a feature. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ kvm-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/kvm-devel
