<stefano.stabellini@eu.citrix.com>
2011-Sep-06 16:41 UTC
[Xen-devel] [PATCH] xen: disable PV spinlocks on HVM
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> PV spinlocks cannot possibly work with the current code because they are enabled after pvops patching has already been done, and because PV spinlocks use a different data structure than native spinlocks so we cannot switch between them dynamically. A spinlock that has been taken once by the native code (__ticket_spin_lock) cannot be taken by __xen_spin_lock even after it has been released. Reported-by: Stefan Bader <stefan.bader@canonical.com> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- arch/x86/xen/smp.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c index e79dbb9..51339b4 100644 --- a/arch/x86/xen/smp.c +++ b/arch/x86/xen/smp.c @@ -522,7 +522,6 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus) WARN_ON(xen_smp_intr_init(0)); xen_init_lock_cpu(0); - xen_init_spinlocks(); } static int __cpuinit xen_hvm_cpu_up(unsigned int cpu) -- 1.7.2.3 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Sep-06 17:02 UTC
Re: [Xen-devel] [PATCH] xen: disable PV spinlocks on HVM
On Tue, Sep 06, 2011 at 05:41:47PM +0100, stefano.stabellini@eu.citrix.com wrote:> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > PV spinlocks cannot possibly work with the current code because they are > enabled after pvops patching has already been done, and because PV > spinlocks use a different data structure than native spinlocks so we > cannot switch between them dynamically. A spinlock that has been taken > once by the native code (__ticket_spin_lock) cannot be taken by > __xen_spin_lock even after it has been released.Let me stick it on my 3.1-rc5 bug-fix list and add stable@kernel.org to it. Stefan, if you have some time this week, and can test it - I can also stick ''Tested-by'' if you would like. Thinking to send the patches on Friday.> > Reported-by: Stefan Bader <stefan.bader@canonical.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > arch/x86/xen/smp.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c > index e79dbb9..51339b4 100644 > --- a/arch/x86/xen/smp.c > +++ b/arch/x86/xen/smp.c > @@ -522,7 +522,6 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus) > WARN_ON(xen_smp_intr_init(0)); > > xen_init_lock_cpu(0); > - xen_init_spinlocks(); > } > > static int __cpuinit xen_hvm_cpu_up(unsigned int cpu) > -- > 1.7.2.3 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefan Bader
2011-Sep-06 17:18 UTC
Re: [Xen-devel] [PATCH] xen: disable PV spinlocks on HVM
On 06.09.2011 10:02, Konrad Rzeszutek Wilk wrote:> On Tue, Sep 06, 2011 at 05:41:47PM +0100, stefano.stabellini@eu.citrix.com wrote: >> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> PV spinlocks cannot possibly work with the current code because they are >> enabled after pvops patching has already been done, and because PV >> spinlocks use a different data structure than native spinlocks so we >> cannot switch between them dynamically. A spinlock that has been taken >> once by the native code (__ticket_spin_lock) cannot be taken by >> __xen_spin_lock even after it has been released. > > Let me stick it on my 3.1-rc5 bug-fix list and add stable@kernel.org to it. > > Stefan, if you have some time this week, and can test it - I can also > stick ''Tested-by'' if you would like. > > Thinking to send the patches on Friday. > >> >> Reported-by: Stefan Bader <stefan.bader@canonical.com> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> --- >> arch/x86/xen/smp.c | 1 - >> 1 files changed, 0 insertions(+), 1 deletions(-) >> >> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c >> index e79dbb9..51339b4 100644 >> --- a/arch/x86/xen/smp.c >> +++ b/arch/x86/xen/smp.c >> @@ -522,7 +522,6 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus) >> WARN_ON(xen_smp_intr_init(0)); >> >> xen_init_lock_cpu(0); >> - xen_init_spinlocks(); >> } >> >> static int __cpuinit xen_hvm_cpu_up(unsigned int cpu) >> -- >> 1.7.2.3 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-develI''ll try. Currently I only got a version tested that takes out the xen_init_lock_cpu calls as well. -Stefan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefan Bader
2011-Sep-07 20:22 UTC
Re: [Xen-devel] [PATCH] xen: disable PV spinlocks on HVM
On 06.09.2011 10:02, Konrad Rzeszutek Wilk wrote:> On Tue, Sep 06, 2011 at 05:41:47PM +0100, stefano.stabellini@eu.citrix.com wrote: >> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> >> PV spinlocks cannot possibly work with the current code because they are >> enabled after pvops patching has already been done, and because PV >> spinlocks use a different data structure than native spinlocks so we >> cannot switch between them dynamically. A spinlock that has been taken >> once by the native code (__ticket_spin_lock) cannot be taken by >> __xen_spin_lock even after it has been released. > > Let me stick it on my 3.1-rc5 bug-fix list and add stable@kernel.org to it. > > Stefan, if you have some time this week, and can test it - I can also > stick ''Tested-by'' if you would like. > > Thinking to send the patches on Friday. > >> >> Reported-by: Stefan Bader <stefan.bader@canonical.com> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> --- >> arch/x86/xen/smp.c | 1 - >> 1 files changed, 0 insertions(+), 1 deletions(-) >> >> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c >> index e79dbb9..51339b4 100644 >> --- a/arch/x86/xen/smp.c >> +++ b/arch/x86/xen/smp.c >> @@ -522,7 +522,6 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus) >> WARN_ON(xen_smp_intr_init(0)); >> >> xen_init_lock_cpu(0); >> - xen_init_spinlocks(); >> } >> >> static int __cpuinit xen_hvm_cpu_up(unsigned int cpu) >> -- >> 1.7.2.3 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-develOk, tried with a kernel that only disables the init call and it does work the same. That said, something still is not completely right with the NIC device interrupts. Setup looks good and pings work. But doing any larger trnasfers seems to loose interrupts, resulting in tx queue being full and getting timeouts... -Stefan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2011-Sep-07 21:24 UTC
[Xen-devel] Re: [PATCH] xen: disable PV spinlocks on HVM
On 09/06/2011 09:41 AM, stefano.stabellini@eu.citrix.com wrote:> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > PV spinlocks cannot possibly work with the current code because they are > enabled after pvops patching has already been done, and because PV > spinlocks use a different data structure than native spinlocks so we > cannot switch between them dynamically. A spinlock that has been taken > once by the native code (__ticket_spin_lock) cannot be taken by > __xen_spin_lock even after it has been released. > > Reported-by: Stefan Bader <stefan.bader@canonical.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > arch/x86/xen/smp.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c > index e79dbb9..51339b4 100644 > --- a/arch/x86/xen/smp.c > +++ b/arch/x86/xen/smp.c > @@ -522,7 +522,6 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus) > WARN_ON(xen_smp_intr_init(0)); > > xen_init_lock_cpu(0); > - xen_init_spinlocks(); > }Should I add this back here on the pv ticketlock branch? J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Sep-08 12:45 UTC
[Xen-devel] Re: [PATCH] xen: disable PV spinlocks on HVM
On Wed, 7 Sep 2011, Jeremy Fitzhardinge wrote:> On 09/06/2011 09:41 AM, stefano.stabellini@eu.citrix.com wrote: > > From: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > PV spinlocks cannot possibly work with the current code because they are > > enabled after pvops patching has already been done, and because PV > > spinlocks use a different data structure than native spinlocks so we > > cannot switch between them dynamically. A spinlock that has been taken > > once by the native code (__ticket_spin_lock) cannot be taken by > > __xen_spin_lock even after it has been released. > > > > Reported-by: Stefan Bader <stefan.bader@canonical.com> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > arch/x86/xen/smp.c | 1 - > > 1 files changed, 0 insertions(+), 1 deletions(-) > > > > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c > > index e79dbb9..51339b4 100644 > > --- a/arch/x86/xen/smp.c > > +++ b/arch/x86/xen/smp.c > > @@ -522,7 +522,6 @@ static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus) > > WARN_ON(xen_smp_intr_init(0)); > > > > xen_init_lock_cpu(0); > > - xen_init_spinlocks(); > > } > > Should I add this back here on the pv ticketlock branch?I think that Konrad is going to pick it up and send it as an important bug fix, CC''ing stable too. However I also have another patch to enable PV spinlock on HVM again, that relies on your pv ticket lock work. This one should probably go at the end of your series (appended). --- xen: initialize PV spinlocks for HVM guests before patching is done Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c index 51339b4..e24e9a2 100644 --- a/arch/x86/xen/smp.c +++ b/arch/x86/xen/smp.c @@ -551,4 +551,5 @@ void __init xen_hvm_smp_init(void) smp_ops.cpu_die = xen_hvm_cpu_die; smp_ops.send_call_func_ipi = xen_smp_send_call_function_ipi; smp_ops.send_call_func_single_ipi = xen_smp_send_call_function_single_ipi; + xen_init_spinlocks(); } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel