public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: 2.6.13-rt3
@ 2005-09-01 16:24 Steven Rostedt
  2005-09-01 16:46 ` 2.6.13-rt3 Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2005-09-01 16:24 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML

Ingo,

Here's a patch to fix some of the problems when defining ALL_TASKS_PI.
The pi_setprio logic is currently incorrect.  This should fix that. I
converted ALL_TASKS_PI to a constant number, so that it can be used in
if statements.

-- Steve

Note: I compiled this, but I haven't run it yet. I'll run it right after
I send this note and respond how it worked.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux_realtime_goliath/kernel/rt.c
===================================================================
--- linux_realtime_goliath/kernel/rt.c	(revision 314)
+++ linux_realtime_goliath/kernel/rt.c	(working copy)
@@ -104,7 +104,7 @@
  * in the system fall under PI handling. Normally only SCHED_FIFO/RR
  * tasks are PI-handled:
  */
-#define ALL_TASKS_PI
+#define ALL_TASKS_PI 1
 
 #ifdef CONFIG_RT_DEADLOCK_DETECT
 # define __EIP_DECL__ , unsigned long eip
@@ -663,7 +663,7 @@
 
 #endif
 
-#if defined(ALL_TASKS_PI) && defined(CONFIG_RT_DEADLOCK_DETECT)
+#if ALL_TASKS_PI && defined(CONFIG_RT_DEADLOCK_DETECT)
 
 static void
 check_pi_list_present(struct rt_mutex *lock, struct rt_mutex_waiter *waiter,
@@ -674,7 +674,6 @@
 
 	__raw_spin_lock(&old_owner->task->pi_lock);
 	TRACE_WARN_ON_LOCKED(plist_empty(&waiter->pi_list));
-	TRACE_WARN_ON_LOCKED(lock_owner(lock));
 
 	plist_for_each(curr1, &old_owner->task->pi_waiters) {
 		w = plist_entry(curr1, struct rt_mutex_waiter, pi_list);
@@ -851,7 +850,7 @@
 			__raw_spin_lock(&l->wait_lock);
 
 		TRACE_BUG_ON_LOCKED(!lock_owner(l));
-		if (rt_task(p) && plist_empty(&w->pi_list)) {
+		if ((ALL_TASKS_PI || rt_task(p)) && plist_empty(&w->pi_list)) {
 			TRACE_BUG_ON_LOCKED(was_rt);
 			plist_init(&w->pi_list, prio);
 			plist_add(&w->pi_list, &lock_owner(l)->task->pi_waiters);
@@ -868,7 +867,7 @@
 		 * (TODO: this can be unfair to SCHED_NORMAL tasks if they
 		 *        get PI handled.)
 		 */
-		if (!rt_task(p) && !plist_empty(&w->pi_list)) {
+		if (!ALL_TASKS_PI && !rt_task(p) && !plist_empty(&w->pi_list)) {
 			TRACE_BUG_ON_LOCKED(!was_rt);
 			plist_del(&w->pi_list, &lock_owner(l)->task->pi_waiters);
 			plist_del(&w->list, &l->wait_list);
@@ -970,7 +969,7 @@
 	 */
 	TRACE_BUG_ON_LOCKED(!spin_is_locked(&task->pi_lock));
 	TRACE_BUG_ON_LOCKED(!spin_is_locked(&lock->wait_lock));
-#ifndef ALL_TASKS_PI
+#if !ALL_TASKS_PI
 	if (!rt_task(task)) {
 		plist_add(&waiter->list, &lock->wait_list);
 		set_lock_owner_pending(lock);
@@ -1083,7 +1082,7 @@
 #endif
 	trace_special_pid(waiter->ti->task->pid, waiter->ti->task->prio, 0);
 
-#ifdef ALL_TASKS_PI
+#if ALL_TASKS_PI
 	check_pi_list_present(lock, waiter, old_owner);
 #endif
 	new_owner = waiter->ti;
@@ -1543,7 +1542,7 @@
 	list_del_init(&lock->held_list);
 #endif
 
-#ifdef ALL_TASKS_PI
+#if ALL_TASKS_PI
 	if (plist_empty(&lock->wait_list))
 		check_pi_list_empty(lock, lock_owner(lock));
 #endif



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: 2.6.13-rt3
  2005-09-01 16:24 2.6.13-rt3 Steven Rostedt
@ 2005-09-01 16:46 ` Steven Rostedt
  2005-09-01 19:38   ` 2.6.13-rt3 Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2005-09-01 16:46 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML

On Thu, 2005-09-01 at 12:24 -0400, Steven Rostedt wrote:
> 
> Note: I compiled this, but I haven't run it yet. I'll run it right after
> I send this note and respond how it worked.
> 

I'm currently sending this message while running the kernel with this
patch.  But I haven't compiled with RT_DEADLOCK_DETECT. Instead I'm
recompiling with my changes to turn on all TRACE_{WARN,BUG} to be real
WARN and BUG without the deadlock detect code.  This way I can run it
without the trace_lock helping.

-- Steve



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: 2.6.13-rt3
  2005-09-01 16:46 ` 2.6.13-rt3 Steven Rostedt
