Re: PREEMPT_ACTIVE too low error with all asm-generic headers for some arches

[Mike Frysinger]

On Wed, Jun 24, 2009 at 09:13, Ingo Molnar wrote:
> * Mike Frysinger <vapier.adi@xxxxxxxxx> wrote:
>> after pulling the latest mainline code, Blackfin started hitting a
>> build failure like so:
>> Â CC Â Â Âarch/blackfin/kernel/asm-offsets.s
>> In file included from include/linux/interrupt.h:12,
>> Â Â Â Â Â Â Â Â Âfrom include/linux/kernel_stat.h:8,
>> Â Â Â Â Â Â Â Â Âfrom arch/blackfin/kernel/asm-offsets.c:32:
>> include/linux/hardirq.h:66:2: error: #error PREEMPT_ACTIVE is too low!
>> make[1]: *** [arch/blackfin/kernel/asm-offsets.s] Error 1
>>
>> this is because we've converted to asm-generic for most of our headers
>> (including hardirq.h). Âoriginally we were defining HARDIRQ_BITS
>> ourselves to 8, but then we dropped that in favor of the
>> asm-generic/hardirq.h which setup a default of 8. Âbut then they
>> dropped it in favor of the linux/hardirq.h default handling ... but it
>> sets it to MAX_HARDIRQ_BITS by default which is 10. Âwhich pushes
>> Blackfin over the edge and into this build error.
>
> hm, you wrote this mail to me but i havent touched asm-generic nor
> blackfin in this cycle.

i didnt say you did.  you seemed to be the guy who would know about
sane values in hardirq/preempt, i was merely giving background on what
lead me here -- those changes arent wrong in any way.

>> if we look at linux/hardirq.h, it makes this claim:
>> Â* - bit 28 is the PREEMPT_ACTIVE flag
>> if that's true, then why are we letting any arch set this define ? Âa
>> quick survey shows that half the arches (11) are using 0x10000000 (bit
>> 28) while the other half (10) are using 0x4000000 (bit 26). Âand then
>> there is the ia64 oddity which uses bit 30. Âthe exact value here
>> shouldnt really matter across arches though should it ?
>
> Correct - what matters is to have no collision between the fields.
>
>> how about adding this to linux/thread_info.h:
>> #ifndef PREEMPT_ACTIVE
>> # ifndef PREEMPT_ACTIVE_BIT
>> # Âdefine PREEMPT_ACTIVE_BIT 28
>> # endif
>> # define PREEMPT_ACTIVE (1 << PREEMPT_ACTIVE_BIT)
>> #endif
>
> Makes sense i guess - but do we really need that level of
> #ifdef nesting? PREEMPT_ACTIVE_BIT should be the main control - with
> a default to 28 if it's not set. PREEMPT_ACTIVE is then derived off
> that, without any #ifdefs.

well, i didnt want to write it like so:
#ifndef PREEMPT_ACTIVE_BIT
# define PREEMPT_ACTIVE_BIT 28
#endif
#ifndef PREEMPT_ACTIVE
# define PREEMPT_ACTIVE (1 << PREEMPT_ACTIVE_BIT)
#endif

because if the arch has defined PREEMPT_ACTIVE but not
PREEMPT_ACTIVE_BIT, then things could go bad.  since the only consumer
of PREEMPT_ACTIVE_BIT that i can see is one ia64 assembly file, we can
just avoid the indirection.  i wanted to make it clear that this is
indeed defaulting to bit 28 like the comments in hardirq.h say.  i
also wanted to avoid having to change any arch files other than my own
(i.e. allow people to be grandfathered in).

i guess we can reformat it as:
#ifndef PREEMPT_ACTIVE_BIT
# define PREEMPT_ACTIVE_BIT 28
#endif
#define PREEMPT_ACTIVE (1 << PREEMPT_ACTIVE_BIT)
which makes me do the footwork of converting everyone over to PREEMPT_ACTIVE_BIT

> Anyway ... no objections from me in this area (and your build is
> broken so i suspect you want a fix quickly), just please make the
> override clean. Btw., why cannot blackfin use the defaults?

Blackfin is using the defaults.  the issue i pointed out is that the
defaults set up 10 hardirq bits which ultimately conflict with any
arch (and there are 10 of them) that is using bit 26 for
PREEMPT_ACTIVE.  there is no default value for PREEMPT_ACTIVE (yet).
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/