diff mbox

ARM: Re-enable TRACE_IRQFLAGS_SUPPORT on ARMv7-M

Message ID 1433802288-25508-1-git-send-email-mcoquelin.stm32@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Coquelin June 8, 2015, 10:24 p.m. UTC
Commit cb1293e2f594 ("ARM: 8375/1: disable some options on ARMv7-M") causes
build failure on ARMv7-M machines:

  CC      arch/arm/kernel/asm-offsets.s
In file included from include/linux/sem.h:5:0,
                 from include/linux/sched.h:35,
                 from arch/arm/kernel/asm-offsets.c:14:
include/linux/rcupdate.h: In function 'rcu_read_lock_sched_held':
include/linux/rcupdate.h:539:2: error: implicit declaration of function 'arch_irqs_disabled' [-Werror=implicit-function-declaration]
  return preempt_count() != 0 || irqs_disabled();
  ^
Commit cb1293e2f594 was initially introduced to fix build errors with randconfig,
as ARMv7-m selects TRACE_IRQFLAGS_SUPPORT but breaks build when TRACE_IRQFLAGS
is selected.

As there is no proper support yet for TRACE_IRQFLAGS, this patch partially
reverts commit cb1293e2f594, with the downside of build errors with randconfig
when TRACE_IRQFLAGS gets selected.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
---
 arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Russell King - ARM Linux June 8, 2015, 10:47 p.m. UTC | #1
On Tue, Jun 09, 2015 at 12:24:48AM +0200, Maxime Coquelin wrote:
> Commit cb1293e2f594 ("ARM: 8375/1: disable some options on ARMv7-M") causes
> build failure on ARMv7-M machines:
> 
>   CC      arch/arm/kernel/asm-offsets.s
> In file included from include/linux/sem.h:5:0,
>                  from include/linux/sched.h:35,
>                  from arch/arm/kernel/asm-offsets.c:14:
> include/linux/rcupdate.h: In function 'rcu_read_lock_sched_held':
> include/linux/rcupdate.h:539:2: error: implicit declaration of function 'arch_irqs_disabled' [-Werror=implicit-function-declaration]
>   return preempt_count() != 0 || irqs_disabled();
>   ^

The real solution is to provide a definition _in asm-generic_ for
arch_irqs_disabled(), rather than having almost every arch doing:

static inline bool arch_irqs_disabled(void)
{
        return arch_irqs_disabled_flags(arch_local_save_flags());
}

I'm personally refusing to take a patch for ARM which adds yet another
copy of the above.  This is, after all, exactly the kind of stuff that
should be in asm-generic, or if not, in include/linux but overridable
by arch stuff.

We keep going between the two extremes of "lets push lots of stuff into
arch stuff" and "lets try to extract the common bits out of arch code".

Let's try and settle on one approach, and apply it universally.

In the mean time, I think the right answer is to drop Arnd's patch -
subsituting a randconfig build error for a useful-config build error
is not something we want to do - and even partially reverting the
patch results in randconfig build errors returning, so...
Maxime Coquelin June 9, 2015, 9:14 a.m. UTC | #2
2015-06-09 0:47 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Jun 09, 2015 at 12:24:48AM +0200, Maxime Coquelin wrote:
>> Commit cb1293e2f594 ("ARM: 8375/1: disable some options on ARMv7-M") causes
>> build failure on ARMv7-M machines:
>>
>>   CC      arch/arm/kernel/asm-offsets.s
>> In file included from include/linux/sem.h:5:0,
>>                  from include/linux/sched.h:35,
>>                  from arch/arm/kernel/asm-offsets.c:14:
>> include/linux/rcupdate.h: In function 'rcu_read_lock_sched_held':
>> include/linux/rcupdate.h:539:2: error: implicit declaration of function 'arch_irqs_disabled' [-Werror=implicit-function-declaration]
>>   return preempt_count() != 0 || irqs_disabled();
>>   ^
>
> The real solution is to provide a definition _in asm-generic_ for
> arch_irqs_disabled(), rather than having almost every arch doing:
>
> static inline bool arch_irqs_disabled(void)
> {
>         return arch_irqs_disabled_flags(arch_local_save_flags());
> }
>
> I'm personally refusing to take a patch for ARM which adds yet another
> copy of the above.  This is, after all, exactly the kind of stuff that
> should be in asm-generic, or if not, in include/linux but overridable
> by arch stuff.
>
> We keep going between the two extremes of "lets push lots of stuff into
> arch stuff" and "lets try to extract the common bits out of arch code".
>
> Let's try and settle on one approach, and apply it universally.

I agree on the idea but I don't measure all the impacts it would have.

>
> In the mean time, I think the right answer is to drop Arnd's patch -
> subsituting a randconfig build error for a useful-config build error
> is not something we want to do - and even partially reverting the
> patch results in randconfig build errors returning, so...

Ok, should I send a revert, or you can still drop Arnd's patch directly?

Thanks,
Maxime
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b47457d..e4e1694 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -171,7 +171,7 @@  config LOCKDEP_SUPPORT
 
 config TRACE_IRQFLAGS_SUPPORT
 	bool
-	default !CPU_V7M
+	default y
 
 config RWSEM_XCHGADD_ALGORITHM
 	bool