Real-Time Preemption V0.7.52-07: rt_init_MUTEX_LOCKED declaration
PATCH 1/1 Real-Time Preemption V0.7.52-07: rt_init_MUTEX_LOCKED declaration
This patch fixes broken rt_init_MUTEX_LOCKED declaration using rt_sema_init() macro. This way we fix a potential compile bug: rt_init_MUTEX_LOCKED calls there_is_no_init_MUTEX_LOCKED_for_RT_semaphores, which is not referenced. (e.g. drivers/char/watchdog/cpu5wdt.c: "cpu5wdt: Unknown symbol there_is_no_init_MUTEX_LOCKED_for_RT_semaphores")
--- realtime-preempt-2.6.13-rc4-RT-V0.7.52-07.orig 2005-07-31 23:35:18.000000000
+0000
+++ realtime-preempt-2.6.13-rc4-RT-V0.7.52-07 2005-08-01 15:59:31.000000000 +0000
@@ -8342,16 +8342,6 @@ Index: linux/drivers/cpufreq/cpufreq.c
===================================================================
--- linux.orig/drivers/cpufreq/cpufreq.c
+++ linux/drivers/cpufreq/cpufreq.c
-@@ -605,7 +605,8 @@ static int cpufreq_add_dev (struct sys_d
- policy->cpu = cpu;
- policy->cpus = cpumask_of_cpu(cpu);
-
-- init_MUTEX_LOCKED(&policy->lock);
-+ init_MUTEX(&policy->lock);
-+ down(&policy->lock);
- init_completion(&policy->kobj_unregister);
- INIT_WORK(&policy->update, handle_update, (void *)(long)cpu);
-
@@ -614,6 +615,7 @@ static int cpufreq_add_dev (struct sys_d
*/
ret = cpufreq_driver->init(policy);
@@ -14350,7 +14340,7 @@ Index: linux/include/linux/rt_lock.h
===================================================================
--- /dev/null
+++ linux/include/linux/rt_lock.h
-@@ -0,0 +1,391 @@
+@@ -0,0 +1,385 @@
+#ifndef __LINUX_RT_LOCK_H
+#define __LINUX_RT_LOCK_H
+
@@ -14589,14 +14579,8 @@ Index: linux/include/linux/rt_lock.h
+extern void FASTCALL(__init_MUTEX(struct semaphore *sem, char *name, char
*file, int line));
+#define rt_init_MUTEX(sem) \
+ __init_MUTEX(sem, #sem, __FILE__, __LINE__)
-+
-+extern void there_is_no_init_MUTEX_LOCKED_for_RT_semaphores(void); -+
-+/*
-+ * No locked initialization for RT semaphores
-+ */
+#define rt_init_MUTEX_LOCKED(sem) \
-+ there_is_no_init_MUTEX_LOCKED_for_RT_semaphores()
++ rt_sema_init(sem, 0)
+extern void FASTCALL(rt_down(struct semaphore *sem));
+extern int FASTCALL(rt_down_interruptible(struct semaphore *sem)); +extern int FASTCALL(rt_down_trylock(struct semaphore *sem));
Regards,
Signature · 1 line
Luca
Quoted 7 lines · Luca Falavigna
This patch fixes broken
rt_init_MUTEX_LOCKEDdeclaration usingrt_sema_init()macro. This way we fix a potential compile bug:rt_init_MUTEX_LOCKEDcallsthere_is_no_init_MUTEX_LOCKED_for_RT_semaphores, which is not referenced. (e.g.drivers/char/watchdog/cpu5wdt.c: "cpu5wdt: Unknown symbolthere_is_no_init_MUTEX_LOCKED_for_RT_semaphores")
Ingo,
When did you solve the problem of ownership of locked semaphores? ;-)
Luca,
Unless Ingo did solve the problem of semaphores that can be locked by one task and unlocked by another task, I wouldn't use your patch. There's a problem with priority inheritance when it comes to these semaphores. That is who owns a locked semaphore that will later be unlocked by someone else? When a RT process blocks on this semaphore, who does it boost to release it? Ingo purposely put this in to crash the compile so that we know where this can be a problem right away.
The patch you wanted to send was:
drivers/char/watchdog/cpu5wdt.c +1 -1
@@ -56,7 +56,7 @@/* some device data */ static struct { - struct semaphore stop; + struct compat_semaphore stop; volatile int running; struct timer_list timer; volatile int queue;
Quoted 2 lines
Ingo purposely put this in to crash
the compile so that we know where this can be a problem right away.
And it works nice ;)
Quoted 9 lines
The patch you wanted to send was:
Signed-off-by Steven Rostedt <[email protected]>Index:
linux_realtime_ernie/drivers/char/watchdog/cpu5wdt.c
===================================================================
drivers/char/watchdog/cpu5wdt.c (working copy) line 62
Quoted 8 lines
drivers/char/watchdog/cpu5wdt.c (working copy) renamed from drivers/char/watchdog/cpu5wdt.c (revision 265) +1
-1@@ -56,7 +56,7 @@ /* some device data */ static struct { - struct semaphore stop; + struct compat_semaphore stop; volatile int running; struct timer_list timer; volatile int queue;
Another solution could be this (as shown in drivers/cpufreq/cpufreq.c):
- init_MUTEX_LOCKED(&policy->lock);
+ init_MUTEX(&policy->lock);
+ down(&policy->lock);
Quoted 19 lines · Luca Falavigna
Quoted 9 lines
The patch you wanted to send was:
Signed-off-by Steven Rostedt <[email protected]>Index:
linux_realtime_ernie/drivers/char/watchdog/cpu5wdt.c
===================================================================drivers/char/watchdog/cpu5wdt.c (working copy) line 62
Quoted 8 lines
drivers/char/watchdog/cpu5wdt.c (working copy) renamed from drivers/char/watchdog/cpu5wdt.c (revision 265) +1
-1@@ -56,7 +56,7 @@ /* some device data */ static struct { - struct semaphore stop; + struct compat_semaphore stop; volatile int running; struct timer_list timer; volatile int queue;Another solution could be this (as shown in
drivers/cpufreq/cpufreq.c):
Review
Quoted 4 lines
(quoted diff) +2
-2- init_MUTEX_LOCKED(&policy->lock); + init_MUTEX(&policy->lock); + down(&policy->lock); -
If the semaphore is being used as a mutex,
then it could be converted to an RT-mutex.
That would nicely side-step the problem.
Note this won't work if you are counting with the sema.
Sven
Quoted 12 lines · Sven-Thorsten Dietrich
Review
Quoted 7 lines · Luca Falavigna
Another solution could be this (as shown in
drivers/cpufreq/cpufreq.c):(quoted diff) +2
-2- init_MUTEX_LOCKED(&policy->lock); + init_MUTEX(&policy->lock); + down(&policy->lock); -If the semaphore is being used as a mutex,
then it could be converted to an RT-mutex.
Quoted 3 lines
That would nicely side-step the problem.
Note this won't work if you are counting with the sema.
Well it's not really a counter. It's one of these situations where the semaphore is locked on init, later someone else ups it on a trigger, and then the most horrible, it gets down again on module exit! I'm not really sure what it is doing. The only place that an up is called is on the watchdog trigger. I guess it's being used to not let you unload the module if the watchdog hasn't gone off yet, or something to that effect.
-- Steve
Quoted 6 lines · Luca Falavigna
This patch fixes broken
rt_init_MUTEX_LOCKEDdeclaration usingrt_sema_init()macro. This way we fix a potential compile bug:rt_init_MUTEX_LOCKEDcallsthere_is_no_init_MUTEX_LOCKED_for_RT_semaphores, which is not referenced. (e.g.drivers/char/watchdog/cpu5wdt.c: "cpu5wdt: Unknown symbolthere_is_no_init_MUTEX_LOCKED_for_RT_semaphores")
the right solution would be to mark policy->lock as a compat_semaphore. That will revert things back to the stock semantics. (at the price of not having PI, which isnt a big issue in this case.)
Quoted 1 line
-+extern void
there_is_no_init_MUTEX_LOCKED_for_RT_semaphores(void);
the reason for not allowing init_MUTEX_LOCKED() is that in basically every case that does it, the semaphore is not used as a true mutex in the strict sense. So the affected semaphore should be changed to compat_semaphore, to gain full semantics.
Ingo
drivers/char/watchdog/cpu5wdt.c line 59
Quoted 2 lines · Steven Rostedt
drivers/char/watchdog/cpu5wdt.c +1
-1-struct semaphore stop; +struct compat_semaphore stop;
i think it's policy->lock that is the issue here?
Ingo
Quoted 7 lines · Ingo Molnar
drivers/char/watchdog/cpu5wdt.c line 59
Quoted 2 lines · Steven Rostedt
drivers/char/watchdog/cpu5wdt.c +1
-1-struct semaphore stop; +struct compat_semaphore stop;i think it's
policy->lockthat is the issue here?
I was looking at Luca's original message where he showed the bug of -- drivers/char/watchdog/cpu5wdt.c: "cpu5wdt: Unknown symbol there_is_no_init_MUTEX_LOCKED_for_RT_semaphores") -- Looking into this file the only init_MUTEX_LOCKED that I found was
static int __devinit cpu5wdt_init(void)
{
unsigned int val;
int err;[...]
/* watchdog reboot? */ val = inb(port + CPU5WDT_STATUS_REG); val = (val >> 2) & 1; if ( !val ) printk(KERN_INFO PFX "sorry, was my fault\n"); init_MUTEX_LOCKED(&cpu5wdt_device.stop); cpu5wdt_device.queue = 0; clear_bit(0, &cpu5wdt_device.inuse);
Here I see that cpu5wdt_device.stop is being initialized with init_MUTEX_LOCKED, so that is what I went to fix. I even added the driver to my config and compiled it before sending it in. I don't have the device, but the driver compiled :-)
-- Steve
Quoted 34 lines · Steven Rostedt
Quoted 7 lines · Ingo Molnar
drivers/char/watchdog/cpu5wdt.c line 59
Quoted 2 lines · Steven Rostedt
drivers/char/watchdog/cpu5wdt.c +1
-1-struct semaphore stop; +struct compat_semaphore stop;i think it's
policy->lockthat is the issue here?I was looking at Luca's original message where he showed the bug of --
drivers/char/watchdog/cpu5wdt.c: "cpu5wdt: Unknown symbolthere_is_no_init_MUTEX_LOCKED_for_RT_semaphores") -- Looking into this file the onlyinit_MUTEX_LOCKEDthat I found wasstatic int
__devinitcpu5wdt_init(void){ unsigned int val; int err;[...]
/* watchdog reboot? */ val = inb(port + CPU5WDT_STATUS_REG); val = (val >> 2) & 1; if ( !val ) printk(KERN_INFO PFX "sorry, was my fault\n"); init_MUTEX_LOCKED(&cpu5wdt_device.stop); cpu5wdt_device.queue = 0; clear_bit(0, &cpu5wdt_device.inuse);
Quoted 4 lines
Here I see that cpu5wdt_device.stop is being initialized with
init_MUTEX_LOCKED, so that is what I went to fix. I even added the driver to my config and compiled it before sending it in. I don't have the device, but the driver compiled :-)
ok - the fix is in -52-10.
Ingo