@ 2005-09-01 19:38   ` Steven Rostedt
  2005-09-01 20:28     ` 2.6.13-rt3 Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2005-09-01 19:38 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: LKML

Ingo,

I just found a __MAJOR__ bug in my code.  Below is the patch that fixes
this bug,  zaps the WARN_ON in check_pi_list_present, and changes
ALL_TASKS_PI to a booleon instead of just a define.

The major bug was in __down_trylock.  See anything wrong with this
code :-)  I'm surprised that this worked as well as it did!


	if (likely(!old_owner) || __grab_lock(lock, task, old_owner->task)) {
		/* granted */
		TRACE_WARN_ON_LOCKED(!plist_empty(&lock->wait_list) && !old_owner);
		if (old_owner) {
			__raw_spin_lock(&old_owner->task->pi_lock);
			set_new_owner(lock, old_owner, ti __EIP__);
			__raw_spin_unlock(&old_owner->task->pi_lock);
		} else
			set_new_owner(lock, old_owner, ti __EIP__);
		__raw_spin_unlock(&lock->wait_lock);
		__raw_spin_unlock(&task->pi_lock);
		ret = 1;
	}
	__raw_spin_unlock(&lock->wait_lock);
	__raw_spin_unlock(&task->pi_lock);


Here's the patch:

-- Steve

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux_realtime_goliath/kernel/rt.c
===================================================================
--- linux_realtime_goliath/kernel/rt.c	(revision 314)
+++ linux_realtime_goliath/kernel/rt.c	(working copy)
@@ -104,7 +104,7 @@
  * in the system fall under PI handling. Normally only SCHED_FIFO/RR
  * tasks are PI-handled:
  */
-#define ALL_TASKS_PI
+#define ALL_TASKS_PI 1
 
 #ifdef CONFIG_RT_DEADLOCK_DETECT
 # define __EIP_DECL__ , unsigned long eip
@@ -663,7 +663,7 @@
 
 #endif
 
