[PATCH] RT: epca_lock to DEFINE_SPINLOCK

[PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Daniel Walker]

Convert epca_lock to the new syntax.

Signed-Off-By: Daniel Walker <dwalker@mvista.com>

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 

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Thomas Gleixner]

On Tue, 2005-09-27 at 11:32 -0700, Daniel Walker wrote:
> Convert epca_lock to the new syntax.
> 
> Signed-Off-By: Daniel Walker <dwalker@mvista.com>
> 
> 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

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> Convert epca_lock to the new syntax.

thanks, applied.

	Ingo

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Arjan van de Ven]

On Tue, 2005-09-27 at 11:32 -0700, Daniel Walker wrote:
> Convert epca_lock to the new syntax.
> 
> Signed-Off-By: Daniel Walker <dwalker@mvista.com>
> 
> 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.

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Daniel Walker]

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 

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Roland Dreier]

    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.

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Nikita Danilov]

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.

Re: [PATCH] RT: epca_lock to DEFINE_SPINLOCK

[Steven Rostedt]

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