lkml.org 
[lkml]   [2006]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] lockdep: annotate i386 apm
    From
    Date
    On Wed, 2006-10-11 at 14:18 -0700, Andrew Morton wrote:
    > On Wed, 11 Oct 2006 15:40:22 +0200
    > Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
    >
    > >
    > > Lockdep doesn't like to enable interrupts when they are enabled already.
    > >
    > > ...
    > >
    > > --- linux-2.6.18.noarch.orig/arch/i386/kernel/apm.c
    > > +++ linux-2.6.18.noarch/arch/i386/kernel/apm.c
    > > @@ -539,11 +539,22 @@ static inline void apm_restore_cpus(cpum
    > > * Also, we KNOW that for the non error case of apm_bios_call, there
    > > * is no useful data returned in the low order 8 bits of eax.
    > > */
    > > -#define APM_DO_CLI \
    > > - if (apm_info.allow_ints) \
    > > - local_irq_enable(); \
    > > - else \
    > > - local_irq_disable();
    > > +#define APM_DO_CLI \
    > > + do { \
    > > + if (apm_info.allow_ints) { \
    > > + if (irqs_disabled_flags(flags)) \
    > > + local_irq_enable(); \
    > > + } else \
    > > + local_irq_disable(); \
    > > + } while (0)
    > > +
    > > +#define APM_DO_STI \
    > > + do { \
    > > + if (irqs_disabled_flags(flags)) \
    > > + local_irq_disable(); \
    > > + else if (irqs_disabled()) \
    > > + local_irq_enable(); \
    > > + } while (0)
    > >
    > > #ifdef APM_ZERO_SEGS
    > > # define APM_DECL_SEGS \
    > > @@ -600,7 +611,7 @@ static u8 apm_bios_call(u32 func, u32 eb
    > > APM_DO_SAVE_SEGS;
    > > apm_bios_call_asm(func, ebx_in, ecx_in, eax, ebx, ecx, edx, esi);
    > > APM_DO_RESTORE_SEGS;
    > > - local_irq_restore(flags);
    > > + APM_DO_STI;
    > > gdt[0x40 / 8] = save_desc_40;
    > > put_cpu();
    > > apm_restore_cpus(cpus);
    > > @@ -644,7 +655,7 @@ static u8 apm_bios_call_simple(u32 func,
    > > APM_DO_SAVE_SEGS;
    > > error = apm_bios_call_simple_asm(func, ebx_in, ecx_in, eax);
    > > APM_DO_RESTORE_SEGS;
    > > - local_irq_restore(flags);
    > > + APM_DO_STI;
    > > gdt[0x40 / 8] = save_desc_40;
    > > put_cpu();
    > > apm_restore_cpus(cpus);
    >
    > ick.
    >
    > Does this work?
    >
    > --- a/arch/i386/kernel/apm.c~lockdep-annotate-i386-apm
    > +++ a/arch/i386/kernel/apm.c
    > @@ -540,12 +540,6 @@ static inline void apm_restore_cpus(cpum
    > * Also, we KNOW that for the non error case of apm_bios_call, there
    > * is no useful data returned in the low order 8 bits of eax.
    > */
    > -#define APM_DO_CLI \
    > - if (apm_info.allow_ints) \
    > - local_irq_enable(); \
    > - else \
    > - local_irq_disable();
    > -
    > #ifdef APM_ZERO_SEGS
    > # define APM_DECL_SEGS \
    > unsigned int saved_fs; unsigned int saved_gs;
    > @@ -596,8 +590,9 @@ static u8 apm_bios_call(u32 func, u32 eb
    > save_desc_40 = gdt[0x40 / 8];
    > gdt[0x40 / 8] = bad_bios_desc;
    >
    > - local_save_flags(flags);
    > - APM_DO_CLI;
    > + local_irq_save(flags);
    > + if (apm_info.allow_ints)
    > + local_irq_enable();
    > APM_DO_SAVE_SEGS;
    > apm_bios_call_asm(func, ebx_in, ecx_in, eax, ebx, ecx, edx, esi);
    > APM_DO_RESTORE_SEGS;
    > @@ -640,8 +635,9 @@ static u8 apm_bios_call_simple(u32 func,
    > save_desc_40 = gdt[0x40 / 8];
    > gdt[0x40 / 8] = bad_bios_desc;
    >
    > - local_save_flags(flags);
    > - APM_DO_CLI;
    > + local_irq_save(flags);
    > + if (apm_info.allow_ints)
    > + local_irq_enable();
    > APM_DO_SAVE_SEGS;
    > error = apm_bios_call_simple_asm(func, ebx_in, ecx_in, eax);
    > APM_DO_RESTORE_SEGS;
    > _

    #define local_irq_restore(flags) \
    do { \
    if (raw_irqs_disabled_flags(flags)) { \
    raw_local_irq_restore(flags); \
    trace_hardirqs_off(); \
    } else { \
    trace_hardirqs_on(); \
    raw_local_irq_restore(flags); \
    } \
    } while (0)

    So, say interrupts were enabled when entering apm_bios_call*(); you now
    save that in flags, disable interrupts, and enable them again.
    Upon reaching local_irq_restore(), we'll hit the else branch with irq's
    enabled and call trace_hardirqs_on(), which goes EEEK!




    -
    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: 2006-10-12 08:09    [W:0.030 / U:22.528 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site