Linux PARISC architecture development
 help / color / mirror / Atom feed
* [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2]
@ 2005-06-03 17:31 Matthew Wilcox
  2005-06-04 11:55 ` Joel Soete
  2005-06-06  6:05 ` [parisc-linux] Re: [patch] spinlock consolidation, v2 Grant Grundler
  0 siblings, 2 replies; 17+ messages in thread
From: Matthew Wilcox @ 2005-06-03 17:31 UTC (permalink / raw)
  To: parisc-linux


Anyone have time to test this out?

----- Forwarded message from Ingo Molnar <mingo@elte.hu> -----

Date:	Fri, 3 Jun 2005 17:40:29 +0200
From:	Ingo Molnar <mingo@elte.hu>
Subject: [patch] spinlock consolidation, v2

the latest version of the spinlock consolidation patch can be found at:

  http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch

the patch is now complete in the sense that it does everything i wanted 
it to do. If you have any other suggestions (or i have missed to 
incorporate an earlier suggestion of yours), please yell.

Changes:

 - all architectures have been converted to the new spinlock code.
   arm, i386, ia64, ppc, ppc64, s390/s390x, x64 was build-tested via
   crosscompilers. Alpha, m32r, mips, parisc, sh, sparc, sparc64 has
   not been tested yet, but should be mostly fine. x86 and x64 was
   boot-tested in all relevant .config variations. It all brought a nice 
   reduction in source code size:

     62 files changed, 1455 insertions(+), 3025 deletions(-)

   Al, would you be interested in checking this patch on your build 
   farm? It should build on all architectures, UP and SMP alike.

   (NOTE: i've switched sparc32, sparc64, alpha, ppc, parisc to use the 
    generic spinlock debugging code. I believe the generic debugging 
    code is now capable enough to be a replacement - but especially the 
    Sparc ones are pretty advanced; so if i've missed some important
    feature please let me know and i'll implement it in the generic 
    code.)

 - linux/spinlock_types.h: new, pure header file that can be used
   by other headers to define spinlock fields - without having to
   pull in all the other include files that are needed on the
   implementational side. (Roman Zippel)

 - lib/spinlock_debug.c: got rid of the __FILE__/__LINE__ debug output 
   (suggested by Ingo Oeser), and streamlined the debug output. 
   Implemented 'lockup detection' which is a must for architectures that 
   dont have the equivalent of an NMI watchdog. (but is useful on other 
   architectures as well.) Both spinlocks and rwlocks are now fully 
   debugged.

 - linux/spinlock.h: got rid of the ATOMIC_DEC_AND_LOCK cruft. This is 
   achieved by not doing the UP-nonpreempt-nondebug specific 
   optimization but letting it pick the generic _atomic_dec_and_lock 
   function. The assembly looks sane on x86 (no locked ops, etc.) so 
   there's no performance problem. Other architectures should work fine 
   too, those which implement _atomic_dec_and_lock unconditionally might 
   want to review whether they want to use the CONFIG_HAVE_DEC_LOCK 
   mechanism to get the optimized (generic) version of the function on 
   UP.

 - asm-generic/spinlock_types_up.h: further simplifications (suggested
   by Arjan van de Ven), typo fixed

 - linux/spinlock_up.h: since this an UP-nondebug branch now, the macros 
   were simplified and streamlined significantly.

 - asm-generic/spinlock_up.h: further cleanups, reordering of op
   definitions into 'natural' op order.

 - asm-i386/spinlock.h and asm-x86_64/spinlock.h: reordering of ops, 
   cleanups

 - include/linux/spinlock.h: more cleanups, reordering

 - linux/spinlock_smp.h: cleanups, reordering of prototypes

 - kernel/spinlock.c: fixed bug in generic_raw_read_trylock and renamed 
   it to generic__raw_read_trylock to ease conversion.

 - lib/kernel_lock.c: simplification

 - (lots of small details i forgot)

	Ingo

----- End forwarded message -----

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2]
  2005-06-03 17:31 [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2] Matthew Wilcox
@ 2005-06-04 11:55 ` Joel Soete
  2005-06-04 18:03   ` Grant Grundler
  2005-06-06  6:05 ` [parisc-linux] Re: [patch] spinlock consolidation, v2 Grant Grundler
  1 sibling, 1 reply; 17+ messages in thread
From: Joel Soete @ 2005-06-04 11:55 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: parisc-linux

Hello Matthew, Mingo,

Matthew Wilcox wrote:
> Anyone have time to test this out?
> 
> ----- Forwarded message from Ingo Molnar <mingo@elte.hu> -----
> 
> Date:	Fri, 3 Jun 2005 17:40:29 +0200
> From:	Ingo Molnar <mingo@elte.hu>
> Subject: [patch] spinlock consolidation, v2
> 
> the latest version of the spinlock consolidation patch can be found at:
> 
>   http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch
> 
> the patch is now complete in the sense that it does everything i wanted 
> it to do. If you have any other suggestions (or i have missed to 
> incorporate an earlier suggestion of yours), please yell.
> 
I applied against one of our latest 2.6.12-rc5-pa1 without pb :-)

Any way it failled to compile very early with following messages:
   gcc -Wp,-MD,arch/parisc/kernel/.asm-offsets.s.d  -nostdinc -isystem /usr/lib/gcc-lib/hppa-linux/3.3.5/include -D__KERNEL__ 
-Iinclude -Iinclude2 -I/usr/src/linux-2.6.12-rc5-pa1-050601/include -I/usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel 
-Iarch/parisc/kernel -Wall -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -ffreestanding -O2 
-fomit-frame-pointer -pipe -mno-space-regs -mfast-indirect-calls -ffunction-sections -march=1.1 -mschedule=7200 
-DKBUILD_BASENAME=asm_offsets -DKBUILD_MODNAME=asm_offsets -S -o arch/parisc/kernel/asm-offsets.s 
/usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c
In file included from include2/asm/bitops.h:5,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/bitops.h:77,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/thread_info.h:20,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock.h:51,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c:31:
include2/asm/system.h:174: error: syntax error before "pa_tlb_lock"
include2/asm/system.h:174: warning: type defaults to `int' in declaration of `pa_tlb_lock'
include2/asm/system.h:174: warning: data definition has no type or storage class
In file included from include2/asm/atomic.h:17,
                  from include2/asm/bitops.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/bitops.h:77,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/thread_info.h:20,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock.h:51,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c:31:
include2/asm/spinlock.h:11: error: syntax error before '*' token
include2/asm/spinlock.h:12: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_spin_is_locked':
include2/asm/spinlock.h:13: error: `x' undeclared (first use in this function)
include2/asm/spinlock.h:13: error: (Each undeclared identifier is reported only once
include2/asm/spinlock.h:13: error: for each function it appears in.)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:21: error: syntax error before '*' token
include2/asm/spinlock.h:22: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_spin_lock':
include2/asm/spinlock.h:26: error: `x' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:32: error: syntax error before '*' token
include2/asm/spinlock.h:33: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_spin_unlock':
include2/asm/spinlock.h:36: error: `x' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:41: error: syntax error before '*' token
include2/asm/spinlock.h:42: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_spin_trylock':
include2/asm/spinlock.h:47: error: `x' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:64: error: syntax error before '*' token
include2/asm/spinlock.h:65: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_read_lock':
include2/asm/spinlock.h:68: error: `rw' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:76: error: syntax error before '*' token
include2/asm/spinlock.h:77: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_read_unlock':
include2/asm/spinlock.h:80: error: `rw' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:97: error: syntax error before '*' token
include2/asm/spinlock.h:98: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_write_lock':
include2/asm/spinlock.h:100: error: `rw' undeclared (first use in this function)
include2/asm/spinlock.h:107: warning: implicit declaration of function `cpu_relax'
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:118: error: syntax error before '*' token
include2/asm/spinlock.h:119: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_write_unlock':
include2/asm/spinlock.h:120: error: `rw' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:124: error: syntax error before '*' token
include2/asm/spinlock.h:125: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_write_trylock':
include2/asm/spinlock.h:126: error: `rw' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:139: error: syntax error before '*' token
include2/asm/spinlock.h:140: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_is_read_locked':
include2/asm/spinlock.h:141: error: `rw' undeclared (first use in this function)
include2/asm/spinlock.h: At top level:
include2/asm/spinlock.h:144: error: syntax error before '*' token
include2/asm/spinlock.h:145: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_is_write_locked':
include2/asm/spinlock.h:146: error: `rw' undeclared (first use in this function)
In file included from include2/asm/bitops.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/bitops.h:77,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/thread_info.h:20,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock.h:51,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c:31:
include2/asm/atomic.h: At top level:
include2/asm/atomic.h:27: error: syntax error before "__atomic_hash"
include2/asm/atomic.h:27: warning: type defaults to `int' in declaration of `__atomic_hash'
include2/asm/atomic.h:27: warning: data definition has no type or storage class
include2/asm/atomic.h: In function `__atomic_add_return':
include2/asm/atomic.h:143: error: `spinlock_t' undeclared (first use in this function)
include2/asm/atomic.h:143: error: `s' undeclared (first use in this function)
include2/asm/atomic.h:143: warning: implicit declaration of function `_raw_spin_lock'
include2/asm/atomic.h:147: warning: implicit declaration of function `_raw_spin_unlock'
include2/asm/atomic.h: In function `atomic_set':
include2/asm/atomic.h:154: error: `spinlock_t' undeclared (first use in this function)
include2/asm/atomic.h:154: error: `s' undeclared (first use in this function)
In file included from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/bitops.h:77,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/thread_info.h:20,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock.h:51,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c:31:
include2/asm/bitops.h: In function `set_bit':
include2/asm/bitops.h:45: error: `spinlock_t' undeclared (first use in this function)
include2/asm/bitops.h:45: error: `s' undeclared (first use in this function)
include2/asm/bitops.h: In function `clear_bit':
include2/asm/bitops.h:63: error: `spinlock_t' undeclared (first use in this function)
include2/asm/bitops.h:63: error: `s' undeclared (first use in this function)
include2/asm/bitops.h: In function `change_bit':
include2/asm/bitops.h:81: error: `spinlock_t' undeclared (first use in this function)
include2/asm/bitops.h:81: error: `s' undeclared (first use in this function)
include2/asm/bitops.h: In function `test_and_set_bit':
include2/asm/bitops.h:100: error: `spinlock_t' undeclared (first use in this function)
include2/asm/bitops.h:100: error: `s' undeclared (first use in this function)
include2/asm/bitops.h: In function `test_and_clear_bit':
include2/asm/bitops.h:127: error: `spinlock_t' undeclared (first use in this function)
include2/asm/bitops.h:127: error: `s' undeclared (first use in this function)
include2/asm/bitops.h: In function `test_and_change_bit':
include2/asm/bitops.h:154: error: `spinlock_t' undeclared (first use in this function)
include2/asm/bitops.h:154: error: `s' undeclared (first use in this function)
In file included from include2/asm/thread_info.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/thread_info.h:21,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock.h:51,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c:31:
include2/asm/processor.h: At top level:
include2/asm/processor.h:90: error: syntax error before "spinlock_t"
include2/asm/processor.h:90: warning: no semicolon at end of struct or union
include2/asm/processor.h:102: error: syntax error before '}' token
In file included from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock.h:79,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa1-050601/arch/parisc/kernel/asm-offsets.c:31:
/usr/src/linux-2.6.12-rc5-pa1-050601/include/linux/spinlock_types.h:26: error: `spinlock_t' used prior to declaration
make[2]: *** [arch/parisc/kernel/asm-offsets.s] Error 1
make[1]: *** [arch/parisc/kernel/asm-offsets.s] Error 2
make: *** [vmlinux] Error 2
====<>====

What did I missed?

Thanks,
     Joel
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2]
  2005-06-04 11:55 ` Joel Soete
@ 2005-06-04 18:03   ` Grant Grundler
  2005-06-05 19:13     ` Joel Soete
  0 siblings, 1 reply; 17+ messages in thread
From: Grant Grundler @ 2005-06-04 18:03 UTC (permalink / raw)
  To: Joel Soete; +Cc: parisc-linux

On Sat, Jun 04, 2005 at 11:55:03AM +0000, Joel Soete wrote:
...
> Any way it failled to compile very early with following messages:
...
> include2/asm/system.h:174: error: syntax error before "pa_tlb_lock"

It's because of the obscene parisc interdependence between
system.h, bitops.h, and spinlock.h.
Mingo's patch will help break up that a bit better.

I'm working on fixing that up. Two of the commits I made
last night are small steps in that direction.

grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2]
  2005-06-04 18:03   ` Grant Grundler
@ 2005-06-05 19:13     ` Joel Soete
  0 siblings, 0 replies; 17+ messages in thread
From: Joel Soete @ 2005-06-05 19:13 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux

Hello Grant,

Grant Grundler wrote:
> On Sat, Jun 04, 2005 at 11:55:03AM +0000, Joel Soete wrote:
> ...
> 
>>Any way it failled to compile very early with following messages:
> 
> ...
> 
>>include2/asm/system.h:174: error: syntax error before "pa_tlb_lock"
> 
> 
> It's because of the obscene parisc interdependence between
> system.h, bitops.h, and spinlock.h.
> Mingo's patch will help break up that a bit better.
> 
> I'm working on fixing that up. Two of the commits I made
> last night are small steps in that direction.
> 
Much for record: after latest pa3 co (and a make clean), I still get the same error:
   gcc -Wp,-MD,arch/parisc/kernel/.asm-offsets.s.d  -nostdinc -isystem /usr/lib/gcc-lib/hppa-linux/3.3.5/include -D__KERNEL__ 
-Iinclude -Iinclude2 -I/usr/src/linux-2.6.12-rc5-pa3-050604/include -I/usr/src/linux-2.6.12-rc5-pa3-050604/arch/parisc/kernel 
-Iarch/parisc/kernel -Wall -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -ffreestanding -O2 
-fomit-frame-pointer -pipe -mno-space-regs -mfast-indirect-calls -ffunction-sections -march=1.1 -mschedule=7200 
-DKBUILD_BASENAME=asm_offsets -DKBUILD_MODNAME=asm_offsets -S -o arch/parisc/kernel/asm-offsets.s 
/usr/src/linux-2.6.12-rc5-pa3-050604/arch/parisc/kernel/asm-offsets.c
                       ^^^^^^^^^^^^^^
In file included from include2/asm/atomic.h:17,
                  from include2/asm/bitops.h:8,
                  from /usr/src/linux-2.6.12-rc5-pa3-050604/include/linux/bitops.h:77,
                  from /usr/src/linux-2.6.12-rc5-pa3-050604/include/linux/thread_info.h:20,
                  from /usr/src/linux-2.6.12-rc5-pa3-050604/include/linux/spinlock.h:51,
                  from /usr/src/linux-2.6.12-rc5-pa3-050604/include/linux/capability.h:45,
                  from /usr/src/linux-2.6.12-rc5-pa3-050604/include/linux/sched.h:7,
                  from /usr/src/linux-2.6.12-rc5-pa3-050604/arch/parisc/kernel/asm-offsets.c:31:
include2/asm/spinlock.h:11: error: syntax error before '*' token
include2/asm/spinlock.h:12: warning: function declaration isn't a prototype
include2/asm/spinlock.h: In function `__raw_spin_is_locked':
include2/asm/spinlock.h:13: error: `x' undeclared (first use in this function)
[...]

seems that's because spinlock_t (which became raw_spinlock_t and move to asm/spinlock_types.h) was removed from asm-parisc/system.h

Thanks,
	Joel
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-03 17:31 [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2] Matthew Wilcox
  2005-06-04 11:55 ` Joel Soete
@ 2005-06-06  6:05 ` Grant Grundler
  2005-06-06  7:36   ` Ingo Molnar
  1 sibling, 1 reply; 17+ messages in thread
From: Grant Grundler @ 2005-06-06  6:05 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Ingo Molnar, parisc-linux

On Fri, Jun 03, 2005 at 06:31:32PM +0100, Matthew Wilcox wrote:
> 
> Anyone have time to test this out?
> 
> ----- Forwarded message from Ingo Molnar <mingo@elte.hu> -----
> 
> Date:	Fri, 3 Jun 2005 17:40:29 +0200
> From:	Ingo Molnar <mingo@elte.hu>
> Subject: [patch] spinlock consolidation, v2
> 
> the latest version of the spinlock consolidation patch can be found at:
> 
>   http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch

I've adapted parisc-linux to this patch and put my changes into
a patch that goes on top of Ingo's.
The patch is parked here:
	ftp://ftp.parisc-linux.org/patches/consolidate-spinlocks-parisc.patch

 arch/parisc/kernel/processor.c  |    3 ++-
 arch/parisc/kernel/smp.c        |   12 ++++++++----
 arch/parisc/lib/bitops.c        |    5 +++--
 include/asm-parisc/atomic.h     |   13 +++++++------
 include/asm-parisc/bitops.h     |    2 +-
 include/asm-parisc/cacheflush.h |    1 +
 include/asm-parisc/processor.h  |    3 ++-
 include/asm-parisc/spinlock.h   |    2 ++

More notes in the patch file.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-06  6:05 ` [parisc-linux] Re: [patch] spinlock consolidation, v2 Grant Grundler
@ 2005-06-06  7:36   ` Ingo Molnar
  2005-06-06 15:12     ` Grant Grundler
       [not found]     ` <20050606175029.GC24437@colo.lackof.org>
  0 siblings, 2 replies; 17+ messages in thread
From: Ingo Molnar @ 2005-06-06  7:36 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux, Matthew Wilcox


* Grant Grundler <grundler@parisc-linux.org> wrote:

> > the latest version of the spinlock consolidation patch can be found at:
> > 
> >   http://redhat.com/~mingo/spinlock-patches/consolidate-spinlocks.patch
> 
> I've adapted parisc-linux to this patch and put my changes into
> a patch that goes on top of Ingo's.

i'm wondering, is the conversion to raw_spinlock_t absolutely necessary 
to get a working PARISC kernel? I feel a bit uneasy about using the raw 
spinlocks directly - they were not intended to be used like that, for 
the time being. Right now they are internal types. You should be able to 
use spinlock_types.h just as much to simplify your include file 
dependencies. (as long as you use the spinlock APIs only in .c files)

	Ingo
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-06  7:36   ` Ingo Molnar
@ 2005-06-06 15:12     ` Grant Grundler
  2005-06-06 16:31       ` Ingo Molnar
       [not found]     ` <20050606175029.GC24437@colo.lackof.org>
  1 sibling, 1 reply; 17+ messages in thread
From: Grant Grundler @ 2005-06-06 15:12 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: parisc-linux, Matthew Wilcox

On Mon, Jun 06, 2005 at 09:36:27AM +0200, Ingo Molnar wrote:
> > I've adapted parisc-linux to this patch and put my changes into
> > a patch that goes on top of Ingo's.
> 
> i'm wondering, is the conversion to raw_spinlock_t absolutely necessary 
> to get a working PARISC kernel?

No. They were regular spinlocks before too. But there was a pretty
ugly set of interdependencies between asm/system.h, spinlock.h,
atomic.h, and bitops.h.

The cleanup necessary might be alot more baggage than you want
to carry around for this patch.

>  I feel a bit uneasy about using the raw 
> spinlocks directly - they were not intended to be used like that, for 
> the time being. Right now they are internal types.

They are arch specific types (asm/spinlock_types.h) and
I'm only using raw_spinlock_t in arch specific code.

>  You should be able to 
> use spinlock_types.h just as much to simplify your include file 
> dependencies. (as long as you use the spinlock APIs only in .c files)

I'll take another quick look
and see where the circular dependencies are now.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-06 15:12     ` Grant Grundler
@ 2005-06-06 16:31       ` Ingo Molnar
  2005-06-06 17:55         ` Grant Grundler
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2005-06-06 16:31 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux, Matthew Wilcox


* Grant Grundler <grundler@parisc-linux.org> wrote:

> >  I feel a bit uneasy about using the raw 
> > spinlocks directly - they were not intended to be used like that, for 
> > the time being. Right now they are internal types.
> 
> They are arch specific types (asm/spinlock_types.h) and I'm only using 
> raw_spinlock_t in arch specific code.

please dont use the raw spinlocks. They dont carry some features such as 
debugging. (or any other future feature of the generic spinlock_t type)

	Ingo
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-06 16:31       ` Ingo Molnar
@ 2005-06-06 17:55         ` Grant Grundler
  0 siblings, 0 replies; 17+ messages in thread
From: Grant Grundler @ 2005-06-06 17:55 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: parisc-linux, Matthew Wilcox

On Mon, Jun 06, 2005 at 06:31:41PM +0200, Ingo Molnar wrote:
> > They are arch specific types (asm/spinlock_types.h) and I'm only using 
> > raw_spinlock_t in arch specific code.
> 
> please dont use the raw spinlocks. They dont carry some features such as 
> debugging. (or any other future feature of the generic spinlock_t type)
> 

TBH, I'd rather not use them. Please look at the -02 patch I just sent
and advise on how you think the cyclic dependency can be killed.
raw spinlocks is obviously just one way.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [parisc-linux] Re: [patch] spinlock consolidation, v2
       [not found] ` <4282FEEC0000AB95@mail-3-bnl.tiscali.it>
@ 2005-06-08 16:05   ` Grant Grundler
  0 siblings, 0 replies; 17+ messages in thread
From: Grant Grundler @ 2005-06-08 16:05 UTC (permalink / raw)
  To: Joel Soete; +Cc: Ingo Molnar, parisc-linux

On Wed, Jun 08, 2005 at 03:31:14PM +0200, Joel Soete wrote:
> error: structure has no member named `lock'
...
> the pb is this hunk:
> --- linux/include/asm-parisc/processor.h-orig   20 May 2005 00:05:13 -0000
> +++ linux/include/asm-parisc/processor.h        6 Jun 2005 04:32:36 -0000
> @@ -87,7 +88,7 @@ struct cpuinfo_parisc {
>         unsigned long hpa;          /* Host Physical address */
>         unsigned long txn_addr;     /* MMIO addr of EIR or id_eid */
>  #ifdef CONFIG_SMP
> -       spinlock_t lock;            /* synchronization for ipi's */
> +       raw_spinlock_t pending_lock;  /* protect sender/receiver races */
>         unsigned long pending_ipi;  /* bitmap of type ipi_message_type */
>         unsigned long ipi_count;    /* number ipi Interrupts */
>  #endif
> ====<>====

Sorry - my bad. That chunk is not in my source tree.
I've respun the patch without that bit.
See -03 in the same location.

> Freeing unused kernel memory: Badness in smp_call_function at /Develop/linux-2.6.12-rc5-pa3-050606_4Consolidate/arch/parisc/kernel/0
> Backtrace:
>  [<1011604c>] smp_call_function+0x6c/0x384
>  [<10106024>] flush_data_cache+0x24/0x40
...

This is not a real problem. We have a paranoid WARN_ON in the code
and it's been warning us about this one case for a long time now.

> 364k freed
> [...]
> 
> hanging?

What was the last output?

> The analayse of a TOC giving me following info:
> -----------------  Processor 0 TOC Information -------------------
> GR[02] == rp = 0000000010276d7c
> Func: uart_shutdown, Off: 0xbc, Addr: 0x10276d7c
...
> Parse IAOQ = 0x0000000010158b80 for CPU[0]
> Func: synchronize_irq, Off: 0x18, Addr: 0x10158b80


> GR[02] == rp = 0000000010195000
> Func: chrdev_open, Off: 0xe0, Addr: 0x10195000
...
> Parse IAOQ = 0x0000000010104404 for CPU[1]
> Func: lock_kernel, Off: 0x34, Addr: 0x10104404

I'm not sure what the issue is here.
Let me respin the patch so that I know the lock is correctly declared.

grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
       [not found]     ` <20050606175029.GC24437@colo.lackof.org>
@ 2005-06-12  6:49       ` Ingo Molnar
  2005-06-12  7:25         ` Grant Grundler
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2005-06-12  6:49 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux, Matthew Wilcox


* Grant Grundler <grundler@parisc-linux.org> wrote:

> IPI and TLB locks can be normal locks.
> I've dropped the parts that changed those.
> 
> New (smaller) patch parked at:
>     ftp://ftp.parisc-linux.org/patches/consolidate-spinlocks-parisc.patch-02

thanks. I'm still wondering about the fundamental question though: why 
doesnt the box boot without your patch? Why does the __atomic_hash and 
the pending_lock have to be converted to a raw_spinlock? This patch does 
not change anything fundamental - you are still getting the same 
spinlock primitives from spinlock_t. The only thing that changes is with 
spinlock debugging: there another (generic) piece of code kicks in. The 
only problem could be with assembly code that 'knows' the layout of the 
spinlocks but doesnt take debugging into account - but is this possible 
in the PARISC case? (Yet another possibility would be if i messed up the 
raw type completely, but then your box wouldnt boot at all.)

so this is still quite much of a mystery to me!

	Ingo
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-12  6:49       ` Ingo Molnar
@ 2005-06-12  7:25         ` Grant Grundler
  2005-06-12  7:34           ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Grant Grundler @ 2005-06-12  7:25 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: parisc-linux, Matthew Wilcox

On Sun, Jun 12, 2005 at 08:49:22AM +0200, Ingo Molnar wrote:
> 
> * Grant Grundler <grundler@parisc-linux.org> wrote:
> 
> > IPI and TLB locks can be normal locks.
> > I've dropped the parts that changed those.
> > 
> > New (smaller) patch parked at:
> >     ftp://ftp.parisc-linux.org/patches/consolidate-spinlocks-parisc.patch-02
> 
> thanks. I'm still wondering about the fundamental question though: why 
> doesnt the box boot without your patch?

Did you mean to ask: Why doesn't the kernel build w/o my patch?

> Why does the __atomic_hash and the pending_lock have to be converted
> to a raw_spinlock?

Only the __atomic_hash is raw_spinlock_t in this last patch (-03).

> This patch does not change anything fundamental

Eh?! Of course your patch does!
It dramatically changes the (nearly) circular dependencies in parisc
kernel header files. I avoid the circular dependency by using
a "raw spinlock" in that one case.

Look at the chain of nested include files in my previous message and
follow the chain and how different consumers of spinlocks are defined
in it.  It should be clear why that lock has to be a raw lock.

>  - you are still getting the same spinlock primitives from spinlock_t.

Yes - but which header files are involved in defining spinlock_t?
And which ones does parisc atomic.h and bitops.h need?

>  The only thing that changes is with 
> spinlock debugging: there another (generic) piece of code kicks in. The 
> only problem could be with assembly code that 'knows' the layout of the 
> spinlocks but doesnt take debugging into account - but is this possible 
> in the PARISC case? (Yet another possibility would be if i messed up the 
> raw type completely, but then your box wouldnt boot at all.)

No - I don't you messed up the raw type. And I personally like that change.
SMP boots and "basically" works fine for me.
(Some other bugs are outstanding and not related to spinlock definitions.)

Debug code is just a distraction. It's not relevant to this problem IMHO.

> so this is still quite much of a mystery to me!

parisc has circular dependencies between system.h, atomic.h, bitops.h,
and spinlock.h in the include/asm-parisc header files.
Why?  Because we only have one atomic operation: LDCW. We use
it to define spinlocks....which are used for atomic-ops
and bitops...I've forgotten the exact dependency now since
I sorted this out about 2 years ago.

But it was the reason I added "raw spinlocks"....you've introduced
"raw_spinlock_t" and that's only complicated the circular dependency
but not eliminated it.

grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-12  7:25         ` Grant Grundler
@ 2005-06-12  7:34           ` Ingo Molnar
  2005-06-13  6:29             ` Grant Grundler
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2005-06-12  7:34 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux, Matthew Wilcox


* Grant Grundler <grundler@parisc-linux.org> wrote:

> > thanks. I'm still wondering about the fundamental question though: why 
> > doesnt the box boot without your patch?
> 
> Did you mean to ask: Why doesn't the kernel build w/o my patch?

ah, ok. If that's the case, then the main problem is that i've added 
#include <asm/atomic.h> to linux/spinlock.h? If you remove that
include from linux/spinlock.h and if you hack atomic_dec_and_lock to not 
use atomic_t but void *, does it build fine?

	Ingo
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-12  7:34           ` Ingo Molnar
@ 2005-06-13  6:29             ` Grant Grundler
  2005-06-13  7:44               ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Grant Grundler @ 2005-06-13  6:29 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: parisc-linux, Matthew Wilcox

On Sun, Jun 12, 2005 at 09:34:53AM +0200, Ingo Molnar wrote:
> 
> * Grant Grundler <grundler@parisc-linux.org> wrote:
> 
> > > thanks. I'm still wondering about the fundamental question though: why 
> > > doesnt the box boot without your patch?
> > 
> > Did you mean to ask: Why doesn't the kernel build w/o my patch?
> 
> ah, ok. If that's the case, then the main problem is that i've added 
> #include <asm/atomic.h> to linux/spinlock.h? 

Hrmm...I'm not sure I'd call that a "problem".
linux/spinlock.h uses atomic_t and following current rules,
it should include some version of atomic.h to get a definition
of atomic_t.

> If you remove that
> include from linux/spinlock.h and if you hack atomic_dec_and_lock to not 
> use atomic_t but void *, does it build fine?

I remove the include but continued to use "atomic_t" anyway.
That built too. :^)

Oh, sorry. I just realized that's still with my patch in place.
Tomorrow I'll back out my patch and see if it still builds.

BTW, I do not like "void *" if we can (and should) use a special typedef.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-13  6:29             ` Grant Grundler
@ 2005-06-13  7:44               ` Ingo Molnar
  2005-06-13 18:39                 ` Grant Grundler
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2005-06-13  7:44 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux, Matthew Wilcox


* Grant Grundler <grundler@parisc-linux.org> wrote:

> > If you remove that
> > include from linux/spinlock.h and if you hack atomic_dec_and_lock to not 
> > use atomic_t but void *, does it build fine?
> 
> I remove the include but continued to use "atomic_t" anyway.
> That built too. :^)
> 
> Oh, sorry. I just realized that's still with my patch in place. 
> Tomorrow I'll back out my patch and see if it still builds.
> 
> BTW, I do not like "void *" if we can (and should) use a special 
> typedef.

yeah, agreed - that was just a quick hack, to see whether the dependency 
problem is sorted out via it. (i'm not sure it will be resolved) I'd 
like to address these dependency problems without having to change any 
spinlock to raw_spinlock.

	Ingo
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-13  7:44               ` Ingo Molnar
@ 2005-06-13 18:39                 ` Grant Grundler
  2005-06-21 11:22                   ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Grant Grundler @ 2005-06-13 18:39 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: parisc-linux, Matthew Wilcox

On Mon, Jun 13, 2005 at 09:44:07AM +0200, Ingo Molnar wrote:
> > BTW, I do not like "void *" if we can (and should) use a special 
> > typedef.
> 
> yeah, agreed - that was just a quick hack, to see whether the dependency 
> problem is sorted out via it.

Understood - I just don't think it's worth solving this problem generically
by dropping the type checking.  I think this problem is unique to parisc.
No matter how we sliced and dice it, bitops and spinlock will collide
on parisc.

>  (i'm not sure it will be resolved) I'd 
> like to address these dependency problems without having to change any 
> spinlock to raw_spinlock.

I don't want to hold up your patch since I believe it's a step
in the right direction.  I'm ok with changing to a raw_spinlock_t
for bitops.h for now.

Thinking more about circular dependency. It's basically
something like this:
	bitops.h depends on atomic.h for _atomic_spin_lock_irqsave/et al.
	atomic.h depends on spinlock.h for _raw_spin_lock* to
		define _atomic_spin_lock_irqsave/et al.
	linux/spinlock.h now depends on asm/atomic.h for atomic_t
		definition used in _atomic_dec_and_lock().

The next step may to seperate the definition of atomic_t (e.g moving
that to asm/types.h) from the inline code (usage) and function prototypes.
ie have 4 header files:
	spinlock_types.h	/* spinlock_t, SPIN_UNLOCKED, et al */
	spinlocks		/* spin_lock_*() */
	atomic_types.h		/* atomic_t */
	atomic.h		/* _atomic_spin_lock_*(), __xchg(), et al  */

Then linux/spinlock.h can include asm/atomic_types.h to get just the
subset it needs. asm/atomic.h will have to include linux/spinlock.h
then to use regular spinlocks then.  TBH, I don't like the general idea
of an asm/*.h depending on a linux/*.h (atomic.h including spinlock.h
respectively in this case).  But could live with it if you feel strongly
about no one using _raw_spinlocks directly.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [parisc-linux] Re: [patch] spinlock consolidation, v2
  2005-06-13 18:39                 ` Grant Grundler
@ 2005-06-21 11:22                   ` Ingo Molnar
  0 siblings, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2005-06-21 11:22 UTC (permalink / raw)
  To: Grant Grundler; +Cc: parisc-linux, Matthew Wilcox


* Grant Grundler <grundler@parisc-linux.org> wrote:

> The next step may to seperate the definition of atomic_t (e.g moving
> that to asm/types.h) from the inline code (usage) and function prototypes.
> ie have 4 header files:
> 	spinlock_types.h	/* spinlock_t, SPIN_UNLOCKED, et al */
> 	spinlocks		/* spin_lock_*() */
> 	atomic_types.h		/* atomic_t */
> 	atomic.h		/* _atomic_spin_lock_*(), __xchg(), et al  */
> 
> Then linux/spinlock.h can include asm/atomic_types.h to get just the 
> subset it needs. asm/atomic.h will have to include linux/spinlock.h 
> then to use regular spinlocks then.  TBH, I don't like the general 
> idea of an asm/*.h depending on a linux/*.h (atomic.h including 
> spinlock.h respectively in this case).  But could live with it if you 
> feel strongly about no one using _raw_spinlocks directly.

perhaps we could further simplify things by requiring arch 
spinlock_types.h to include any other types the main spinlock.h needs.  
For most arches that would be a simple #include <asm/atomic.h>, for 
PARISC it would be #include <asm/atomic_types.h>. But i also like the 
idea of splitting up atomic.h into atomic.h and atomic_types.h.

on PARISC, asm/atomic.h would have to include linux/spinlock.h, and i 
dont see that as an ugly thing: your atomic type implementation does 
depend on spinlocks, and spinlocks are defined by the linux/spinlock*.h 
files.

	Ingo
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2005-06-21 11:22 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-06-03 17:31 [parisc-linux] [mingo@elte.hu: [patch] spinlock consolidation, v2] Matthew Wilcox
2005-06-04 11:55 ` Joel Soete
2005-06-04 18:03   ` Grant Grundler
2005-06-05 19:13     ` Joel Soete
2005-06-06  6:05 ` [parisc-linux] Re: [patch] spinlock consolidation, v2 Grant Grundler
2005-06-06  7:36   ` Ingo Molnar
2005-06-06 15:12     ` Grant Grundler
2005-06-06 16:31       ` Ingo Molnar
2005-06-06 17:55         ` Grant Grundler
     [not found]     ` <20050606175029.GC24437@colo.lackof.org>
2005-06-12  6:49       ` Ingo Molnar
2005-06-12  7:25         ` Grant Grundler
2005-06-12  7:34           ` Ingo Molnar
2005-06-13  6:29             ` Grant Grundler
2005-06-13  7:44               ` Ingo Molnar
2005-06-13 18:39                 ` Grant Grundler
2005-06-21 11:22                   ` Ingo Molnar
     [not found] <20050607180315.GH29220@colo.lackof.org>
     [not found] ` <4282FEEC0000AB95@mail-3-bnl.tiscali.it>
2005-06-08 16:05   ` Grant Grundler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox