Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Philippe Gerum wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>
>>> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405
>>>
>> always-put-xnthread-base-into-registry.patch:
>> I understand the need, but I will cowardly let Philippe decide whether
>> he likes the implementation details.
>>
>
> I'm ok with the basic purpose of those changes, but if they are aimed at
> allowing direct lookups from the xnsynch code into the registry so that base
> object pointers can be safely assumed to be returned for threads, I would
> define
> specialized xnthread_register() and xnthread_lookup() calls to explicitly
> state
> that we do want to index struct xnthread pointers, and not any random type as
> the registry would otherwise permit.
>
> Also, for readability purpose, please factor the outer lookup code as much as
> possible.
>
> e.g.
> static inline RT_TASK *lookup_task(xnhandle_t handle)
> {
> return thread2rtask(xnthread_lookup(handle));
> }
Makes sense, will refactor the patch accordingly.
>
>> handle-base-xn_sys_current-1.patch:
>> In some places (pse51_mutex_timedlock_inner for instances) you use
>> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use
>> NULL, are the two equivalents ? If yes, should not we always use the
>> same consistently ? Otherwise looks ok.
>>
>> remove-xnarch_atomic_intptr.patch:
>> Ok.
>>
>> spread-xeno_set_current.patch:
>> Ok. This is even a bug fix.
>>
>> xnsynch refactoring:
>> things have moved too much to see what has really changed in
>> xnsynch_wakeup_one_sleeper and xnsynch_sleep_on. But is not there a
>> common behaviour between the old and new services that could be factored
>> ? But otherwise I agree with the general idea of the patch, this is what
>> we had discussed with Philippe.
>>
The rest, specifically the approach of the last patch, is OK for you as
well?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: >>> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405 >>> always-put-xnthread-base-into-registry.patch: >>> I understand the need, but I will cowardly let Philippe decide >>> whether >>> he likes the implementation details. >>> >>> handle-base-xn_sys_current-1.patch: >>> In some places (pse51_mutex_timedlock_inner for instances) you >>> use >>> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use >>> NULL, are the two equivalents ? If yes, should not we always use the >>> same consistently ? Otherwise looks ok. >> I fail to find the NULL spots - which pse51_mutex_timedlock do you mean? > A few excerpts: Ah, you mean checking against non-zero - that can be changed of course. Updated patch below, hope I caught them all. >>> Ok. Looks good to me. Minus the bug in mutex_save_count, but this can be >>> changed later. >> ??? Which bug? > > The fact that mutex_save_count uses xnsynch_nsleepers instead of > mutex->sleepers. Ah, ok. Forgot about this as some of my rejected patches changed that part anyway. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >> >>> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405 >>> >> always-put-xnthread-base-into-registry.patch: >> I understand the need, but I will cowardly let Philippe decide whether >> he likes the implementation details. >> >> handle-base-xn_sys_current-1.patch: >> In some places (pse51_mutex_timedlock_inner for instances) you use >> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use >> NULL, are the two equivalents ? If yes, should not we always use the >> same consistently ? Otherwise looks ok. > I fail to find the NULL spots - which pse51_mutex_timedlock do you mean? A few excerpts: >>> Ah, you mean checking against non-zero - that can be changed of course. >>> Updated patch below, hope I caught them all. >> Ok. Looks good to me. Minus the bug in mutex_save_count, but this can be >> changed later. > > ??? Which bug? The fact that mutex_save_count uses xnsynch_nsleepers instead of mutex->sleepers. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Gilles Chanteperdrix wrote: > Jan Kiszka wrote: > >> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405 >> > always-put-xnthread-base-into-registry.patch: > I understand the need, but I will cowardly let Philippe decide whether > he likes the implementation details. > > handle-base-xn_sys_current-1.patch: > In some places (pse51_mutex_timedlock_inner for instances) you use > XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use > NULL, are the two equivalents ? If yes, should not we always use the > same consistently ? Otherwise looks ok. I fail to find the NULL spots - which pse51_mutex_timedlock do you mean? >>> A few excerpts: >> Ah, you mean checking against non-zero - that can be changed of course. >> Updated patch below, hope I caught them all. > > Ok. Looks good to me. Minus the bug in mutex_save_count, but this can be > changed later. ??? Which bug? Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405 > always-put-xnthread-base-into-registry.patch: I understand the need, but I will cowardly let Philippe decide whether he likes the implementation details. handle-base-xn_sys_current-1.patch: In some places (pse51_mutex_timedlock_inner for instances) you use XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use NULL, are the two equivalents ? If yes, should not we always use the same consistently ? Otherwise looks ok. >>> I fail to find the NULL spots - which pse51_mutex_timedlock do you mean? >> A few excerpts: > > Ah, you mean checking against non-zero - that can be changed of course. > Updated patch below, hope I caught them all. Ok. Looks good to me. Minus the bug in mutex_save_count, but this can be changed later. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>
>> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405
>>
> always-put-xnthread-base-into-registry.patch:
> I understand the need, but I will cowardly let Philippe decide whether
> he likes the implementation details.
>
I'm ok with the basic purpose of those changes, but if they are aimed at
allowing direct lookups from the xnsynch code into the registry so that base
object pointers can be safely assumed to be returned for threads, I would define
specialized xnthread_register() and xnthread_lookup() calls to explicitly state
that we do want to index struct xnthread pointers, and not any random type as
the registry would otherwise permit.
Also, for readability purpose, please factor the outer lookup code as much as
possible.
e.g.
static inline RT_TASK *lookup_task(xnhandle_t handle)
{
return thread2rtask(xnthread_lookup(handle));
}
> handle-base-xn_sys_current-1.patch:
> In some places (pse51_mutex_timedlock_inner for instances) you use
> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use
> NULL, are the two equivalents ? If yes, should not we always use the
> same consistently ? Otherwise looks ok.
>
> remove-xnarch_atomic_intptr.patch:
> Ok.
>
> spread-xeno_set_current.patch:
> Ok. This is even a bug fix.
>
> xnsynch refactoring:
> things have moved too much to see what has really changed in
> xnsynch_wakeup_one_sleeper and xnsynch_sleep_on. But is not there a
> common behaviour between the old and new services that could be factored
> ? But otherwise I agree with the general idea of the patch, this is what
> we had discussed with Philippe.
>
--
Philippe.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>
[1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405
>>> always-put-xnthread-base-into-registry.patch:
>>> I understand the need, but I will cowardly let Philippe decide whether
>>> he likes the implementation details.
>>>
>>> handle-base-xn_sys_current-1.patch:
>>> In some places (pse51_mutex_timedlock_inner for instances) you use
>>> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use
>>> NULL, are the two equivalents ? If yes, should not we always use the
>>> same consistently ? Otherwise looks ok.
>> I fail to find the NULL spots - which pse51_mutex_timedlock do you mean?
>
> A few excerpts:
>
> @@ -101,9 +103,14 @@ pse51_mutex_trylock_internal(xnthread_t
> return ERR_PTR(-EPERM);
> #endif /* XENO_DEBUG(POSIX) */
>
> - owner = xnarch_atomic_intptr_cmpxchg(mutex->owner, NULL, cur);
> - if (unlikely(owner != NULL))
> + ownerh = xnarch_atomic_cmpxchg(mutex->owner, XN_NO_HANDLE,
> +xnthread_handle(cur));
> + if (unlikely(ownerh)) {
> + owner = xnregistry_fetch(clear_claimed(ownerh));
> + if (!owner)
> + return ERR_PTR(-EINVAL);
> return owner;
> + }
>
> shadow->lockcnt = count;
> return NULL;
>
>
> @@ -128,32 +136,41 @@ static inline int pse51_mutex_timedlock_
> (...)
> - old = xnarch_atomic_intptr_cmpxchg(mutex->owner,
> -owner, set_claimed(owner,
> 1));
> - if (likely(old == owner))
> + old = xnarch_atomic_cmpxchg(mutex->owner, ownerh,
> + set_claimed(ownerh, 1));
> + if (likely(old == ownerh))
> break;
> test_no_owner:
> - if (old == NULL) {
> + if (!old) {
> /* Owner called fast mutex_unlock
> (on another cpu) */
> xnlock_put_irqrestore(&nklock, s);
>
> @@ -163,7 +163,7 @@ int __wrap_pthread_mutex_lock(pthread_mu
> goto out;
> }
>
> - owner = xnarch_atomic_intptr_cmpxchg(get_ownerp(shadow), NULL, cur);
> + owner = xnarch_atomic_cmpxchg(get_ownerp(shadow), XN_NO_HANDLE, cur);
> if (likely(!owner)) {
> shadow->lockcnt = 1;
> cb_read_unlock(&shadow->lock, s);
>
>
> @@ -210,7 +210,7 @@ int __wrap_pthread_mutex_timedlock(pthre
> int err = 0;
>
> #ifdef CONFIG_XENO_FASTSEM
> - xnthread_t *cur, *owner;
> + xnhandle_t cur, owner;
>
> cur = xeno_get_current();
> if (!cur)
>
>
>
> @@ -224,7 +224,7 @@ int __wrap_pthread_mutex_timedlock(pthre
> goto out;
> }
>
> - owner = xnarch_atomic_intptr_cmpxchg(get_ownerp(shadow), NULL, cur);
> + owner = xnarch_atomic_cmpxchg(get_ownerp(shadow), XN_NO_HANDLE, cur);
> if (likely(!owner)) {
> shadow->lockcnt = 1;
> cb_read_unlock(&shadow->lock, s);
>
> @@ -271,7 +271,7 @@ int __wrap_pthread_mutex_trylock(pthread
> int err = 0;
>
> #ifdef CONFIG_XENO_FASTSEM
> - xnthread_t *cur, *owner;
> + xnhandle_t cur, owner;
>
> cur = xeno_get_current();
> if (!cur)
>
> @@ -285,7 +285,7 @@ int __wrap_pthread_mutex_trylock(pthread
> goto out;
> }
>
> - owner = xnarch_atomic_intptr_cmpxchg(get_ownerp(shadow), NULL, cur);
> + owner = xnarch_atomic_cmpxchg(get_ownerp(shadow), XN_NO_HANDLE, cur);
> if (likely(!owner)) {
> shadow->lockcnt = 1;
> cb_read_unlock(&shadow->lock, s);
>
> @@ -325,8 +325,8 @@ int __wrap_pthread_mutex_unlock(pthread_
> int err = 0;
>
> #ifdef CONFIG_XENO_FASTSEM
> - xnarch_atomic_intptr_t *ownerp;
> - xnthread_t *cur;
> + xnarch_atomic_t *ownerp;
> + xnhandle_t cur, owner;
>
> cur = xeno_get_current();
> if (!cur)
>
Ah, you mean checking against non-zero - that can be changed of course.
Updated patch below, hope I caught them all.
Jan
---
include/asm-generic/bits/bind.h|9 -
include/asm-generic/bits/current.h |5 +-
include/nucleus/types.h| 13 +++
ksrc/nucleus/shadow.c | 17 -
ksrc/skins/native/Kconfig |1
ksrc/skins/native/task.c | 14 +++-
ksrc/skins/posix/Kconfig |1
ksrc/skins/posix/cb_lock.h | 15 ++--
ksrc/skins/posix/cond.c| 12 --
ksrc/skins/posix/mutex.c | 21 +++-
ksrc/skins/posix/mutex.h | 64 -
ksrc/skins/posix/syscall.c | 11 +++---
ksrc/skins/posix/thread.c | 16 +
ksrc/skins/psos+/Kconfig |2 -
ksrc/skins/rtai/Kconfig|1
ksrc/sk
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>
>>> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405
>>>
>> always-put-xnthread-base-into-registry.patch:
>> I understand the need, but I will cowardly let Philippe decide whether
>> he likes the implementation details.
>>
>> handle-base-xn_sys_current-1.patch:
>> In some places (pse51_mutex_timedlock_inner for instances) you use
>> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use
>> NULL, are the two equivalents ? If yes, should not we always use the
>> same consistently ? Otherwise looks ok.
>
> I fail to find the NULL spots - which pse51_mutex_timedlock do you mean?
A few excerpts:
@@ -101,9 +103,14 @@ pse51_mutex_trylock_internal(xnthread_t
return ERR_PTR(-EPERM);
#endif /* XENO_DEBUG(POSIX) */
- owner = xnarch_atomic_intptr_cmpxchg(mutex->owner, NULL, cur);
- if (unlikely(owner != NULL))
+ ownerh = xnarch_atomic_cmpxchg(mutex->owner, XN_NO_HANDLE,
+ xnthread_handle(cur));
+ if (unlikely(ownerh)) {
+ owner = xnregistry_fetch(clear_claimed(ownerh));
+ if (!owner)
+ return ERR_PTR(-EINVAL);
return owner;
+ }
shadow->lockcnt = count;
return NULL;
@@ -128,32 +136,41 @@ static inline int pse51_mutex_timedlock_
(...)
- old = xnarch_atomic_intptr_cmpxchg(mutex->owner,
- owner, set_claimed(owner,
1));
- if (likely(old == owner))
+ old = xnarch_atomic_cmpxchg(mutex->owner, ownerh,
+ set_claimed(ownerh, 1));
+ if (likely(old == ownerh))
break;
test_no_owner:
- if (old == NULL) {
+ if (!old) {
/* Owner called fast mutex_unlock
(on another cpu) */
xnlock_put_irqrestore(&nklock, s);
@@ -163,7 +163,7 @@ int __wrap_pthread_mutex_lock(pthread_mu
goto out;
}
- owner = xnarch_atomic_intptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+ owner = xnarch_atomic_cmpxchg(get_ownerp(shadow), XN_NO_HANDLE, cur);
if (likely(!owner)) {
shadow->lockcnt = 1;
cb_read_unlock(&shadow->lock, s);
@@ -210,7 +210,7 @@ int __wrap_pthread_mutex_timedlock(pthre
int err = 0;
#ifdef CONFIG_XENO_FASTSEM
- xnthread_t *cur, *owner;
+ xnhandle_t cur, owner;
cur = xeno_get_current();
if (!cur)
@@ -224,7 +224,7 @@ int __wrap_pthread_mutex_timedlock(pthre
goto out;
}
- owner = xnarch_atomic_intptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+ owner = xnarch_atomic_cmpxchg(get_ownerp(shadow), XN_NO_HANDLE, cur);
if (likely(!owner)) {
shadow->lockcnt = 1;
cb_read_unlock(&shadow->lock, s);
@@ -271,7 +271,7 @@ int __wrap_pthread_mutex_trylock(pthread
int err = 0;
#ifdef CONFIG_XENO_FASTSEM
- xnthread_t *cur, *owner;
+ xnhandle_t cur, owner;
cur = xeno_get_current();
if (!cur)
@@ -285,7 +285,7 @@ int __wrap_pthread_mutex_trylock(pthread
goto out;
}
- owner = xnarch_atomic_intptr_cmpxchg(get_ownerp(shadow), NULL, cur);
+ owner = xnarch_atomic_cmpxchg(get_ownerp(shadow), XN_NO_HANDLE, cur);
if (likely(!owner)) {
shadow->lockcnt = 1;
cb_read_unlock(&shadow->lock, s);
@@ -325,8 +325,8 @@ int __wrap_pthread_mutex_unlock(pthread_
int err = 0;
#ifdef CONFIG_XENO_FASTSEM
- xnarch_atomic_intptr_t *ownerp;
- xnthread_t *cur;
+ xnarch_atomic_t *ownerp;
+ xnhandle_t cur, owner;
cur = xeno_get_current();
if (!cur)
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>
>> [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405
>>
> always-put-xnthread-base-into-registry.patch:
> I understand the need, but I will cowardly let Philippe decide whether
> he likes the implementation details.
>
> handle-base-xn_sys_current-1.patch:
> In some places (pse51_mutex_timedlock_inner for instances) you use
> XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use
> NULL, are the two equivalents ? If yes, should not we always use the
> same consistently ? Otherwise looks ok.
I fail to find the NULL spots - which pse51_mutex_timedlock do you mean?
>
> remove-xnarch_atomic_intptr.patch:
> Ok.
>
> spread-xeno_set_current.patch:
> Ok. This is even a bug fix.
>
> xnsynch refactoring:
> things have moved too much to see what has really changed in
> xnsynch_wakeup_one_sleeper and xnsynch_sleep_on. But is not there a
> common behaviour between the old and new services that could be factored
> ? But otherwise I agree with the general idea of the patch, this is what
> we had discussed with Philippe.
Yes, the diff is unfortunate due to a reordering of the function within
synch.c. Here is a direct diff of both services:
--- a 2008-09-22 10:06:56.0 +0200
+++ b 2008-09-22 10:07:39.0 +0200
@@ -1,112 +1,47 @@
void xnsynch_sleep_on(xnsynch_t *synch, xnticks_t timeout,
xntmode_t timeout_mode)
{
- xnthread_t *thread = xnpod_current_thread(), *owner;
+ xnthread_t *thread = xnpod_current_thread();
spl_t s;
+ XENO_BUGON(NUCLEUS, testbits(synch->status, XNSYNCH_OWNER));
+
xnlock_get_irqsave(&nklock, s);
trace_mark(xn_nucleus_synch_sleepon,
"thread %p thread_name %s synch %p",
thread, xnthread_name(thread), synch);
- if (!testbits(synch->status, XNSYNCH_PRIO)) { /* i.e. FIFO */
+ if (!testbits(synch->status, XNSYNCH_PRIO)) /* i.e. FIFO */
appendpq(&synch->pendq, &thread->plink);
- xnpod_suspend_thread(thread, XNPEND, timeout, timeout_mode,
synch);
- goto unlock_and_exit;
- }
-
- if (!testbits(synch->status, XNSYNCH_PIP)) { /* i.e. no ownership */
- insertpqf(&synch->pendq, &thread->plink, thread->cprio);
- xnpod_suspend_thread(thread, XNPEND, timeout, timeout_mode,
synch);
- goto unlock_and_exit;
- }
-
-redo:
- owner = synch->owner;
-
- if (!owner) {
- synch->owner = thread;
- xnthread_clear_info(thread, XNRMID | XNTIMEO | XNBREAK);
- goto unlock_and_exit;
- }
-
- if (thread->cprio > owner->cprio) {
- if (xnthread_test_info(owner, XNWAKEN) && owner->wwake ==
synch) {
- /* Ownership is still pending, steal the resource. */
- synch->owner = thread;
- xnthread_clear_info(thread, XNRMID | XNTIMEO | XNBREAK);
- xnthread_set_info(owner, XNROBBED);
- goto unlock_and_exit;
- }
-
- if (!xnthread_test_state(owner, XNBOOST)) {
- owner->bprio = owner->cprio;
- xnthread_set_state(owner, XNBOOST);
- }
-
- if (testbits(synch->status, XNSYNCH_CLAIMED))
- removepq(&owner->claimq, &synch->link);
- else
- __setbits(synch->status, XNSYNCH_CLAIMED);
-
- insertpqf(&owner->claimq, &synch->link, thread->cprio);
- insertpqf(&synch->pendq, &thread->plink, thread->cprio);
- xnsynch_renice_thread(owner, thread->cprio);
- } else
+ else /* i.e. priority-sorted */
insertpqf(&synch->pendq, &thread->plink, thread->cprio);
xnpod_suspend_thread(thread, XNPEND, timeout, timeout_mode, synch);
- if (xnthread_test_info(thread, XNRMID | XNTIMEO | XNBREAK))
- goto unlock_and_exit;
-
- if (xnthread_test_info(thread, XNROBBED)) {
- /* Somebody stole us the ownership while we were ready
- to run, waiting for the CPU: we need to wait again
- for the resource. */
- if (timeout_mode != XN_RELATIVE || timeout == XN_INFINITE)
- goto redo;
- timeout = xntimer_get_timeout_stopped(&thread->rtimer);
- if (timeout > 1) /* Otherwise, it's too late. */
- goto redo;
- xnthread_set_info(thread, XNTIMEO);
- }
-
- unlock_and_exit:
-
- thread->wwake = NULL;
- xnthread_clear_info(thread, XNWAKEN);
-
xnlock_put_irqrestore(&nklock, s);
}
xnthread_t *xnsynch_wakeup_one_sleeper(xnsynch_t *synch)
{
- xnthread_t *thread = NULL, *lastowner;
+ xnthread_t *thread = NULL;
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > [1]http://thread.gmane.org/gmane.linux.real-time.xenomai.devel/5412/focus=5405 > always-put-xnthread-base-into-registry.patch: I understand the need, but I will cowardly let Philippe decide whether he likes the implementation details. handle-base-xn_sys_current-1.patch: In some places (pse51_mutex_timedlock_inner for instances) you use XN_NO_HANDLE, in others (pse51_mutex_timedlock for instances) you use NULL, are the two equivalents ? If yes, should not we always use the same consistently ? Otherwise looks ok. remove-xnarch_atomic_intptr.patch: Ok. spread-xeno_set_current.patch: Ok. This is even a bug fix. xnsynch refactoring: things have moved too much to see what has really changed in xnsynch_wakeup_one_sleeper and xnsynch_sleep_on. But is not there a common behaviour between the old and new services that could be factored ? But otherwise I agree with the general idea of the patch, this is what we had discussed with Philippe. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > Jan Kiszka wrote: >> Slowly moving on toward generic fast mutex support for Xenomai, this >> patch is a proposal to address the increasing divergence of >> owner-tracking vs. owner-less xnsynch objects. >> >> The services dealing with the former will likely include a new, lockless >> prologues for the mutex fastpath. At the the same time, this additional >> code should not disturb too much in those cases where we do not track >> ownership (condition variables, events, semaphores etc.). Moreover, I >> noticed that some of the existing code assumes XNSYNCH_NOPIP means no >> ownership, which is surely not true. The already visible effect is that >> lock stealing is needlessly restricted to XNSYNCH_PIP. >> >> Going through the API, I dug out three diverging services and replaced >> them with two new ones: >> >> Owner-less xnsynch objects: >> - xnsynch_sleep_on >> - xnsynch_wakeup_one_sleeper >> - xnsynch_wakeup_this_sleeper >> >> Owner-tracking xnsynch objects: >> - xnsynch_acquire >> - xnsynch_release >> >> The latter type of objects are marked with the new flag XNSYNCH_OWNER, >> used only for debugging and code documentation purposes in the current >> implementation. > > Any comments on this? I plan to resume the work on fast xnsynch once > this building block is clarified (or replaced by an alternative). I have to admit that I have to dig into my e-mails to find back all the patches to which I have not answered, and this is hard. Is it easy for you to repost all of them in a unique thread ? -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > Slowly moving on toward generic fast mutex support for Xenomai, this > patch is a proposal to address the increasing divergence of > owner-tracking vs. owner-less xnsynch objects. > > The services dealing with the former will likely include a new, lockless > prologues for the mutex fastpath. At the the same time, this additional > code should not disturb too much in those cases where we do not track > ownership (condition variables, events, semaphores etc.). Moreover, I > noticed that some of the existing code assumes XNSYNCH_NOPIP means no > ownership, which is surely not true. The already visible effect is that > lock stealing is needlessly restricted to XNSYNCH_PIP. > > Going through the API, I dug out three diverging services and replaced > them with two new ones: > > Owner-less xnsynch objects: > - xnsynch_sleep_on > - xnsynch_wakeup_one_sleeper > - xnsynch_wakeup_this_sleeper > > Owner-tracking xnsynch objects: > - xnsynch_acquire > - xnsynch_release > > The latter type of objects are marked with the new flag XNSYNCH_OWNER, > used only for debugging and code documentation purposes in the current > implementation. Any comments on this? I plan to resume the work on fast xnsynch once this building block is clarified (or replaced by an alternative). Jan signature.asc Description: OpenPGP digital signature ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > Jan Kiszka wrote: >> Slowly moving on toward generic fast mutex support for Xenomai, this >> patch is a proposal to address the increasing divergence of >> owner-tracking vs. owner-less xnsynch objects. >> >> The services dealing with the former will likely include a new, lockless >> prologues for the mutex fastpath. At the the same time, this additional >> code should not disturb too much in those cases where we do not track >> ownership (condition variables, events, semaphores etc.). Moreover, I >> noticed that some of the existing code assumes XNSYNCH_NOPIP means no >> ownership, which is surely not true. The already visible effect is that >> lock stealing is needlessly restricted to XNSYNCH_PIP. >> >> Going through the API, I dug out three diverging services and replaced >> them with two new ones: >> >> Owner-less xnsynch objects: >> - xnsynch_sleep_on >> - xnsynch_wakeup_one_sleeper >> - xnsynch_wakeup_this_sleeper >> >> Owner-tracking xnsynch objects: >> - xnsynch_acquire >> - xnsynch_release >> >> The latter type of objects are marked with the new flag XNSYNCH_OWNER, >> used only for debugging and code documentation purposes in the current >> implementation. >> >> Find a first draft of this approach attached (compile-tested). Before >> going down this round, I would like to collect opinions and finally an >> Ack on this (or and alternative approach). I also briefly thought about >> branching two xnsynch sub-objects for owner/no-owner. But that would >> likely make the changes far more complicated and invasive. > > Forgot to mention that the patch applies on top of the first 4 patches > of my "fast mutex rework" series (conflicts in posix only). > > Jan > > PS: gna.org is dead. Hope it will recover soon... > Server has moved. DNS are slowly catching up. -- Philippe. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release
Jan Kiszka wrote: > Slowly moving on toward generic fast mutex support for Xenomai, this > patch is a proposal to address the increasing divergence of > owner-tracking vs. owner-less xnsynch objects. > > The services dealing with the former will likely include a new, lockless > prologues for the mutex fastpath. At the the same time, this additional > code should not disturb too much in those cases where we do not track > ownership (condition variables, events, semaphores etc.). Moreover, I > noticed that some of the existing code assumes XNSYNCH_NOPIP means no > ownership, which is surely not true. The already visible effect is that > lock stealing is needlessly restricted to XNSYNCH_PIP. > > Going through the API, I dug out three diverging services and replaced > them with two new ones: > > Owner-less xnsynch objects: > - xnsynch_sleep_on > - xnsynch_wakeup_one_sleeper > - xnsynch_wakeup_this_sleeper > > Owner-tracking xnsynch objects: > - xnsynch_acquire > - xnsynch_release > > The latter type of objects are marked with the new flag XNSYNCH_OWNER, > used only for debugging and code documentation purposes in the current > implementation. > > Find a first draft of this approach attached (compile-tested). Before > going down this round, I would like to collect opinions and finally an > Ack on this (or and alternative approach). I also briefly thought about > branching two xnsynch sub-objects for owner/no-owner. But that would > likely make the changes far more complicated and invasive. Forgot to mention that the patch applies on top of the first 4 patches of my "fast mutex rework" series (conflicts in posix only). Jan PS: gna.org is dead. Hope it will recover soon... -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
