Re: [RFC] spinlock_t & rwlock_t break_lock member initialization (patch seeking comments included)
On Mon, 28 Mar 2005, Jesper Juhl wrote: > On Sun, 27 Mar 2005, Zwane Mwaikambo wrote: > > > On Sun, 27 Mar 2005, Jesper Juhl wrote: > > > > > I've now been running kernels (both PREEMPT, SMP, both and without both) > > > with the patch below applied for a few days and I see no ill effects. I'm > > > still interrested in comments about wether or not something like this > > > makes sense and is acceptable ? > > > > The concept seems fine to me, although i think you should be using named > > initialisers instead. > > > I wrote it the way I did to keep it similar to how the other members were > initialized, I wouldn't mind a different approach, but doing it this way > seemed to be what would "fit in" best, and it did the trick for me > (silenced the annoying warnings) and seemed safe. > > Thank you for taking the time to comment on this trivial issue. Well your original patch would probably cause the least noise amongst the other patches changing code in that area, perhaps try pushing it. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t rwlock_t break_lock member initialization (patch seeking comments included)
On Mon, 28 Mar 2005, Jesper Juhl wrote: On Sun, 27 Mar 2005, Zwane Mwaikambo wrote: On Sun, 27 Mar 2005, Jesper Juhl wrote: I've now been running kernels (both PREEMPT, SMP, both and without both) with the patch below applied for a few days and I see no ill effects. I'm still interrested in comments about wether or not something like this makes sense and is acceptable ? The concept seems fine to me, although i think you should be using named initialisers instead. I wrote it the way I did to keep it similar to how the other members were initialized, I wouldn't mind a different approach, but doing it this way seemed to be what would fit in best, and it did the trick for me (silenced the annoying warnings) and seemed safe. Thank you for taking the time to comment on this trivial issue. Well your original patch would probably cause the least noise amongst the other patches changing code in that area, perhaps try pushing it. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t & rwlock_t break_lock member initialization (patch seeking comments included)
On Sun, 27 Mar 2005, Zwane Mwaikambo wrote: > On Sun, 27 Mar 2005, Jesper Juhl wrote: > > > I've now been running kernels (both PREEMPT, SMP, both and without both) > > with the patch below applied for a few days and I see no ill effects. I'm > > still interrested in comments about wether or not something like this > > makes sense and is acceptable ? > > The concept seems fine to me, although i think you should be using named > initialisers instead. > I wrote it the way I did to keep it similar to how the other members were initialized, I wouldn't mind a different approach, but doing it this way seemed to be what would "fit in" best, and it did the trick for me (silenced the annoying warnings) and seemed safe. Thank you for taking the time to comment on this trivial issue. -- Jesper - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t & rwlock_t break_lock member initialization (patch seeking comments included)
On Sun, 27 Mar 2005, Jesper Juhl wrote: > I've now been running kernels (both PREEMPT, SMP, both and without both) > with the patch below applied for a few days and I see no ill effects. I'm > still interrested in comments about wether or not something like this > makes sense and is acceptable ? The concept seems fine to me, although i think you should be using named initialisers instead. Thanks Jesper, Zwane - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t & rwlock_t break_lock member initialization (patch seeking comments included)
I've now been running kernels (both PREEMPT, SMP, both and without both)
with the patch below applied for a few days and I see no ill effects. I'm
still interrested in comments about wether or not something like this
makes sense and is acceptable ?
--
Jesper Juhl
On Sun, 20 Mar 2005, Jesper Juhl wrote:
>
> I'm often building the tree with gcc -W to look for potential trouble
> spots, and of course I see a lot of warning messages. I'm well aware that
> most of these don't indicate actual problems and should just be ignored,
> but the less warnings there are the easier it is to zoom in on the ones
> that might actually matter, so I try to also locate those that can be
> silenced safely even when they don't indicate a real problem - just to cut
> down on the number of warnings I have to sift through.
> One class of warnings that belong in the "not really a problem but I
> believe I can silence it without doing any harm" category are these :
>
> include/linux/wait.h:82: warning: missing initializer
> include/linux/wait.h:82: warning: (near initialization for
> `(anonymous).break_lock')
>
> include/asm/rwsem.h:88: warning: missing initializer
> include/asm/rwsem.h:88: warning: (near initialization for
> `(anonymous).break_lock')
>
> These stem from the fact that when you enable CONFIG_PREEMPT spinlock_t
> and rwlock_t each gain an extra member "break_lock", but the lock
> initialization code neglects to initialize this extra member in that case.
>
> If you enable CONFIG_PREEMPT and build with gcc -W you'll see a *lot* of
> those, since spinlocks and rwlocks are used all over the place.
>
> I've come up with a patch to that silence those warnings by making sure
> the "break_lock" member will be initialized when CONFIG_PREEMPT is
> enabled.
>
> I would like to know if a patch like this is welcome or just considered
> clutter for no real gain. I would also like to know if I've overlooked
> some implications of doing this - it seems to me that this should be
> completely safe to do and without significant overhead, but I'm also well
> aware that my knowledge of this code is quite shallow, so I need comments
> (especially since lock performance is quite performance critical, so I
> don't want to screw up here).
>
> Here's the patch I came up with - comments are very welcome.
>
>
> (no Signed-off-by since this is not intended to be merged just yet)
>
> --- linux-2.6.11-mm4-orig/include/asm-i386/spinlock.h 2005-03-02
> 08:37:50.0 +0100
> +++ linux-2.6.11-mm4/include/asm-i386/spinlock.h 2005-03-20
> 22:40:46.0 +0100
> @@ -32,7 +32,13 @@
> #define SPINLOCK_MAGIC_INIT /* */
> #endif
>
> -#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT }
> +#ifdef CONFIG_PREEMPT
> +#define SPINLOCK_BREAK_INIT , 0
> +#else
> +#define SPINLOCK_BREAK_INIT /* */
> +#endif
> +
> +#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT
> SPINLOCK_BREAK_INIT }
>
> #define spin_lock_init(x)do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
>
> @@ -182,7 +188,13 @@
> #define RWLOCK_MAGIC_INIT/* */
> #endif
>
> -#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT }
> +#ifdef CONFIG_PREEMPT
> +#define RWLOCK_BREAK_INIT, 0
> +#else
> +#define RWLOCK_BREAK_INIT/* */
> +#endif
> +
> +#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT
> RWLOCK_BREAK_INIT }
>
> #define rwlock_init(x) do { *(x) = RW_LOCK_UNLOCKED; } while(0)
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t rwlock_t break_lock member initialization (patch seeking comments included)
I've now been running kernels (both PREEMPT, SMP, both and without both)
with the patch below applied for a few days and I see no ill effects. I'm
still interrested in comments about wether or not something like this
makes sense and is acceptable ?
--
Jesper Juhl
On Sun, 20 Mar 2005, Jesper Juhl wrote:
I'm often building the tree with gcc -W to look for potential trouble
spots, and of course I see a lot of warning messages. I'm well aware that
most of these don't indicate actual problems and should just be ignored,
but the less warnings there are the easier it is to zoom in on the ones
that might actually matter, so I try to also locate those that can be
silenced safely even when they don't indicate a real problem - just to cut
down on the number of warnings I have to sift through.
One class of warnings that belong in the not really a problem but I
believe I can silence it without doing any harm category are these :
include/linux/wait.h:82: warning: missing initializer
include/linux/wait.h:82: warning: (near initialization for
`(anonymous).break_lock')
include/asm/rwsem.h:88: warning: missing initializer
include/asm/rwsem.h:88: warning: (near initialization for
`(anonymous).break_lock')
These stem from the fact that when you enable CONFIG_PREEMPT spinlock_t
and rwlock_t each gain an extra member break_lock, but the lock
initialization code neglects to initialize this extra member in that case.
If you enable CONFIG_PREEMPT and build with gcc -W you'll see a *lot* of
those, since spinlocks and rwlocks are used all over the place.
I've come up with a patch to that silence those warnings by making sure
the break_lock member will be initialized when CONFIG_PREEMPT is
enabled.
I would like to know if a patch like this is welcome or just considered
clutter for no real gain. I would also like to know if I've overlooked
some implications of doing this - it seems to me that this should be
completely safe to do and without significant overhead, but I'm also well
aware that my knowledge of this code is quite shallow, so I need comments
(especially since lock performance is quite performance critical, so I
don't want to screw up here).
Here's the patch I came up with - comments are very welcome.
(no Signed-off-by since this is not intended to be merged just yet)
--- linux-2.6.11-mm4-orig/include/asm-i386/spinlock.h 2005-03-02
08:37:50.0 +0100
+++ linux-2.6.11-mm4/include/asm-i386/spinlock.h 2005-03-20
22:40:46.0 +0100
@@ -32,7 +32,13 @@
#define SPINLOCK_MAGIC_INIT /* */
#endif
-#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT }
+#ifdef CONFIG_PREEMPT
+#define SPINLOCK_BREAK_INIT , 0
+#else
+#define SPINLOCK_BREAK_INIT /* */
+#endif
+
+#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT
SPINLOCK_BREAK_INIT }
#define spin_lock_init(x)do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
@@ -182,7 +188,13 @@
#define RWLOCK_MAGIC_INIT/* */
#endif
-#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT }
+#ifdef CONFIG_PREEMPT
+#define RWLOCK_BREAK_INIT, 0
+#else
+#define RWLOCK_BREAK_INIT/* */
+#endif
+
+#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT
RWLOCK_BREAK_INIT }
#define rwlock_init(x) do { *(x) = RW_LOCK_UNLOCKED; } while(0)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t rwlock_t break_lock member initialization (patch seeking comments included)
On Sun, 27 Mar 2005, Jesper Juhl wrote: I've now been running kernels (both PREEMPT, SMP, both and without both) with the patch below applied for a few days and I see no ill effects. I'm still interrested in comments about wether or not something like this makes sense and is acceptable ? The concept seems fine to me, although i think you should be using named initialisers instead. Thanks Jesper, Zwane - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] spinlock_t rwlock_t break_lock member initialization (patch seeking comments included)
On Sun, 27 Mar 2005, Zwane Mwaikambo wrote: On Sun, 27 Mar 2005, Jesper Juhl wrote: I've now been running kernels (both PREEMPT, SMP, both and without both) with the patch below applied for a few days and I see no ill effects. I'm still interrested in comments about wether or not something like this makes sense and is acceptable ? The concept seems fine to me, although i think you should be using named initialisers instead. I wrote it the way I did to keep it similar to how the other members were initialized, I wouldn't mind a different approach, but doing it this way seemed to be what would fit in best, and it did the trick for me (silenced the annoying warnings) and seemed safe. Thank you for taking the time to comment on this trivial issue. -- Jesper - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC] spinlock_t & rwlock_t break_lock member initialization (patch seeking comments included)
I'm often building the tree with gcc -W to look for potential trouble
spots, and of course I see a lot of warning messages. I'm well aware that
most of these don't indicate actual problems and should just be ignored,
but the less warnings there are the easier it is to zoom in on the ones
that might actually matter, so I try to also locate those that can be
silenced safely even when they don't indicate a real problem - just to cut
down on the number of warnings I have to sift through.
One class of warnings that belong in the "not really a problem but I
believe I can silence it without doing any harm" category are these :
include/linux/wait.h:82: warning: missing initializer
include/linux/wait.h:82: warning: (near initialization for
`(anonymous).break_lock')
include/asm/rwsem.h:88: warning: missing initializer
include/asm/rwsem.h:88: warning: (near initialization for
`(anonymous).break_lock')
These stem from the fact that when you enable CONFIG_PREEMPT spinlock_t
and rwlock_t each gain an extra member "break_lock", but the lock
initialization code neglects to initialize this extra member in that case.
If you enable CONFIG_PREEMPT and build with gcc -W you'll see a *lot* of
those, since spinlocks and rwlocks are used all over the place.
I've come up with a patch to that silence those warnings by making sure
the "break_lock" member will be initialized when CONFIG_PREEMPT is
enabled.
I would like to know if a patch like this is welcome or just considered
clutter for no real gain. I would also like to know if I've overlooked
some implications of doing this - it seems to me that this should be
completely safe to do and without significant overhead, but I'm also well
aware that my knowledge of this code is quite shallow, so I need comments
(especially since lock performance is quite performance critical, so I
don't want to screw up here).
Here's the patch I came up with - comments are very welcome.
(no Signed-off-by since this is not intended to be merged just yet)
--- linux-2.6.11-mm4-orig/include/asm-i386/spinlock.h 2005-03-02
08:37:50.0 +0100
+++ linux-2.6.11-mm4/include/asm-i386/spinlock.h2005-03-20
22:40:46.0 +0100
@@ -32,7 +32,13 @@
#define SPINLOCK_MAGIC_INIT/* */
#endif
-#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT }
+#ifdef CONFIG_PREEMPT
+#define SPINLOCK_BREAK_INIT, 0
+#else
+#define SPINLOCK_BREAK_INIT/* */
+#endif
+
+#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT
SPINLOCK_BREAK_INIT }
#define spin_lock_init(x) do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
@@ -182,7 +188,13 @@
#define RWLOCK_MAGIC_INIT /* */
#endif
-#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT }
+#ifdef CONFIG_PREEMPT
+#define RWLOCK_BREAK_INIT , 0
+#else
+#define RWLOCK_BREAK_INIT /* */
+#endif
+
+#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT
RWLOCK_BREAK_INIT }
#define rwlock_init(x) do { *(x) = RW_LOCK_UNLOCKED; } while(0)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC] spinlock_t rwlock_t break_lock member initialization (patch seeking comments included)
I'm often building the tree with gcc -W to look for potential trouble
spots, and of course I see a lot of warning messages. I'm well aware that
most of these don't indicate actual problems and should just be ignored,
but the less warnings there are the easier it is to zoom in on the ones
that might actually matter, so I try to also locate those that can be
silenced safely even when they don't indicate a real problem - just to cut
down on the number of warnings I have to sift through.
One class of warnings that belong in the not really a problem but I
believe I can silence it without doing any harm category are these :
include/linux/wait.h:82: warning: missing initializer
include/linux/wait.h:82: warning: (near initialization for
`(anonymous).break_lock')
include/asm/rwsem.h:88: warning: missing initializer
include/asm/rwsem.h:88: warning: (near initialization for
`(anonymous).break_lock')
These stem from the fact that when you enable CONFIG_PREEMPT spinlock_t
and rwlock_t each gain an extra member break_lock, but the lock
initialization code neglects to initialize this extra member in that case.
If you enable CONFIG_PREEMPT and build with gcc -W you'll see a *lot* of
those, since spinlocks and rwlocks are used all over the place.
I've come up with a patch to that silence those warnings by making sure
the break_lock member will be initialized when CONFIG_PREEMPT is
enabled.
I would like to know if a patch like this is welcome or just considered
clutter for no real gain. I would also like to know if I've overlooked
some implications of doing this - it seems to me that this should be
completely safe to do and without significant overhead, but I'm also well
aware that my knowledge of this code is quite shallow, so I need comments
(especially since lock performance is quite performance critical, so I
don't want to screw up here).
Here's the patch I came up with - comments are very welcome.
(no Signed-off-by since this is not intended to be merged just yet)
--- linux-2.6.11-mm4-orig/include/asm-i386/spinlock.h 2005-03-02
08:37:50.0 +0100
+++ linux-2.6.11-mm4/include/asm-i386/spinlock.h2005-03-20
22:40:46.0 +0100
@@ -32,7 +32,13 @@
#define SPINLOCK_MAGIC_INIT/* */
#endif
-#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT }
+#ifdef CONFIG_PREEMPT
+#define SPINLOCK_BREAK_INIT, 0
+#else
+#define SPINLOCK_BREAK_INIT/* */
+#endif
+
+#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 SPINLOCK_MAGIC_INIT
SPINLOCK_BREAK_INIT }
#define spin_lock_init(x) do { *(x) = SPIN_LOCK_UNLOCKED; } while(0)
@@ -182,7 +188,13 @@
#define RWLOCK_MAGIC_INIT /* */
#endif
-#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT }
+#ifdef CONFIG_PREEMPT
+#define RWLOCK_BREAK_INIT , 0
+#else
+#define RWLOCK_BREAK_INIT /* */
+#endif
+
+#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT
RWLOCK_BREAK_INIT }
#define rwlock_init(x) do { *(x) = RW_LOCK_UNLOCKED; } while(0)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
