Skip to contentSkip to conversation

Fixes for IPMI use of timers

PATCH In review Corey Minyard 3 participants 6 messages May 19, 2005 – May 21, 2005 IPMI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fixes for IPMI use of timers

Fix some problems with the high-res timer support.

drivers/char/ipmi/ipmi_si_intf.c +4 -3
Index: linux-2.6.12-rc4/drivers/char/ipmi/ipmi_si_intf.c
===================================================================
@@ -769,10 +769,11 @@
 
 		/* We already have irqsave on, so no need for it
                    here. */
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);
 		jiffies_now = jiffies;
 		smi_info->si_timer.expires = jiffies_now;
 		smi_info->si_timer.sub_expires = get_arch_cycles(jiffies_now);
+		read_unlock_irqrestore(&xtime_lock, flags);
 
 		add_usec_to_timer(&smi_info->si_timer, SI_SHORT_TIMEOUT_USEC);
 
@@ -830,11 +831,11 @@
 		smi_info->short_timeouts++;
 		spin_unlock_irqrestore(&smi_info->count_lock, flags);
 #if defined(CONFIG_HIGH_RES_TIMERS)
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);
                 smi_info->si_timer.expires = jiffies;
                 smi_info->si_timer.sub_expires
                         = get_arch_cycles(smi_info->si_timer.expires);
-                read_unlock(&xtime_lock);
+		read_unlock_irqrestore(&xtime_lock, flags);
 		add_usec_to_timer(&smi_info->si_timer, SI_SHORT_TIMEOUT_USEC);
 #else
 		smi_info->si_timer.expires = jiffies + 1;
#1 lore raw

Review

Quoted 4 lines · Corey Minyard
                /* We already have irqsave on, so no need for it
                    here. */
(quoted diff) +1 -1
-               read_lock(&xtime_lock);
+               read_lock_irqsave(&xtime_lock, flags); 

The comment is now wrong.

Lee

#2 lore raw
Quoted 10 lines · Lee Revell

Review

Quoted 6 lines · Corey Minyard
               /* We already have irqsave on, so no need for it
                   here. */
(quoted diff) +1 -1
-               read_lock(&xtime_lock);
+               read_lock_irqsave(&xtime_lock, flags); 
Quoted 5 lines

The comment is now wrong.

Lee

I know I deleted that at least once. Oh well, here it is with the comment removed.

Thanks.

-Corey

Fix some problems with the high-res timer support.

drivers/char/ipmi/ipmi_si_intf.c +4 -5
Index: linux-2.6.12-rc4/drivers/char/ipmi/ipmi_si_intf.c
===================================================================
@@ -767,12 +767,11 @@
 		   immediately, anyway.  So we only process if we
 		   actually delete the timer. */
 
-		/* We already have irqsave on, so no need for it
-                   here. */
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);
 		jiffies_now = jiffies;
 		smi_info->si_timer.expires = jiffies_now;
 		smi_info->si_timer.sub_expires = get_arch_cycles(jiffies_now);
+		read_unlock_irqrestore(&xtime_lock, flags);
 
 		add_usec_to_timer(&smi_info->si_timer, SI_SHORT_TIMEOUT_USEC);
 
@@ -830,11 +829,11 @@
 		smi_info->short_timeouts++;
 		spin_unlock_irqrestore(&smi_info->count_lock, flags);
 #if defined(CONFIG_HIGH_RES_TIMERS)
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);
                 smi_info->si_timer.expires = jiffies;
                 smi_info->si_timer.sub_expires
                         = get_arch_cycles(smi_info->si_timer.expires);
-                read_unlock(&xtime_lock);
+		read_unlock_irqrestore(&xtime_lock, flags);
 		add_usec_to_timer(&smi_info->si_timer, SI_SHORT_TIMEOUT_USEC);
 #else
 		smi_info->si_timer.expires = jiffies + 1;
#3 lore raw
In reply to Corey Minyard
Quoted 13 lines · Corey Minyard

------------------------------------------------------------------------

Fix some problems with the high-res timer support.

Index: linux-2.6.12-rc4/drivers/char/ipmi/ipmi_si_intf.c
===================================================================

drivers/char/ipmi/ipmi_si_intf.c line 772

Quoted 5 lines
drivers/char/ipmi/ipmi_si_intf.c +1 -1
@@ -769,10 +769,11 @@
 
 		/* We already have irqsave on, so no need for it
                    here. */
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);

I rather hope this fails to compile :) xtime_lock is a sequence lock in the 2.6 kernel.

Quoted 21 lines
drivers/char/ipmi/ipmi_si_intf.c +3 -2
 		jiffies_now = jiffies;
 		smi_info->si_timer.expires = jiffies_now;
 		smi_info->si_timer.sub_expires = get_arch_cycles(jiffies_now);
+		read_unlock_irqrestore(&xtime_lock, flags);
 
 		add_usec_to_timer(&smi_info->si_timer, SI_SHORT_TIMEOUT_USEC);
 
@@ -830,11 +831,11 @@
 		smi_info->short_timeouts++;
 		spin_unlock_irqrestore(&smi_info->count_lock, flags);
 #if defined(CONFIG_HIGH_RES_TIMERS)
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);
                 smi_info->si_timer.expires = jiffies;
                 smi_info->si_timer.sub_expires
                         = get_arch_cycles(smi_info->si_timer.expires);
-                read_unlock(&xtime_lock);
+		read_unlock_irqrestore(&xtime_lock, flags);
 		add_usec_to_timer(&smi_info->si_timer, SI_SHORT_TIMEOUT_USEC);
 #else
 		smi_info->si_timer.expires = jiffies + 1;
Signature · 2 lines
George Anzinger   [email protected]
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
#4 lore raw
Quoted 8 lines · George Anzinger

drivers/char/ipmi/ipmi_si_intf.c line 772

Quoted 4 lines
drivers/char/ipmi/ipmi_si_intf.c +1 -1
 		/* We already have irqsave on, so no need for it
                    here. */
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);

I rather hope this fails to compile :) xtime_lock is a sequence lock in the 2.6 kernel.

Quoted 4 lines

drivers/char/ipmi/ipmi_si_intf.c line 776

Quoted 4 lines
drivers/char/ipmi/ipmi_si_intf.c +1 -0
 		jiffies_now = jiffies;
 		smi_info->si_timer.expires = jiffies_now;
 		smi_info->si_timer.sub_expires = get_arch_cycles(jiffies_now);
+		read_unlock_irqrestore(&xtime_lock, flags);

The old code clearly has an unbalanced lock/unlock, maybe it's sufficient to just fix that?

Lee

#5 lore raw
Quoted 13 lines · Lee Revell
Quoted 10 lines · George Anzinger

Review

Quoted 6 lines
		/* We already have irqsave on, so no need for it
                   here. */
(quoted diff) +1 -1
-		read_lock(&xtime_lock);
+		read_lock_irqsave(&xtime_lock, flags);

I rather hope this fails to compile :) xtime_lock is a sequence lock in the 2.6 kernel.

Quoted 11 lines
Quoted 6 lines

Review

Quoted 6 lines
		jiffies_now = jiffies;
		smi_info->si_timer.expires = jiffies_now;
		smi_info->si_timer.sub_expires = get_arch_cycles(jiffies_now);
(quoted diff) +1 -0
+		read_unlock_irqrestore(&xtime_lock, flags);

The old code clearly has an unbalanced lock/unlock, maybe it's sufficient to just fix that?

Yes, probably just best to fix that up. When high-res timers make it into the kernel, this can be fixed up, this is left over from the 2.4 high-res timers.

-Corey