lkml.org 
[lkml]   [2007]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] VMI paravirt-ops bugfix for 2.6.21
    Zachary Amsden wrote:
    > Critical bugfix; when using software RAID, potentially USB or AIO in
    > highmem configurations, drivers are allowed to use kmap_atomic from
    > interrupt context. This is incompatible with the current implementation
    > of lazy MMU mode, and means the kmap will silently fail, causing either
    > memory corruption or kernel panics. This bug is only visible with >970
    > megs of RAM and extreme memory pressure, but nontheless extremely serious.
    >

    Why's that? Don't some things get preferentially allocated out of highmem?

    > The fix is to disable interrupts on the CPU when entering a lazy MMU
    > state; this is totally safe, as preemption is already disabled, and
    > lazy update state can neither be nested nor overlapping. Thus per-cpu
    > variables to track the state and flags can be used to disable interrupts
    > during this critical region.
    >
    > Signed-off-by: Zachary Amsden <zach@vmware.com>
    >
    > diff -r be8c61492e28 arch/i386/kernel/vmi.c
    > --- a/arch/i386/kernel/vmi.c Fri Mar 30 14:13:45 2007 -0700
    > +++ b/arch/i386/kernel/vmi.c Fri Mar 30 14:18:16 2007 -0700
    > @@ -69,6 +69,7 @@ struct {
    > void (*flush_tlb)(int);
    > void (*set_initial_ap_state)(int, int);
    > void (*halt)(void);
    > + void (*set_lazy_mode)(int mode);
    > } vmi_ops;
    >
    > /* XXX move this to alternative.h */
    > @@ -574,6 +575,31 @@ vmi_startup_ipi_hook(int phys_apicid, un
    > }
    > #endif
    >
    > +static void vmi_set_lazy_mode(int new_mode)
    > +{
    > + static DEFINE_PER_CPU(int, mode);
    > + static DEFINE_PER_CPU(unsigned long, flags);
    > + int cpu = smp_processor_id();
    > +
    > + if (!vmi_ops.set_lazy_mode)
    > + return;
    > +
    > + /*
    > + * Modes do not nest or overlap, so we can simply disable
    > + * irqs when entering a mode and re-enable when leaving.
    > + */
    > + BUG_ON(per_cpu(mode, cpu) && new_mode);
    > + BUG_ON(!new_mode && !per_cpu(mode, cpu));
    >

    It's probably better to use __get_cpu_var() rather than per_cpu(X,
    smp_processor_id()); It's a real pity there's no ^^ operator...

    > +
    > + if (new_mode)
    > + local_irq_save(per_cpu(flags, cpu));
    > + else
    > + local_irq_restore(per_cpu(flags, cpu));
    >

    The comment only talks about disabling interrupts for lazy_mmu, but this
    seems to do it for lazy_cpu as well. Is that OK? What happens if
    someone wants to change interrupt states under lazy_cpu; I can't think
    of an inherent reason why that wouldn't be allowed (though I don't think
    it happens now).

    This kind of logic is a bit clunky anyway; would it be better to simply
    have separate enable/disable functions? Or at least separate functions
    per mode?

    Anyway,

    Acked-by: Jeremy Fitzhardinge <jeremy@xensource.com>

    J
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-03-31 10:13    [W:0.028 / U:30.960 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site