2005-09-27 18:32:14

by Daniel Walker

[permalink] [raw]
Subject: [PATCH] RT: epca_lock to DEFINE_SPINLOCK


Convert epca_lock to the new syntax.

Signed-Off-By: Daniel Walker <[email protected]>

Index: linux-2.6.13/drivers/char/epca.c
===================================================================
--- linux-2.6.13.orig/drivers/char/epca.c
+++ linux-2.6.13/drivers/char/epca.c
@@ -80,7 +80,7 @@ static int invalid_lilo_config;
/* The ISA boards do window flipping into the same spaces so its only sane
with a single lock. It's still pretty efficient */

-static spinlock_t epca_lock = SPIN_LOCK_UNLOCKED;
+static DEFINE_SPINLOCK(epca_lock);

/* -----------------------------------------------------------------------
MAXBOARDS is typically 12, but ISA and EISA cards are restricted to



2005-09-27 22:33:16

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

On Tue, 2005-09-27 at 11:32 -0700, Daniel Walker wrote:
> Convert epca_lock to the new syntax.
>
> Signed-Off-By: Daniel Walker <[email protected]>
>
> Index: linux-2.6.13/drivers/char/epca.c
> ===================================================================
> --- linux-2.6.13.orig/drivers/char/epca.c
> +++ linux-2.6.13/drivers/char/epca.c
> @@ -80,7 +80,7 @@ static int invalid_lilo_config;
> /* The ISA boards do window flipping into the same spaces so its only sane
> with a single lock. It's still pretty efficient */
>
> -static spinlock_t epca_lock = SPIN_LOCK_UNLOCKED;
> +static DEFINE_SPINLOCK(epca_lock);

Please submit also to Andrew/subsystem maintainer. Thats not a -RT
related issue.

tglx


2005-09-28 09:32:49

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK


* Daniel Walker <[email protected]> wrote:

> Convert epca_lock to the new syntax.

thanks, applied.

Ingo

2005-09-28 09:39:24

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

On Tue, 2005-09-27 at 11:32 -0700, Daniel Walker wrote:
> Convert epca_lock to the new syntax.
>
> Signed-Off-By: Daniel Walker <[email protected]>
>
> Index: linux-2.6.13/drivers/char/epca.c
> ===================================================================
> --- linux-2.6.13.orig/drivers/char/epca.c
> +++ linux-2.6.13/drivers/char/epca.c
> @@ -80,7 +80,7 @@ static int invalid_lilo_config;
> /* The ISA boards do window flipping into the same spaces so its only sane
> with a single lock. It's still pretty efficient */
>
> -static spinlock_t epca_lock = SPIN_LOCK_UNLOCKED;
> +static DEFINE_SPINLOCK(epca_lock);
>
> /* ------------------

this is really ugly though; at minimum a DEFINE_STATIC_SPINLOCK() would
be needed to make this less ugly.


2005-09-28 15:56:36

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

On Wed, 2005-09-28 at 11:39 +0200, Arjan van de Ven wrote:

> this is really ugly though; at minimum a DEFINE_STATIC_SPINLOCK() would
> be needed to make this less ugly.


Doesn't exists as far as I know .. Shall we make one ?

Daniel

2005-09-28 16:07:51

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

Arjan> this is really ugly though; at minimum a DEFINE_STATIC_SPINLOCK()
Arjan> would be needed to make this less ugly.

huh? This is a totally standard kernel idiom -- just do

grep -Er 'static (DECLARE|DEFINE)' .

in a kernel tree to see how prevalent it is.

- R.

2005-09-28 20:43:45

by Nikita Danilov

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

Roland Dreier writes:
> Arjan> this is really ugly though; at minimum a DEFINE_STATIC_SPINLOCK()
> Arjan> would be needed to make this less ugly.
>
> huh? This is a totally standard kernel idiom -- just do
>
> grep -Er 'static (DECLARE|DEFINE)' .
>
> in a kernel tree to see how prevalent it is.

It may be widely used and still ugly. The general problem with
DEFINE_FOO() macros is that they obfuscate things: they do not _look_
like C variable declarations, and, in particular, type of variable is
not immediately obvious.

The only reasonable case where DEFINE_FOO(x) is really necessary is when
initializer uses address of x, but even in that case something like

spinlock_t guard = SPINLOCK_UNLOCKED(guard);

is much more readable than

DEFINE_SPIN_LOCK(guard);

The question is: does RT really have to force DEFINE_* as the only way
to define things?

>
> - R.

Nikita.

2005-09-29 06:49:52

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK


On Thu, 29 Sep 2005, Nikita Danilov wrote:

>
> The only reasonable case where DEFINE_FOO(x) is really necessary is when
> initializer uses address of x, but even in that case something like
>
> spinlock_t guard = SPINLOCK_UNLOCKED(guard);
>
> is much more readable than
>
> DEFINE_SPIN_LOCK(guard);
>

Except that the former is also error prone. I just found a bug in my code
(I customize Ingo's RT kernel) where I had a cut and paste error:

spinlock_t a = SPINLOCK_UNLOCKED(a);
spinlock_t b = SPINLOCK_UNLOCKED(a);

This took me two days to find since the problems occurred elsewhere.

-- Steve