| 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
| ||
|
Message-ID: <4A773FFF.7050106@gmail.com> Date: Mon, 03 Aug 2009 20:52:31 +0100 From: Dave <kilroyd@...glemail.com> To: Ingo Molnar <mingo@...e.hu> CC: mingo@...hat.com, hpa@...or.com, linux-kernel@...r.kernel.org, a.p.zijlstra@...llo.nl, tglx@...utronix.de, linux-tip-commits@...r.kernel.org Subject: Re: [tip:core/locking] locking: Check spinlock_t/rwlock_t argument type on non-SMP builds too Ingo Molnar wrote: > * tip-bot for David Kilroy <kilroyd@...glemail.com> wrote: > >> Commit-ID: 8d19bd347e1073e7f1ef9f8057c560a5f6be63d7 >> Gitweb: http://git.kernel.org/tip/8d19bd347e1073e7f1ef9f8057c560a5f6be63d7 >> Author: David Kilroy <kilroyd@...glemail.com> >> AuthorDate: Sun, 2 Aug 2009 19:41:05 +0100 >> Committer: Ingo Molnar <mingo@...e.hu> >> CommitDate: Sun, 2 Aug 2009 21:29:07 +0200 >> >> locking: Check spinlock_t/rwlock_t argument type on non-SMP builds too >> >> When writing code for UP without CONFIG_DEBUG_SPINLOCK it's >> easy to get the first argument to the spinlock/rwlock functions >> wrong. This is because the parameter is not actually used in >> this configuration. >> >> Typically you will only find out it's wrong: >> >> * by rebuilding with CONFIG_SMP or CONFIG_DEBUG_SPINLOCK >> * after you've submitted your beautiful patch series >> >> The first means a long wait, and the latter is a bit late. >> >> Change the intermediate macros into inline functions. >> >> -v2: Use C inlines instead of in-macro type-checks >> -v3: Correct bug in _spin_trylock_bh > > x86 works fine now - but this patch broke architecture builds, such > as the build on Alpha: > > In file included from > /home/mingo/tip/arch/alpha/kernel/sys_sable.c:25: > /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function > 't2_readb': > /home/mingo/tip/arch/alpha/include/asm/core_t2.h:451: error: > '_spin_lock_irqsave' is static but used in inline function > 't2_readb' which is not static I tried a couple things (see below), but don't see a nice simple way forward. One option I didn't investigate is copying what SMP does. I'm inclined to recommend v1 of the patch. Regards, Dave. --- Switching to extern inline on x86 leads to: /usr/src/linux-current/wireless-testing/include/linux/spinlock_api_up.h: In function '_spin_lock_irq': /usr/src/linux-current/wireless-testing/include/linux/spinlock_api_up.h:73: warning: 'raw_local_irq_disable' is static but used in inline function '_spin_lock_irq' which is not static ... /usr/src/linux-current/wireless-testing/include/linux/spinlock_api_up.h:139: warning: 'current_thread_info' is static but used in inline function '_write_unlock_irqrestore' which is not static Looks like one for each of the functions. Seems to link OK. -> cascade the extern inline into raw versions of functions? on all archs? --- Plain old inline (and __attribute__((gnu_inline)) ) on x86 leads to: LD init/mounts.o init/do_mounts_rd.o: In function `_write_unlock_irqrestore': do_mounts_rd.c:(.text+0x0): multiple definition of `_write_unlock_irqrestore' init/do_mounts.o:do_mounts.c:(.text+0x1b5): first defined here ... init/do_mounts_initrd.o: In function `_spin_lock': do_mounts_initrd.c:(.text+0x298): multiple definition of `_spin_lock' init/do_mounts.o:do_mounts.c:(.text+0x44d): first defined here make[2]: *** [init/mounts.o] Error 1 make[1]: *** [init] Error 2 make: *** [sub-make] Error 2 -> clever linker solution? --- The statistics (2.6.31-rc4ish) show a strong preference for static inline: $ find include -name "*.h" | xargs grep "static inline" | wc -l 6461 $ find include -name "*.h" | xargs grep "extern inline" | wc -l 2 $ find arch -name "*.h" | xargs grep "extern inline" | wc -l 125 $ find arch -name "*.h" | xargs grep "static inline" | wc -l 7058 $ find arch -name "*.h" | xargs grep "extern inline" | sed "s?^\(.*\):.*\$?\1?" | sort | uniq arch/alpha/include/asm/core_apecs.h arch/alpha/include/asm/core_cia.h arch/alpha/include/asm/core_irongate.h arch/alpha/include/asm/core_lca.h arch/alpha/include/asm/core_marvel.h arch/alpha/include/asm/core_mcpcia.h arch/alpha/include/asm/core_polaris.h arch/alpha/include/asm/core_t2.h arch/alpha/include/asm/core_titan.h arch/alpha/include/asm/core_tsunami.h arch/alpha/include/asm/core_wildfire.h arch/alpha/include/asm/io.h arch/alpha/include/asm/jensen.h arch/alpha/include/asm/mmu_context.h arch/alpha/include/asm/pci.h arch/alpha/include/asm/pgtable.h arch/alpha/include/asm/processor.h arch/alpha/include/asm/system.h arch/alpha/include/asm/tlbflush.h arch/alpha/include/asm/uaccess.h arch/blackfin/include/asm/string.h arch/m68k/include/asm/sbus.h arch/microblaze/include/asm/cacheflush.h arch/microblaze/include/asm/delay.h arch/microblaze/include/asm/pgalloc.h arch/microblaze/include/asm/processor.h -- 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/
Powered by blists - more mailing lists