[Xenomai-core] [PATCH] Clean up XNWAKEN / wwake tracking

[Xenomai-core] [PATCH] Clean up XNWAKEN / wwake tracking

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3785 bytes --]

Current lock stealing implementation encodes the "new owner woken up,
but not yet scheduled" state redundantly. First, the XNWAKEN thread bit
is set and, second, the wwake field in xnthread_t points to the
xnsynch_t object that the thread is about to own.

Apparently, no technical benefit can be expected from this. It rather
adds some, though minor, overhead to frequently used core services.
Moreover such redundancy can cause confusion for the uninformed reader
who tries to understand the implementation.

This patch encodes the state purely via wwake (XNWAKEN set => wwake !=
NULL). Compared to the first version posted, this one also clears the
state again in case xnsynch_sleep_on is redone after some lock robbery
(compensating the removed XNWAKEN clearance in xnpod_suspend_thread).

Jan

---
 include/nucleus/thread.h |    1 -
 ksrc/nucleus/pod.c       |    2 +-
 ksrc/nucleus/synch.c     |   11 ++++-------
 3 files changed, 5 insertions(+), 9 deletions(-)

Index: b/include/nucleus/thread.h
===================================================================
--- a/include/nucleus/thread.h
+++ b/include/nucleus/thread.h
@@ -112,7 +112,6 @@
 #define XNRMID    0x00000002 /**< Pending on a removed resource */
 #define XNBREAK   0x00000004 /**< Forcibly awaken from a wait state */
 #define XNKICKED  0x00000008 /**< Kicked upon Linux signal (shadow only) */
-#define XNWAKEN   0x00000010 /**< Thread waken up upon resource availability */
 #define XNROBBED  0x00000020 /**< Robbed from resource ownership */
 #define XNATOMIC  0x00000040 /**< In atomic switch from secondary to primary mode */
 #define XNAFFSET  0x00000080 /**< CPU affinity changed from primary mode */
Index: b/ksrc/nucleus/pod.c
===================================================================
--- a/ksrc/nucleus/pod.c
+++ b/ksrc/nucleus/pod.c
@@ -1406,7 +1406,7 @@ void xnpod_suspend_thread(xnthread_t *th
 		}
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 
-		xnthread_clear_info(thread, XNRMID | XNTIMEO | XNBREAK | XNWAKEN | XNROBBED);
+		xnthread_clear_info(thread, XNRMID | XNTIMEO | XNBREAK | XNROBBED);
 	}
 
 	/* Don't start the timer for a thread indefinitely delayed by
Index: b/ksrc/nucleus/synch.c
===================================================================
--- a/ksrc/nucleus/synch.c
+++ b/ksrc/nucleus/synch.c
@@ -199,7 +199,7 @@ redo:
 	}
 
 	if (thread->cprio > owner->cprio) {
-		if (xnthread_test_info(owner, XNWAKEN) && owner->wwake == synch) {
+		if (owner->wwake == synch) {
 			/* Ownership is still pending, steal the resource. */
 			synch->owner = thread;
 			xnthread_clear_info(thread, XNRMID | XNTIMEO | XNBREAK);
@@ -225,6 +225,9 @@ redo:
 
 	xnpod_suspend_thread(thread, XNPEND, timeout, timeout_mode, synch);
 
+	/* We are awake, no one must steal our lock anymore. */
+	thread->wwake = NULL;
+
 	if (xnthread_test_info(thread, XNRMID | XNTIMEO | XNBREAK))
 		goto unlock_and_exit;
 
@@ -241,10 +244,6 @@ redo:
 	}
 
       unlock_and_exit:
-
-	thread->wwake = NULL;
-	xnthread_clear_info(thread, XNWAKEN);
-
 	xnlock_put_irqrestore(&nklock, s);
 }
 
@@ -389,7 +388,6 @@ xnthread_t *xnsynch_wakeup_one_sleeper(x
 		thread->wchan = NULL;
 		thread->wwake = synch;
 		synch->owner = thread;
-		xnthread_set_info(thread, XNWAKEN);
 		trace_mark(xn_nucleus_synch_wakeup_one,
 			   "thread %p thread_name %s synch %p",
 			   thread, xnthread_name(thread), synch);
@@ -503,7 +501,6 @@ xnpholder_t *xnsynch_wakeup_this_sleeper
 	thread->wchan = NULL;
 	thread->wwake = synch;
 	synch->owner = thread;
-	xnthread_set_info(thread, XNWAKEN);
 	trace_mark(xn_nucleus_synch_wakeup_all,
 		   "thread %p thread_name %s synch %p",
 		   thread, xnthread_name(thread), synch);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Xenomai-core] [PATCH] Clean up XNWAKEN / wwake tracking

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> +	/* We are awake, no one must steal our lock anymore. */
> +	thread->wwake = NULL;
> +

This is wrong, whether or not no one must steal our lock anymore will be
decided at the "redo" label, when we "test and set", the synch owner.

-- 
                                                 Gilles.

Re: [Xenomai-core] [PATCH] Clean up XNWAKEN / wwake tracking

[Gilles Chanteperdrix]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> +	/* We are awake, no one must steal our lock anymore. */
>> +	thread->wwake = NULL;
>> +
> 
> This is wrong, whether or not no one must steal our lock anymore will be
> decided at the "redo" label, when we "test and set", the synch owner.
> 

Ok. This was not the latest patch, sorry for the noise.

-- 
                                                 Gilles.

Re: [Xenomai-core] [PATCH] Clean up XNWAKEN / wwake tracking

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> +	/* We are awake, no one must steal our lock anymore. */
>> +	thread->wwake = NULL;
>> +
> 
> This is wrong, whether or not no one must steal our lock anymore will be
> decided at the "redo" label, when we "test and set", the synch owner.
> 

Maybe I should have added "(in case we actually got it)". If not, that
assignment is a nop. If we redo the whole thing, we would have cleared
XNWAKEN in xnpod_suspend_thread before suspension, so the effect should
be the same.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux