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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8bd0f97a0906240622i78135066mcbaa2f0add108593@mail.gmail.com>
Date:	Wed, 24 Jun 2009 09:22:05 -0400
From:	Mike Frysinger <vapier.adi@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Arnd Bergmann <arnd@...db.de>,
	Steven Rostedt <rostedt@...dmis.org>,
	Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: PREEMPT_ACTIVE too low error with all asm-generic headers for 
	some arches

On Wed, Jun 24, 2009 at 09:13, Ingo Molnar wrote:
> * Mike Frysinger <vapier.adi@...il.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/