From: Ingo Molnar <mingo@elte.hu>
To: Glauber de Oliveira Costa <gcosta@redhat.com>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
glommer@gmail.com, tglx@linutronix.de, ehabkost@redhat.com,
jeremy@goop.org, avi@qumranet.com, anthony@codemonkey.ws,
virtualization@lists.linux-foundation.org, rusty@rustcorp.com.au,
ak@suse.de, chrisw@sous-sol.org, rostedt@goodmis.org,
hpa@zytor.com, zach@vmware.com, roland@redhat.com
Subject: Re: [PATCH 18/19] move _set_gate and its users to a common location
Date: Sat, 22 Dec 2007 03:07:00 +0100 [thread overview]
Message-ID: <20071222020700.GA7966@elte.hu> (raw)
In-Reply-To: <1197554404337-git-send-email-gcosta@redhat.com>
* Glauber de Oliveira Costa <gcosta@redhat.com> wrote:
> This patch moves _set_gate and its users to desc.h. We can now use
> common code for x86_64 and i386.
a few days ago i started seeing weird crashes on 64-bit x86 in the
random-kernel-bootup tests. Nothing truly reproducable to be bisectable,
but quality of x86.git went down drastically. Double faults, triple
faults, crashes all around the place, with every few dozen kernel
bootups.
Today i found a config and a workload that triggered the crashes a bit
more reliably. I still needed a 6 hours bisection marathon to pinpoint
the patch - the one in this thread. A review of it with H. Peter Anvin
pinpointed the breakage:
> +static inline void set_system_gate(unsigned int n, void *addr)
> +{
> + BUG_ON((unsigned)n > 0xFF);
> + _set_gate(n, GATE_TRAP, addr, 0x3, 0, __KERNEL_CS);
> +}
> -static inline void set_system_gate(int nr, void *func)
> -{
> - BUG_ON((unsigned)nr > 0xFF);
> - _set_gate(nr, GATE_INTERRUPT, (unsigned long) func, 3, 0);
> -}
you changed the type of system gates on 64-bit from GATE_INTERRUPT to
GATE_TRAP. The effect of this is that these gates enter with interrupts
enabled - instead of interrupts disabled. This, amongst others, affects
the following key gate:
set_system_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
which relies on disabled interrupts to fix up its stack. If an interrupt
comes in the wrong moment then we get a kernel stack corruption that is
not survivable.
the reason for this bug was that you tried to do too many changes in a
single patch. You did a cleanup, you did unification and you moved code
around. It was totally non-obvious what you did and the resulting patch
was not reviewable at all - even after the bisection poinpointed the
patch, it took us almost 30 minutes to figure out where the bug was. If
this unstructured, careless mess of patches continues then we are not
going to be able to accept any more 64-bit paravirt patches into
x86.git.
Ingo
next prev parent reply other threads:[~2007-12-22 2:13 UTC|newest]
Thread overview: 99+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-06 16:16 [PATCH 0/19] desc_struct integration Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 1/19] unify desc_struct Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 2/19] unify struct desc_ptr Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 3/19] change gdt acessor macro name Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 4/19] removed unused variable Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 5/19] introduce gate_desc type Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 6/19] change write_idt_entry signature Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 7/19] introduce ldt_desc type Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 8/19] modify write_ldt function Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 9/19] introduce fill_ldt Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 10/19] change write_gdt_entry signature Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 11/19] change write_ldt_entry signature Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 12/19] move constants to desc_defs.h Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 13/19] unify non-paravirt parts of desc.h Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 14/19] use the same data type for tls_array Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 15/19] modify get_desc_base Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 16/19] provide tss_desc Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 17/19] unify paravirt pieces of descriptor handling Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 18/19] move _set_gate and its users to a common location Glauber de Oliveira Costa
2007-12-06 16:16 ` [PATCH 19/19] unify set_tss_desc Glauber de Oliveira Costa
2007-12-06 19:24 ` [PATCH 1/19] unify desc_struct Jeremy Fitzhardinge
2007-12-06 19:37 ` Glauber de Oliveira Costa
2007-12-06 20:54 ` Andi Kleen
2007-12-06 21:20 ` Glauber de Oliveira Costa
2007-12-06 22:03 ` Jeremy Fitzhardinge
2007-12-12 12:53 ` [PATCH 0/19] desc_struct integration Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 01/19] unify desc_struct Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 02/19] unify struct desc_ptr Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 03/19] change gdt acessor macro name Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 04/19] removed unused variable Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 05/19] introduce gate_desc type Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 06/19] change write_idt_entry signature Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 07/19] introduce ldt_desc type Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 08/19] modify write_ldt function Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 09/19] introduce fill_ldt Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 10/19] provide tss_desc Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 11/19] change write_gdt_entry signature Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 12/19] change write_ldt_entry signature Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 13/19] move constants to desc_defs.h Glauber de Oliveira Costa
2007-12-12 12:53 ` [PATCH 14/19] unify non-paravirt parts of desc.h Glauber de Oliveira Costa
2007-12-12 12:54 ` [PATCH 15/19] use the same data type for tls_array Glauber de Oliveira Costa
2007-12-12 12:54 ` [PATCH 16/19] modify get_desc_base Glauber de Oliveira Costa
2007-12-12 12:54 ` [PATCH 17/19] unify paravirt pieces of descriptor handling Glauber de Oliveira Costa
2007-12-12 12:54 ` [PATCH 18/19] move _set_gate and its users to a common location Glauber de Oliveira Costa
2007-12-12 12:54 ` [PATCH 19/19] unify set_tss_desc Glauber de Oliveira Costa
2007-12-12 17:56 ` [PATCH 09/19] introduce fill_ldt Ingo Molnar
2007-12-12 17:20 ` [PATCH 0/19] desc_struct integration Ingo Molnar
2007-12-12 18:11 ` Ingo Molnar
2007-12-12 18:20 ` Ingo Molnar
2007-12-12 18:27 ` Glauber de Oliveira Costa
2007-12-12 18:33 ` Ingo Molnar
2007-12-12 19:05 ` Glauber de Oliveira Costa
2007-12-12 18:34 ` Ingo Molnar
2007-12-12 23:39 ` H. Peter Anvin
2007-12-13 2:01 ` [PATCH 0/19 - v3] " Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 01/19] unify desc_struct Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 02/19] unify struct desc_ptr Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 03/19] change gdt acessor macro name Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 04/19] removed unused variable Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 05/19] introduce gate_desc type Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 06/19] change write_idt_entry signature Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 07/19] introduce ldt_desc type Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 08/19] modify write_ldt function Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 09/19] introduce fill_ldt Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 10/19] provide tss_desc Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 11/19] change write_gdt_entry signature Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 12/19] change write_ldt_entry signature Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 13/19] move constants to desc_defs.h Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 14/19] unify non-paravirt parts of desc.h Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 15/19] use the same data type for tls_array Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 16/19] modify get_desc_base Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 17/19] unify paravirt pieces of descriptor handling Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 18/19] move _set_gate and its users to a common location Glauber de Oliveira Costa
2007-12-13 2:01 ` [PATCH 19/19] unify set_tss_desc Glauber de Oliveira Costa
2007-12-13 12:46 ` Andi Kleen
2007-12-13 14:50 ` Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 0/19 -v4] desc_struct integration Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 01/19] unify desc_struct Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 02/19] unify struct desc_ptr Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 03/19] change gdt acessor macro name Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 04/19] removed unused variable Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 05/19] introduce gate_desc type Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 06/19] change write_idt_entry signature Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 07/19] introduce ldt_desc type Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 08/19] modify write_ldt function Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 09/19] introduce fill_ldt Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 10/19] provide tss_desc Glauber de Oliveira Costa
2007-12-13 13:57 ` [PATCH 11/19] change write_gdt_entry signature Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 12/19] change write_ldt_entry signature Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 13/19] move constants to desc_defs.h Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 14/19] unify non-paravirt parts of desc.h Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 15/19] use the same data type for tls_array Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 16/19] modify get_desc_base Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 17/19] unify paravirt pieces of descriptor handling Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 18/19] move _set_gate and its users to a common location Glauber de Oliveira Costa
2007-12-13 13:58 ` [PATCH 19/19] unify set_tss_desc Glauber de Oliveira Costa
2007-12-22 2:07 ` Ingo Molnar [this message]
2007-12-13 16:47 ` [PATCH 0/19 -v4] desc_struct integration Ingo Molnar
2007-12-13 17:06 ` Glauber de Oliveira Costa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20071222020700.GA7966@elte.hu \
--to=mingo@elte.hu \
--cc=ak@suse.de \
--cc=akpm@linux-foundation.org \
--cc=anthony@codemonkey.ws \
--cc=avi@qumranet.com \
--cc=chrisw@sous-sol.org \
--cc=ehabkost@redhat.com \
--cc=gcosta@redhat.com \
--cc=glommer@gmail.com \
--cc=hpa@zytor.com \
--cc=jeremy@goop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=roland@redhat.com \
--cc=rostedt@goodmis.org \
--cc=rusty@rustcorp.com.au \
--cc=tglx@linutronix.de \
--cc=virtualization@lists.linux-foundation.org \
--cc=zach@vmware.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).