* Re: [patch 1/2] spinlocks: check spinlock_t/rwlock_t argument type on non-SMP builds [not found] <200909181957.n8IJv94X001996@imap1.linux-foundation.org> @ 2009-09-21 7:43 ` Ingo Molnar 2009-09-21 20:09 ` Andrew Morton 0 siblings, 1 reply; 3+ messages in thread From: Ingo Molnar @ 2009-09-21 7:43 UTC (permalink / raw) To: akpm; +Cc: kilroyd, a.p.zijlstra, linux-kernel * akpm@linux-foundation.org <akpm@linux-foundation.org> wrote: > From: David Kilroy <kilroyd@googlemail.com> > > 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. > > Signed-off-by: David Kilroy <kilroyd@googlemail.com> > Cc: Ingo Molnar <mingo@elte.hu> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > include/linux/spinlock.h | 6 - > include/linux/spinlock_api_up.h | 118 ++++++++++++++++++++++-------- > 2 files changed, 92 insertions(+), 32 deletions(-) > > diff -puN include/linux/spinlock.h~spinlocks-check-spinlock_t-rwlock_t-argument-type-on-non-smp-builds include/linux/spinlock.h > --- a/include/linux/spinlock.h~spinlocks-check-spinlock_t-rwlock_t-argument-type-on-non-smp-builds > +++ a/include/linux/spinlock.h > @@ -240,17 +240,17 @@ static inline void smp_mb__after_lock(vo > #define spin_lock_irqsave(lock, flags) \ > do { \ > typecheck(unsigned long, flags); \ > - _spin_lock_irqsave(lock, flags); \ > + _spin_lock_irqsave(lock, &flags); \ > } while (0) > #define read_lock_irqsave(lock, flags) \ > do { \ > typecheck(unsigned long, flags); \ > - _read_lock_irqsave(lock, flags); \ > + _read_lock_irqsave(lock, &flags); \ > } while (0) these bits broke the Alpha build: /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 /home/mingo/tip/arch/alpha/include/asm/core_t2.h:456: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_readb' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_readw': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:465: error: '_spin_lock_irqsave' is static but used in inline function 't2_readw' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:470: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_readw' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_readl': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:483: error: '_spin_lock_irqsave' is static but used in inline function 't2_readl' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:488: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_readl' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_readq': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:497: error: '_spin_lock_irqsave' is static but used in inline function 't2_readq' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:504: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_readq' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_writeb': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:513: error: '_spin_lock_irqsave' is static but used in inline function 't2_writeb' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:519: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_writeb' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_writew': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:527: error: '_spin_lock_irqsave' is static but used in inline function 't2_writew' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:533: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_writew' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_writel': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:545: error: '_spin_lock_irqsave' is static but used in inline function 't2_writel' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:550: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_writel' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h: In function 't2_writeq': /home/mingo/tip/arch/alpha/include/asm/core_t2.h:558: error: '_spin_lock_irqsave' is static but used in inline function 't2_writeq' which is not static /home/mingo/tip/arch/alpha/include/asm/core_t2.h:565: error: '_spin_unlock_irqrestore' is static but used in inline function 't2_writeq' which is not static make[2]: *** [arch/alpha/kernel/sys_sable.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [arch/alpha/kernel] Error 2 make[1]: *** Waiting for unfinished jobs.... so i've dropped them for now. Ingo ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch 1/2] spinlocks: check spinlock_t/rwlock_t argument type on non-SMP builds 2009-09-21 7:43 ` [patch 1/2] spinlocks: check spinlock_t/rwlock_t argument type on non-SMP builds Ingo Molnar @ 2009-09-21 20:09 ` Andrew Morton 2009-09-21 20:30 ` Dave 0 siblings, 1 reply; 3+ messages in thread From: Andrew Morton @ 2009-09-21 20:09 UTC (permalink / raw) To: Ingo Molnar; +Cc: kilroyd, a.p.zijlstra, linux-kernel On Mon, 21 Sep 2009 09:43:52 +0200 Ingo Molnar <mingo@elte.hu> wrote: > > * akpm@linux-foundation.org <akpm@linux-foundation.org> wrote: > > > From: David Kilroy <kilroyd@googlemail.com> > > > > 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. > > > > Signed-off-by: David Kilroy <kilroyd@googlemail.com> > > Cc: Ingo Molnar <mingo@elte.hu> > > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > --- > > > > include/linux/spinlock.h | 6 - > > include/linux/spinlock_api_up.h | 118 ++++++++++++++++++++++-------- > > 2 files changed, 92 insertions(+), 32 deletions(-) > > > > diff -puN include/linux/spinlock.h~spinlocks-check-spinlock_t-rwlock_t-argument-type-on-non-smp-builds include/linux/spinlock.h > > --- a/include/linux/spinlock.h~spinlocks-check-spinlock_t-rwlock_t-argument-type-on-non-smp-builds > > +++ a/include/linux/spinlock.h > > @@ -240,17 +240,17 @@ static inline void smp_mb__after_lock(vo > > #define spin_lock_irqsave(lock, flags) \ > > do { \ > > typecheck(unsigned long, flags); \ > > - _spin_lock_irqsave(lock, flags); \ > > + _spin_lock_irqsave(lock, &flags); \ > > } while (0) > > #define read_lock_irqsave(lock, flags) \ > > do { \ > > typecheck(unsigned long, flags); \ > > - _read_lock_irqsave(lock, flags); \ > > + _read_lock_irqsave(lock, &flags); \ > > } while (0) > > these bits broke the Alpha build: > > /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 hm. What the heck does that mean? __EXTERN_INLINE u8 t2_readb(const volatile void __iomem *xaddr) Something to do with the extern-inline? Do you have the .config? My alpha builds went OK. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch 1/2] spinlocks: check spinlock_t/rwlock_t argument type on non-SMP builds 2009-09-21 20:09 ` Andrew Morton @ 2009-09-21 20:30 ` Dave 0 siblings, 0 replies; 3+ messages in thread From: Dave @ 2009-09-21 20:30 UTC (permalink / raw) To: Andrew Morton; +Cc: Ingo Molnar, a.p.zijlstra, linux-kernel Andrew Morton wrote: > On Mon, 21 Sep 2009 09:43:52 +0200 > Ingo Molnar <mingo@elte.hu> wrote: > >> * akpm@linux-foundation.org <akpm@linux-foundation.org> wrote: >> >>> From: David Kilroy <kilroyd@googlemail.com> >>> >>> 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. >>> >>> Signed-off-by: David Kilroy <kilroyd@googlemail.com> >>> Cc: Ingo Molnar <mingo@elte.hu> >>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> >>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> >>> --- >>> >>> include/linux/spinlock.h | 6 - >>> include/linux/spinlock_api_up.h | 118 ++++++++++++++++++++++-------- >>> 2 files changed, 92 insertions(+), 32 deletions(-) >>> >>> diff -puN include/linux/spinlock.h~spinlocks-check-spinlock_t-rwlock_t-argument-type-on-non-smp-builds include/linux/spinlock.h >>> --- a/include/linux/spinlock.h~spinlocks-check-spinlock_t-rwlock_t-argument-type-on-non-smp-builds >>> +++ a/include/linux/spinlock.h >>> @@ -240,17 +240,17 @@ static inline void smp_mb__after_lock(vo >>> #define spin_lock_irqsave(lock, flags) \ >>> do { \ >>> typecheck(unsigned long, flags); \ >>> - _spin_lock_irqsave(lock, flags); \ >>> + _spin_lock_irqsave(lock, &flags); \ >>> } while (0) >>> #define read_lock_irqsave(lock, flags) \ >>> do { \ >>> typecheck(unsigned long, flags); \ >>> - _read_lock_irqsave(lock, flags); \ >>> + _read_lock_irqsave(lock, &flags); \ >>> } while (0) >> these bits broke the Alpha build: >> >> /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 > > hm. What the heck does that mean? > > __EXTERN_INLINE u8 t2_readb(const volatile void __iomem *xaddr) > > Something to do with the extern-inline? This came up the last time this went into Ingos tree. And yes, it's the extern inlines using the spinlock functions that causes the error. > Do you have the .config? My alpha builds went OK. Last I checked, you need to have: CONFIG_SMP=N CONFIG_DEBUG_SPINLOCK=N The latter gets auto-selected, so I set: CONFIG_DEBUG_LOCK_ALLOC=N CONFIG_LOCK_STAT=N CONFIG_PROVE_LOCKING=N CONFIG_LOCK_STAT=N Regards, Dave. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-09-21 20:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200909181957.n8IJv94X001996@imap1.linux-foundation.org>
2009-09-21 7:43 ` [patch 1/2] spinlocks: check spinlock_t/rwlock_t argument type on non-SMP builds Ingo Molnar
2009-09-21 20:09 ` Andrew Morton
2009-09-21 20:30 ` Dave
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox