Re: [PATCH] xen: fix logical error in tlb flushing
On Wed, Sep 05, 2012 at 05:41:37AM +, Ren, Yongjie wrote: > > -Original Message- > > From: Shi, Alex > > Sent: Wednesday, September 05, 2012 1:35 PM > > To: Jan Beulich > > Cc: Konrad Rzeszutek Wilk; t...@linutronix.de; mi...@redhat.com; > > linux-kernel@vger.kernel.org; h...@zytor.com; Ren, Yongjie > > Subject: Re: [PATCH] xen: fix logical error in tlb flushing > > > > On 08/25/2012 03:45 AM, Jan Beulich wrote: > > > > >>>> On 24.08.12 at 20:17, Konrad Rzeszutek Wilk > > wrote: > > >> On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: > > >>>>>> On 24.08.12 at 10:55, Alex Shi wrote: > > >>>> While TLB_FLUSH_ALL gets passed as 'end' argument to > > >>>> flush_tlb_others(), the Xen code was made to check its 'start' > > >>>> parameter. That may give a incorrect op.cmd to > > MMUEXT_INVLPG_MULTI > > >>>> instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page > > can not > > >>>> be flushed from TLB. > > >>>> > > >>>> This patch fixed this issue. > > >>>> > > >>>> Reported-by: Jan Beulich > > >>>> Signed-off-by: Alex Shi > > >>> > > >>> Acked-by: Jan Beulich > > > Tested-by: Yongjie Ren > > > > > CC to Yongjie, > > Could you like to test this patch on PV guest > > > Xen Dom0 and a PV guest kernel with this patch can boot up correctly. OK, applied for v3.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
>>> On 05.09.12 at 07:34, Alex Shi wrote: > On 08/25/2012 03:45 AM, Jan Beulich wrote: > On 24.08.12 at 20:17, Konrad Rzeszutek Wilk > wrote: >>> How can I reproduce this >> >> I don't know, I spotted this while looking at the code. > > Again, since the old buggy code doesn't cause trouble in PV guest, guess > the hypercall for MMUEXT_INVLPG_MULTI was translated or treated as > MMUEXT_TLB_FLUSH_MULTI. If so, believe correct this will bring a big > performance benefit. It's not clear to me what was buggy with the code prior to your change. And no, there's no magic widening of the scope of these MMU operations - if you ask the hypervisor for a single page invalidation, that's what it's going to do. But of course, there are cases where extra (full) invalidations need to be done without a guest asking for them. But that's nothing a guest can validly make itself dependent upon. Jan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On 05.09.12 at 07:34, Alex Shi alex@intel.com wrote: On 08/25/2012 03:45 AM, Jan Beulich wrote: On 24.08.12 at 20:17, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: How can I reproduce this I don't know, I spotted this while looking at the code. Again, since the old buggy code doesn't cause trouble in PV guest, guess the hypercall for MMUEXT_INVLPG_MULTI was translated or treated as MMUEXT_TLB_FLUSH_MULTI. If so, believe correct this will bring a big performance benefit. It's not clear to me what was buggy with the code prior to your change. And no, there's no magic widening of the scope of these MMU operations - if you ask the hypervisor for a single page invalidation, that's what it's going to do. But of course, there are cases where extra (full) invalidations need to be done without a guest asking for them. But that's nothing a guest can validly make itself dependent upon. Jan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On Wed, Sep 05, 2012 at 05:41:37AM +, Ren, Yongjie wrote: -Original Message- From: Shi, Alex Sent: Wednesday, September 05, 2012 1:35 PM To: Jan Beulich Cc: Konrad Rzeszutek Wilk; t...@linutronix.de; mi...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com; Ren, Yongjie Subject: Re: [PATCH] xen: fix logical error in tlb flushing On 08/25/2012 03:45 AM, Jan Beulich wrote: On 24.08.12 at 20:17, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: On 24.08.12 at 10:55, Alex Shi alex@intel.com wrote: While TLB_FLUSH_ALL gets passed as 'end' argument to flush_tlb_others(), the Xen code was made to check its 'start' parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not be flushed from TLB. This patch fixed this issue. Reported-by: Jan Beulich jbeul...@suse.com Signed-off-by: Alex Shi alex@intel.com Acked-by: Jan Beulich jbeul...@suse.com Tested-by: Yongjie Ren yongjie@intel.com CC to Yongjie, Could you like to test this patch on PV guest Xen Dom0 and a PV guest kernel with this patch can boot up correctly. OK, applied for v3.6 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] xen: fix logical error in tlb flushing
> -Original Message- > From: Shi, Alex > Sent: Wednesday, September 05, 2012 1:35 PM > To: Jan Beulich > Cc: Konrad Rzeszutek Wilk; t...@linutronix.de; mi...@redhat.com; > linux-kernel@vger.kernel.org; h...@zytor.com; Ren, Yongjie > Subject: Re: [PATCH] xen: fix logical error in tlb flushing > > On 08/25/2012 03:45 AM, Jan Beulich wrote: > > >>>> On 24.08.12 at 20:17, Konrad Rzeszutek Wilk > wrote: > >> On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: > >>>>>> On 24.08.12 at 10:55, Alex Shi wrote: > >>>> While TLB_FLUSH_ALL gets passed as 'end' argument to > >>>> flush_tlb_others(), the Xen code was made to check its 'start' > >>>> parameter. That may give a incorrect op.cmd to > MMUEXT_INVLPG_MULTI > >>>> instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page > can not > >>>> be flushed from TLB. > >>>> > >>>> This patch fixed this issue. > >>>> > >>>> Reported-by: Jan Beulich > >>>> Signed-off-by: Alex Shi > >>> > >>> Acked-by: Jan Beulich > Tested-by: Yongjie Ren > > CC to Yongjie, > Could you like to test this patch on PV guest > Xen Dom0 and a PV guest kernel with this patch can boot up correctly. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On 08/25/2012 03:45 AM, Jan Beulich wrote: On 24.08.12 at 20:17, Konrad Rzeszutek Wilk wrote: >> On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: >> On 24.08.12 at 10:55, Alex Shi wrote: While TLB_FLUSH_ALL gets passed as 'end' argument to flush_tlb_others(), the Xen code was made to check its 'start' parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not be flushed from TLB. This patch fixed this issue. Reported-by: Jan Beulich Signed-off-by: Alex Shi >>> >>> Acked-by: Jan Beulich CC to Yongjie, Could you like to test this patch on PV guest >> >> How can I reproduce this > > I don't know, I spotted this while looking at the code. Again, since the old buggy code doesn't cause trouble in PV guest, guess the hypercall for MMUEXT_INVLPG_MULTI was translated or treated as MMUEXT_TLB_FLUSH_MULTI. If so, believe correct this will bring a big performance benefit. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On 08/25/2012 03:45 AM, Jan Beulich wrote: On 24.08.12 at 20:17, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: On 24.08.12 at 10:55, Alex Shi alex@intel.com wrote: While TLB_FLUSH_ALL gets passed as 'end' argument to flush_tlb_others(), the Xen code was made to check its 'start' parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not be flushed from TLB. This patch fixed this issue. Reported-by: Jan Beulich jbeul...@suse.com Signed-off-by: Alex Shi alex@intel.com Acked-by: Jan Beulich jbeul...@suse.com CC to Yongjie, Could you like to test this patch on PV guest How can I reproduce this I don't know, I spotted this while looking at the code. Again, since the old buggy code doesn't cause trouble in PV guest, guess the hypercall for MMUEXT_INVLPG_MULTI was translated or treated as MMUEXT_TLB_FLUSH_MULTI. If so, believe correct this will bring a big performance benefit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] xen: fix logical error in tlb flushing
-Original Message- From: Shi, Alex Sent: Wednesday, September 05, 2012 1:35 PM To: Jan Beulich Cc: Konrad Rzeszutek Wilk; t...@linutronix.de; mi...@redhat.com; linux-kernel@vger.kernel.org; h...@zytor.com; Ren, Yongjie Subject: Re: [PATCH] xen: fix logical error in tlb flushing On 08/25/2012 03:45 AM, Jan Beulich wrote: On 24.08.12 at 20:17, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: On 24.08.12 at 10:55, Alex Shi alex@intel.com wrote: While TLB_FLUSH_ALL gets passed as 'end' argument to flush_tlb_others(), the Xen code was made to check its 'start' parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not be flushed from TLB. This patch fixed this issue. Reported-by: Jan Beulich jbeul...@suse.com Signed-off-by: Alex Shi alex@intel.com Acked-by: Jan Beulich jbeul...@suse.com Tested-by: Yongjie Ren yongjie@intel.com CC to Yongjie, Could you like to test this patch on PV guest Xen Dom0 and a PV guest kernel with this patch can boot up correctly. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
>>> On 24.08.12 at 20:17, Konrad Rzeszutek Wilk wrote: > On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: >> >>> On 24.08.12 at 10:55, Alex Shi wrote: >> > While TLB_FLUSH_ALL gets passed as 'end' argument to >> > flush_tlb_others(), the Xen code was made to check its 'start' >> > parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI >> > instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not >> > be flushed from TLB. >> > >> > This patch fixed this issue. >> > >> > Reported-by: Jan Beulich >> > Signed-off-by: Alex Shi >> >> Acked-by: Jan Beulich > > How can I reproduce this I don't know, I spotted this while looking at the code. > and should this also go to sta...@kernel.org? No, the problem got introduced in 3.6-rc1. Jan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote:
> >>> On 24.08.12 at 10:55, Alex Shi wrote:
> > While TLB_FLUSH_ALL gets passed as 'end' argument to
> > flush_tlb_others(), the Xen code was made to check its 'start'
> > parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI
> > instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not
> > be flushed from TLB.
> >
> > This patch fixed this issue.
> >
> > Reported-by: Jan Beulich
> > Signed-off-by: Alex Shi
>
> Acked-by: Jan Beulich
How can I reproduce this and should this also go to sta...@kernel.org?
>
> > ---
> > arch/x86/xen/mmu.c |2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> > index b65a761..5141d80 100644
> > --- a/arch/x86/xen/mmu.c
> > +++ b/arch/x86/xen/mmu.c
> > @@ -1283,7 +1283,7 @@ static void xen_flush_tlb_others(const struct cpumask
> > *cpus,
> > cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));
> >
> > args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
> > - if (start != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
> > + if (end != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
> > args->op.cmd = MMUEXT_INVLPG_MULTI;
> > args->op.arg1.linear_addr = start;
> > }
> > --
> > 1.7.5.4
> >
> > .
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
>>> On 24.08.12 at 10:55, Alex Shi wrote:
> While TLB_FLUSH_ALL gets passed as 'end' argument to
> flush_tlb_others(), the Xen code was made to check its 'start'
> parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI
> instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not
> be flushed from TLB.
>
> This patch fixed this issue.
>
> Reported-by: Jan Beulich
> Signed-off-by: Alex Shi
Acked-by: Jan Beulich
> ---
> arch/x86/xen/mmu.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index b65a761..5141d80 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -1283,7 +1283,7 @@ static void xen_flush_tlb_others(const struct cpumask
> *cpus,
> cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));
>
> args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
> - if (start != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
> + if (end != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
> args->op.cmd = MMUEXT_INVLPG_MULTI;
> args->op.arg1.linear_addr = start;
> }
> --
> 1.7.5.4
>
> .
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On 24.08.12 at 10:55, Alex Shi alex@intel.com wrote:
While TLB_FLUSH_ALL gets passed as 'end' argument to
flush_tlb_others(), the Xen code was made to check its 'start'
parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI
instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not
be flushed from TLB.
This patch fixed this issue.
Reported-by: Jan Beulich jbeul...@suse.com
Signed-off-by: Alex Shi alex@intel.com
Acked-by: Jan Beulich jbeul...@suse.com
---
arch/x86/xen/mmu.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index b65a761..5141d80 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1283,7 +1283,7 @@ static void xen_flush_tlb_others(const struct cpumask
*cpus,
cpumask_clear_cpu(smp_processor_id(), to_cpumask(args-mask));
args-op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- if (start != TLB_FLUSH_ALL (end - start) = PAGE_SIZE) {
+ if (end != TLB_FLUSH_ALL (end - start) = PAGE_SIZE) {
args-op.cmd = MMUEXT_INVLPG_MULTI;
args-op.arg1.linear_addr = start;
}
--
1.7.5.4
.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote:
On 24.08.12 at 10:55, Alex Shi alex@intel.com wrote:
While TLB_FLUSH_ALL gets passed as 'end' argument to
flush_tlb_others(), the Xen code was made to check its 'start'
parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI
instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not
be flushed from TLB.
This patch fixed this issue.
Reported-by: Jan Beulich jbeul...@suse.com
Signed-off-by: Alex Shi alex@intel.com
Acked-by: Jan Beulich jbeul...@suse.com
How can I reproduce this and should this also go to sta...@kernel.org?
---
arch/x86/xen/mmu.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index b65a761..5141d80 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1283,7 +1283,7 @@ static void xen_flush_tlb_others(const struct cpumask
*cpus,
cpumask_clear_cpu(smp_processor_id(), to_cpumask(args-mask));
args-op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- if (start != TLB_FLUSH_ALL (end - start) = PAGE_SIZE) {
+ if (end != TLB_FLUSH_ALL (end - start) = PAGE_SIZE) {
args-op.cmd = MMUEXT_INVLPG_MULTI;
args-op.arg1.linear_addr = start;
}
--
1.7.5.4
.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xen: fix logical error in tlb flushing
On 24.08.12 at 20:17, Konrad Rzeszutek Wilk konrad.w...@oracle.com wrote: On Fri, Aug 24, 2012 at 04:16:39PM +0100, Jan Beulich wrote: On 24.08.12 at 10:55, Alex Shi alex@intel.com wrote: While TLB_FLUSH_ALL gets passed as 'end' argument to flush_tlb_others(), the Xen code was made to check its 'start' parameter. That may give a incorrect op.cmd to MMUEXT_INVLPG_MULTI instead of MMUEXT_TLB_FLUSH_MULTI. Then it causes some page can not be flushed from TLB. This patch fixed this issue. Reported-by: Jan Beulich jbeul...@suse.com Signed-off-by: Alex Shi alex@intel.com Acked-by: Jan Beulich jbeul...@suse.com How can I reproduce this I don't know, I spotted this while looking at the code. and should this also go to sta...@kernel.org? No, the problem got introduced in 3.6-rc1. Jan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
