lkml.org 
[lkml]   [2008]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC] x86: merge nmi_32-64 to nmi.c
    [Thomas Gleixner - Sun, May 18, 2008 at 12:34:15AM +0200]
    | On Sat, 17 May 2008, Mikael Pettersson wrote:
    | > Maciej W. Rozycki writes:
    | > > On Sat, 17 May 2008, Tom Spink wrote:
    | > >
    | > > > static inline unsigned int get_nmi_count(int cpu)
    | > > > {
    | > > > #ifdef CONFIG_X86_64
    | > > > return cpu_pda(cpu)->__nmi_count;
    | > > > #else
    | > > > return nmi_count(cpu);
    | > > > #endif
    | > > > }
    | > > >
    | > > > I know it introduces a lot of these conditionals, but at least there
    | > > > is one place to look for the get_nmi_count function, instead of
    | > > > searching for all variants of the function.
    | > >
    | > > Well, I suppose some header should provide a definition like:
    | > >
    | > > #ifdef CONFIG_X86_64
    | > > #define cpu_x86_64 1
    | > > #else
    | > > #define cpu_x86_64 0
    | > > #endif
    | > >
    | > > and the you can remove the horrible #ifdef clutter and make the quoted
    | > > function look like:
    | > >
    | > > static inline unsigned int get_nmi_count(int cpu)
    | > > {
    | > > return cpu_x86_64 ? cpu_pda(cpu)->__nmi_count : nmi_count(cpu);
    | > > }
    | > >
    | > > Much better -- isn't it?
    | >
    | > IMO, no, the #ifdef is preferable.
    | >
    | > Why? Because the #ifdef is a very visible signal to the platform
    | > people that there are (in this case) subarch differences that force
    | > "clients" to behave differently on different subarchs. By removing
    | > the #ifdef you're IMO making it less likely for the platform people
    | > to take notice and work towards eliminating those differences.
    |
    | The #ifdef is a poor choice. Maciej is damned right, that the single
    | function with a clear distinction of the return value is better in
    | terms of readability and maintenance.
    |
    | As I said before, We can make this more visible with an uppercase
    | CONFIG_WHATEVER instaed of the innocent cpu_x86_64 one, but both
    | solutions are better than #ifdefs and provide simple grepable
    | patterns.
    |
    | The awareness of those differences does not depend at all on an
    | #ifdef. Developers who are aware of the platform differences prefer a
    | readable not ifdef poluted code base. People who need to be poked into
    | the difference via an #ifdef are probably not those who can actually
    | clean it up.
    |
    | Thanks,
    |
    | tglx
    |

    Eventually these helpers could look like this, objections?
    ---

    static inline unsigned int get_nmi_count(int cpu)
    {
    #ifdef CONFIG_X86_64
    return cpu_pda(cpu)->__nmi_count;
    #else
    return nmi_count(cpu);
    #endif
    }

    static inline void __die_nmi(char *str, struct pt_regs *regs, int do_panic)
    {
    #ifdef CONFIG_X86_64
    die_nmi(str, regs, do_panic);
    #else
    die_nmi(regs, str);
    #endif
    }

    static inline int mce_in_progress(void)
    {
    #if defined(CONFIX_X86_64) && defined(CONFIG_X86_MCE)
    return atomic_read(&mce_entry) > 0;
    #endif
    return 0;
    }
    ---

    they are pretty ugly anyway ;)

    - Cyrill -


    \
     
     \ /
      Last update: 2008-05-18 12:07    [W:0.033 / U:30.088 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site