-#if defined(ALL_TASKS_PI) && defined(CONFIG_RT_DEADLOCK_DETECT)
+#if ALL_TASKS_PI && defined(CONFIG_RT_DEADLOCK_DETECT)
 
 static void
 check_pi_list_present(struct rt_mutex *lock, struct rt_mutex_waiter *waiter,
@@ -674,7 +674,6 @@
 
 	__raw_spin_lock(&old_owner->task->pi_lock);
 	TRACE_WARN_ON_LOCKED(plist_empty(&waiter->pi_list));
-	TRACE_WARN_ON_LOCKED(lock_owner(lock));
 
 	plist_for_each(curr1, &old_owner->task->pi_waiters) {
 		w = plist_entry(curr1, struct rt_mutex_waiter, pi_list);
@@ -851,7 +850,7 @@
 			__raw_spin_lock(&l->wait_lock);
 
 		TRACE_BUG_ON_LOCKED(!lock_owner(l));
-		if (rt_task(p) && plist_empty(&w->pi_list)) {
+		if ((ALL_TASKS_PI || rt_task(p)) && plist_empty(&w->pi_list)) {
 			TRACE_BUG_ON_LOCKED(was_rt);
 			plist_init(&w->pi_list, prio);
 			plist_add(&w->pi_list, &lock_owner(l)->task->pi_waiters);
@@ -868,7 +867,7 @@
 		 * (TODO: this can be unfair to SCHED_NORMAL tasks if they
 		 *        get PI handled.)
 		 */
-		if (!rt_task(p) && !plist_empty(&w->pi_list)) {
+		if (!ALL_TASKS_PI && !rt_task(p) && !plist_empty(&w->pi_list)) {
 			TRACE_BUG_ON_LOCKED(!was_rt);
 			plist_del(&w->pi_list, &lock_owner(l)->task->pi_waiters);
 			plist_del(&w->list, &l->wait_list);
@@ -970,7 +969,7 @@
 	 */
 	TRACE_BUG_ON_LOCKED(!spin_is_locked(&task->pi_lock));
 	TRACE_BUG_ON_LOCKED(!spin_is_locked(&lock->wait_lock));
-#ifndef ALL_TASKS_PI
+#if !ALL_TASKS_PI
 	if (!rt_task(task)) {
 		plist_add(&waiter->list, &lock->wait_list);
 		set_lock_owner_pending(lock);
@@ -1071,6 +1070,7 @@
 	struct rt_mutex_waiter *waiter = NULL;
 	struct thread_info *new_owner;
 
+	TRACE_BUG_ON_LOCKED(!spin_is_locked(&lock->wait_lock));
 	/*
 	 * Get the highest prio one:
 	 *
@@ -1083,7 +1083,7 @@
 #endif
 	trace_special_pid(waiter->ti->task->pid, waiter->ti->task->prio, 0);
 
-#ifdef ALL_TASKS_PI
+#if ALL_TASKS_PI
 	check_pi_list_present(lock, waiter, old_owner);
 #endif
 	new_owner = waiter->ti;
@@ -1543,7 +1543,7 @@
 	list_del_init(&lock->held_list);
 #endif
 
-#ifdef ALL_TASKS_PI
+#if ALL_TASKS_PI
 	if (plist_empty(&lock->wait_list))
 		check_pi_list_empty(lock, lock_owner(lock));
 #endif
@@ -1906,8 +1906,6 @@
 			__raw_spin_unlock(&old_owner->task->pi_lock);
 		} else
 			set_new_owner(lock, old_owner, ti __EIP__);
-		__raw_spin_unlock(&lock->wait_lock);
-		__raw_spin_unlock(&task->pi_lock);
 		ret = 1;
 	}
 	__raw_spin_unlock(&lock->wait_lock);



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: 2.6.13-rt3
  2005-09-01 19:38   ` 2.6.13-rt3 Steven Rostedt
@ 2005-09-01 20:28     ` Ingo Molnar
  2005-09-08 13:41       ` 2.6.13-rt3 Stephane Couture
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2005-09-01 20:28 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: LKML


* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo,
> 
> I just found a __MAJOR__ bug in my code.  Below is the patch that 
> fixes this bug, zaps the WARN_ON in check_pi_list_present, and changes 
> ALL_TASKS_PI to a booleon instead of just a define.
> 
> The major bug was in __down_trylock.  See anything wrong with this 
> code :-) I'm surprised that this worked as well as it did!

ok, i've released -rt4 with this fix included. The 8-way box boots fine 
now.

	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: 2.6.13-rt3
  2005-09-01 20:28     ` 2.6.13-rt3 Ingo Molnar
@ 2005-09-08 13:41       ` Stephane Couture
  0 siblings, 0 replies; 5+ messages in thread
From: Stephane Couture @ 2005-09-08 13:41 UTC (permalink / raw)
  To: LKML; +Cc: Ingo Molnar

Ingo Molnar wrote:
> * Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> 
>>Ingo,
>>
>>I just found a __MAJOR__ bug in my code.  Below is the patch that 
>>fixes this bug, zaps the WARN_ON in check_pi_list_present, and changes 
>>ALL_TASKS_PI to a booleon instead of just a define.
>>
>>The major bug was in __down_trylock.  See anything wrong with this 
>>code :-) I'm surprised that this worked as well as it did!
> 
> 
> ok, i've released -rt4 with this fix included. The 8-way box boots fine 
> now.
> 
> 	Ingo

On 2.6.13-rt4, I get this error when HIGH_RES_TIMERS is disabled :

arch/ppc/kernel/built-in.o(.text+0x1dbc): In function `timer_interrupt':
arch/ppc/kernel/time.c:241: undefined reference to `do_hr_timer_int'

There is some #ifdef CONFIG_HIGH_RES_TIMERS missing in 
arch/ppc/kernel/time.c

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2005-09-08 13:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-01 16:24 2.6.13-rt3 Steven Rostedt
2005-09-01 16:46 ` 2.6.13-rt3 Steven Rostedt
2005-09-01 19:38   ` 2.6.13-rt3 Steven Rostedt
2005-09-01 20:28     ` 2.6.13-rt3 Ingo Molnar
2005-09-08 13:41       ` 2.6.13-rt3 Stephane Couture

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox