Re: [patch 1/1] unified spinlock initialization arch/um/drivers/port_kern.c
From: Russell King <hidden>
Date: 2005-03-09 17:12:52
4 messages, 3 authors, 2005-03-11 · open the first message on its own page
From: Russell King <hidden>
Date: 2005-03-09 17:12:52
On Wed, Mar 09, 2005 at 10:42:33AM +0100, blaisorblade@yahoo.it wrote:
From: <redacted> Cc: <redacted>, <redacted>, <redacted>, <redacted> Unify the spinlock initialization as far as possible.
Are you sure this is really the best option in this instance? Sometimes, static data initialisation is more efficient than code-based manual initialisation, especially when the memory is written to anyway. -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core
From: Blaisorblade <hidden>
Date: 2005-03-09 20:01:19
On Wednesday 09 March 2005 18:12, Russell King wrote:
On Wed, Mar 09, 2005 at 10:42:33AM +0100, blaisorblade@yahoo.it wrote:quoted
From: <redacted> Cc: <redacted>, <redacted>, [off-list ref], [off-list ref] Unify the spinlock initialization as far as possible.
Are you sure this is really the best option in this instance? Sometimes, static data initialisation is more efficient than code-based manual initialisation, especially when the memory is written to anyway.
Agreed, theoretically, but this was done for multiple reasons globally, for instance as a preparation to Ingo Molnar's preemption patches. There was mention of this on lwn.net about this: http://lwn.net/Articles/108719/ Ok? -- Paolo Giarrusso, aka Blaisorblade Linux registered user n. 292729 http://www.user-mode-linux.org/~blaisorblade
From: Thomas Gleixner <hidden>
Date: 2005-03-10 08:14:00
On Wed, 2005-03-09 at 20:52 +0100, Blaisorblade wrote:
quoted
Are you sure this is really the best option in this instance? Sometimes, static data initialisation is more efficient than code-based manual initialisation, especially when the memory is written to anyway.Agreed, theoretically, but this was done for multiple reasons globally, for instance as a preparation to Ingo Molnar's preemption patches. There was mention of this on lwn.net about this: http://lwn.net/Articles/108719/
Those patches did only the conversion of
static spinlock_t lock = SPIN_LOCK_UNLOCKED;
lock = SPIN_LOCK_UNLOCKED;
to
static DEFINE_SPINLOCK(lock);
spin_lock_init(lock);
If you want to do static initialization inside of structures, then you
have to define a seperate MACRO similar to the static initialization of
list_head's inside of structures:
static struct sysfs_dirent sysfs_root = {
.s_sibling = LIST_HEAD_INIT(sysfs_root.s_sibling),
tglx
From: Blaisorblade <hidden>
Date: 2005-03-11 20:29:49
On Thursday 10 March 2005 09:12, Thomas Gleixner wrote:
On Wed, 2005-03-09 at 20:52 +0100, Blaisorblade wrote:quoted
quoted
Are you sure this is really the best option in this instance? Sometimes, static data initialisation is more efficient than code-based manual initialisation, especially when the memory is written to anyway.Agreed, theoretically, but this was done for multiple reasons globally, for instance as a preparation to Ingo Molnar's preemption patches. There was mention of this on lwn.net about this: http://lwn.net/Articles/108719/Those patches did only the conversion of static spinlock_t lock = SPIN_LOCK_UNLOCKED; lock = SPIN_LOCK_UNLOCKED; to static DEFINE_SPINLOCK(lock); spin_lock_init(lock);
First: I didn't write the patch, only forwarded it, so I just guessed why it was done. The latter is spin_lock_init(&lock); (since someone got confused about this). However, this is a .lock = SPIN_LOCK_UNLOCKED -> spin_lock_init(&struct.lock), so I don't understand what changes... the structure is initialized inside a function, so there's no change.
If you want to do static initialization inside of structures, then you have to define a seperate MACRO similar to the static initialization of list_head's inside of structures:
static struct sysfs_dirent sysfs_root = {
.s_sibling = LIST_HEAD_INIT(sysfs_root.s_sibling),
I don't see the need here... and the initialization is not in static code; it
changes this code snippet:
void *port_data(int port_num)
{
//...
*port = ((struct port_list)
{ .list = LIST_HEAD_INIT(port->list),
.has_connection = 0,
.sem = __SEMAPHORE_INITIALIZER(port->sem,
0),
.lock = SPIN_LOCK_UNLOCKED,
.port = port_num,
.fd = fd,
.pending = LIST_HEAD_INIT(port->pending),
.connections =
LIST_HEAD_INIT(port->connections) });
So you are all doing some confusion (in fact I guess Andrew realized this when
he merged this anyway).
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade