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
> -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 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/
[PATCH] xen: fix logical error in tlb flushing
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
---
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/
