lkml.org 
[lkml]   [2007]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 18/19] move _set_gate and its users to a common location

    * 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


    \
     
     \ /
      Last update: 2007-12-22 03:15    [W:2.233 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site