Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners - v2

2008-09-05 Thread Gilles Chanteperdrix
Jan Kiszka wrote:
 @@ -230,18 +230,20 @@ static inline int mutex_save_count(xnthr
  
   mutex = shadow-mutex;
  
 - if (clear_claimed(xnarch_atomic_intptr_get(mutex-owner)) != cur)
 + if (clear_claimed(xnarch_atomic_get(mutex-owner)) !=
 + xnthread_handle(cur))
   return EPERM;
  
   *count_ptr = shadow-lockcnt;
  
 - if (likely(xnarch_atomic_intptr_cmpxchg(mutex-owner, cur, NULL) == 
 cur))
 + if (likely(xnarch_atomic_cmpxchg(mutex-owner, cur, XN_NO_HANDLE) ==
 +xnthread_handle(cur)))
   return 0;
  
   owner = xnsynch_wakeup_one_sleeper(mutex-synchbase);
 - xnarch_atomic_intptr_set
 - (mutex-owner,
 -  set_claimed(owner,xnsynch_nsleepers(mutex-synchbase)));
 + xnarch_atomic_set(mutex-owner,
 +   set_claimed(xnthread_handle(owner),
 +   xnsynch_nsleepers(mutex-synchbase)));

Ok. Can we avoid xnthread_handle() everywhere by storing its result in a
local variable and reusing this local variable, here and in the other
functions where xnthread_handle is used multiple times ?

-- 
 Gilles.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners - v2

2008-09-05 Thread Jan Kiszka
Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 @@ -230,18 +230,20 @@ static inline int mutex_save_count(xnthr
  
  mutex = shadow-mutex;
  
 -if (clear_claimed(xnarch_atomic_intptr_get(mutex-owner)) != cur)
 +if (clear_claimed(xnarch_atomic_get(mutex-owner)) !=
 +xnthread_handle(cur))
  return EPERM;
  
  *count_ptr = shadow-lockcnt;
  
 -if (likely(xnarch_atomic_intptr_cmpxchg(mutex-owner, cur, NULL) == 
 cur))
 +if (likely(xnarch_atomic_cmpxchg(mutex-owner, cur, XN_NO_HANDLE) ==
 +   xnthread_handle(cur)))
  return 0;
  
  owner = xnsynch_wakeup_one_sleeper(mutex-synchbase);
 -xnarch_atomic_intptr_set
 -(mutex-owner,
 - set_claimed(owner,xnsynch_nsleepers(mutex-synchbase)));
 +xnarch_atomic_set(mutex-owner,
 +  set_claimed(xnthread_handle(owner),
 +  xnsynch_nsleepers(mutex-synchbase)));
 
 Ok. Can we avoid xnthread_handle() everywhere by storing its result in a
 local variable and reusing this local variable, here and in the other
 functions where xnthread_handle is used multiple times ?

True for 'cur' here (will fix), but the other case have already been
optimized as far as possible (i.e. within the respective scope).

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] [PATCH 2/9] Switch to handle-based fast mutex owners - v2

2008-09-05 Thread Gilles Chanteperdrix
Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 @@ -230,18 +230,20 @@ static inline int mutex_save_count(xnthr
  
 mutex = shadow-mutex;
  
 -   if (clear_claimed(xnarch_atomic_intptr_get(mutex-owner)) != cur)
 +   if (clear_claimed(xnarch_atomic_get(mutex-owner)) !=
 +   xnthread_handle(cur))
 return EPERM;
  
 *count_ptr = shadow-lockcnt;
  
 -   if (likely(xnarch_atomic_intptr_cmpxchg(mutex-owner, cur, NULL) == 
 cur))
 +   if (likely(xnarch_atomic_cmpxchg(mutex-owner, cur, XN_NO_HANDLE) ==
 +  xnthread_handle(cur)))
 return 0;
  
 owner = xnsynch_wakeup_one_sleeper(mutex-synchbase);
 -   xnarch_atomic_intptr_set
 -   (mutex-owner,
 -set_claimed(owner,xnsynch_nsleepers(mutex-synchbase)));
 +   xnarch_atomic_set(mutex-owner,
 + set_claimed(xnthread_handle(owner),
 + xnsynch_nsleepers(mutex-synchbase)));
 Ok. Can we avoid xnthread_handle() everywhere by storing its result in a
 local variable and reusing this local variable, here and in the other
 functions where xnthread_handle is used multiple times ?
 
 True for 'cur' here (will fix), but the other case have already been
 optimized as far as possible (i.e. within the respective scope).

I have found several other spots where xnthread_handle is called
multiple times in the same function. mutex_unlock comes to my mind.

-- 
 Gilles.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners - v2

2008-09-05 Thread Jan Kiszka
Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 @@ -230,18 +230,20 @@ static inline int mutex_save_count(xnthr
  
mutex = shadow-mutex;
  
 -  if (clear_claimed(xnarch_atomic_intptr_get(mutex-owner)) != cur)
 +  if (clear_claimed(xnarch_atomic_get(mutex-owner)) !=
 +  xnthread_handle(cur))
return EPERM;
  
*count_ptr = shadow-lockcnt;
  
 -  if (likely(xnarch_atomic_intptr_cmpxchg(mutex-owner, cur, NULL) == 
 cur))
 +  if (likely(xnarch_atomic_cmpxchg(mutex-owner, cur, XN_NO_HANDLE) ==
 + xnthread_handle(cur)))
return 0;
  
owner = xnsynch_wakeup_one_sleeper(mutex-synchbase);
 -  xnarch_atomic_intptr_set
 -  (mutex-owner,
 -   set_claimed(owner,xnsynch_nsleepers(mutex-synchbase)));
 +  xnarch_atomic_set(mutex-owner,
 +set_claimed(xnthread_handle(owner),
 +xnsynch_nsleepers(mutex-synchbase)));
 Ok. Can we avoid xnthread_handle() everywhere by storing its result in a
 local variable and reusing this local variable, here and in the other
 functions where xnthread_handle is used multiple times ?
 True for 'cur' here (will fix), but the other case have already been
 optimized as far as possible (i.e. within the respective scope).
 
 I have found several other spots where xnthread_handle is called
 multiple times in the same function. mutex_unlock comes to my mind.

Please point me to the concrete spot. I went again through all
xnthread_handle instances in this patch, checking that they aren't
needed due to owner changes, but only found the one above. Specifically
pthread_mutex_unlock, pse51_mutex_unlock_internal and
__pthread_mutex_unlock have only a single reference or reference
different threads.

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] [PATCH 2/9] Switch to handle-based fast mutex owners - v2

2008-09-05 Thread Gilles Chanteperdrix
Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 @@ -230,18 +230,20 @@ static inline int mutex_save_count(xnthr
  
   mutex = shadow-mutex;
  
 - if (clear_claimed(xnarch_atomic_intptr_get(mutex-owner)) != cur)
 + if (clear_claimed(xnarch_atomic_get(mutex-owner)) !=
 + xnthread_handle(cur))
   return EPERM;
  
   *count_ptr = shadow-lockcnt;
  
 - if (likely(xnarch_atomic_intptr_cmpxchg(mutex-owner, cur, NULL) == 
 cur))
 + if (likely(xnarch_atomic_cmpxchg(mutex-owner, cur, XN_NO_HANDLE) ==
 +xnthread_handle(cur)))
   return 0;
  
   owner = xnsynch_wakeup_one_sleeper(mutex-synchbase);
 - xnarch_atomic_intptr_set
 - (mutex-owner,
 -  set_claimed(owner,xnsynch_nsleepers(mutex-synchbase)));
 + xnarch_atomic_set(mutex-owner,
 +   set_claimed(xnthread_handle(owner),
 +   xnsynch_nsleepers(mutex-synchbase)));
 Ok. Can we avoid xnthread_handle() everywhere by storing its result in a
 local variable and reusing this local variable, here and in the other
 functions where xnthread_handle is used multiple times ?
 True for 'cur' here (will fix), but the other case have already been
 optimized as far as possible (i.e. within the respective scope).
 I have found several other spots where xnthread_handle is called
 multiple times in the same function. mutex_unlock comes to my mind.
 
 Please point me to the concrete spot. I went again through all
 xnthread_handle instances in this patch, checking that they aren't
 needed due to owner changes, but only found the one above. Specifically
 pthread_mutex_unlock, pse51_mutex_unlock_internal and
 __pthread_mutex_unlock have only a single reference or reference
 different threads.

Ok. I had grepped xnthread_handle and found that it was used twice in
mutex_unlock, but did not checked if it was used for the same thread.


-- 
 Gilles.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners

2008-09-02 Thread Gilles Chanteperdrix
Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 To improve robustness of the fast mutex implementation in POSIX (and
 later on in native), it is better to track the mutex owner by handle
 instead of kernel object pointer. Therefore, this patch changes
 __xn_sys_current (xeno_set_current) so that it returns
 xnthread_handle(current_thread). It furthermore converts the POSIX mutex
 implementation to pick up and store the lock owner as handle in the
 kernel/user-shared mutex. Finally it ensures that all skins (except for
 RTDM which must not mess around with foreign skins anyway) add their
 threads to the registry so that at least anonymous handles are
 available.

 As the value stored in the mutex variable is now an integer, we can
 switch over to xnarch_atomic_t, removing all atomic_intptr users.
 The current implementation allows RTDM threads to use POSIX skin
 mutexes. I do not see why this should change.
 Such mixup might technically be possible now. But there is neither a
 need nor does it make the resulting driver more portable. I don't want
 to introduce needless thread registration to RTDM just to cover
 theoretical use cases that should not exist in the first place.

 Nevertheless, some sanity check will have to be added to the
 to-be-written generic xnsynch support to catch missing thread handles.
 That make sense for checking future skin implementation as well.
 That is overhead in the hot path, whereas adding the thread registration
 takes place in a non hot path.
 
 It will be a debug check.

Ok. But the other points remain.

-- 
 Gilles.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners

2008-09-01 Thread Gilles Chanteperdrix
Jan Kiszka wrote:
 To improve robustness of the fast mutex implementation in POSIX (and
 later on in native), it is better to track the mutex owner by handle
 instead of kernel object pointer. Therefore, this patch changes
 __xn_sys_current (xeno_set_current) so that it returns
 xnthread_handle(current_thread). It furthermore converts the POSIX mutex
 implementation to pick up and store the lock owner as handle in the
 kernel/user-shared mutex. Finally it ensures that all skins (except for
 RTDM which must not mess around with foreign skins anyway) add their
 threads to the registry so that at least anonymous handles are
 available.
 
 As the value stored in the mutex variable is now an integer, we can
 switch over to xnarch_atomic_t, removing all atomic_intptr users.

The current implementation allows RTDM threads to use POSIX skin
mutexes. I do not see why this should change.

-- 
 Gilles.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners

2008-09-01 Thread Jan Kiszka
Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 To improve robustness of the fast mutex implementation in POSIX (and
 later on in native), it is better to track the mutex owner by handle
 instead of kernel object pointer. Therefore, this patch changes
 __xn_sys_current (xeno_set_current) so that it returns
 xnthread_handle(current_thread). It furthermore converts the POSIX mutex
 implementation to pick up and store the lock owner as handle in the
 kernel/user-shared mutex. Finally it ensures that all skins (except for
 RTDM which must not mess around with foreign skins anyway) add their
 threads to the registry so that at least anonymous handles are
 available.

 As the value stored in the mutex variable is now an integer, we can
 switch over to xnarch_atomic_t, removing all atomic_intptr users.
 
 The current implementation allows RTDM threads to use POSIX skin
 mutexes. I do not see why this should change.

Such mixup might technically be possible now. But there is neither a
need nor does it make the resulting driver more portable. I don't want
to introduce needless thread registration to RTDM just to cover
theoretical use cases that should not exist in the first place.

Nevertheless, some sanity check will have to be added to the
to-be-written generic xnsynch support to catch missing thread handles.
That make sense for checking future skin implementation 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] [PATCH 2/9] Switch to handle-based fast mutex owners

2008-09-01 Thread Gilles Chanteperdrix
Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 To improve robustness of the fast mutex implementation in POSIX (and
 later on in native), it is better to track the mutex owner by handle
 instead of kernel object pointer. Therefore, this patch changes
 __xn_sys_current (xeno_set_current) so that it returns
 xnthread_handle(current_thread). It furthermore converts the POSIX mutex
 implementation to pick up and store the lock owner as handle in the
 kernel/user-shared mutex. Finally it ensures that all skins (except for
 RTDM which must not mess around with foreign skins anyway) add their
 threads to the registry so that at least anonymous handles are
 available.

 As the value stored in the mutex variable is now an integer, we can
 switch over to xnarch_atomic_t, removing all atomic_intptr users.
 The current implementation allows RTDM threads to use POSIX skin
 mutexes. I do not see why this should change.
 
 Such mixup might technically be possible now. But there is neither a
 need nor does it make the resulting driver more portable. I don't want
 to introduce needless thread registration to RTDM just to cover
 theoretical use cases that should not exist in the first place.
 
 Nevertheless, some sanity check will have to be added to the
 to-be-written generic xnsynch support to catch missing thread handles.
 That make sense for checking future skin implementation as well.

That is overhead in the hot path, whereas adding the thread registration
takes place in a non hot path.

-- 
 Gilles.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH 2/9] Switch to handle-based fast mutex owners

2008-09-01 Thread Jan Kiszka
Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 To improve robustness of the fast mutex implementation in POSIX (and
 later on in native), it is better to track the mutex owner by handle
 instead of kernel object pointer. Therefore, this patch changes
 __xn_sys_current (xeno_set_current) so that it returns
 xnthread_handle(current_thread). It furthermore converts the POSIX mutex
 implementation to pick up and store the lock owner as handle in the
 kernel/user-shared mutex. Finally it ensures that all skins (except for
 RTDM which must not mess around with foreign skins anyway) add their
 threads to the registry so that at least anonymous handles are
 available.

 As the value stored in the mutex variable is now an integer, we can
 switch over to xnarch_atomic_t, removing all atomic_intptr users.
 The current implementation allows RTDM threads to use POSIX skin
 mutexes. I do not see why this should change.
 Such mixup might technically be possible now. But there is neither a
 need nor does it make the resulting driver more portable. I don't want
 to introduce needless thread registration to RTDM just to cover
 theoretical use cases that should not exist in the first place.

 Nevertheless, some sanity check will have to be added to the
 to-be-written generic xnsynch support to catch missing thread handles.
 That make sense for checking future skin implementation as well.
 
 That is overhead in the hot path, whereas adding the thread registration
 takes place in a non hot path.

It will be a debug check.

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] [PATCH 2/9] Switch to handle-based fast mutex owners

2008-09-01 Thread Jan Kiszka
Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 Gilles Chanteperdrix wrote:
 Jan Kiszka wrote:
 To improve robustness of the fast mutex implementation in POSIX (and
 later on in native), it is better to track the mutex owner by handle
 instead of kernel object pointer. Therefore, this patch changes
 __xn_sys_current (xeno_set_current) so that it returns
 xnthread_handle(current_thread). It furthermore converts the POSIX mutex
 implementation to pick up and store the lock owner as handle in the
 kernel/user-shared mutex. Finally it ensures that all skins (except for
 RTDM which must not mess around with foreign skins anyway) add their
 threads to the registry so that at least anonymous handles are
 available.

 As the value stored in the mutex variable is now an integer, we can
 switch over to xnarch_atomic_t, removing all atomic_intptr users.
 The current implementation allows RTDM threads to use POSIX skin
 mutexes. I do not see why this should change.
 Such mixup might technically be possible now. But there is neither a
 need nor does it make the resulting driver more portable. I don't want
 to introduce needless thread registration to RTDM just to cover
 theoretical use cases that should not exist in the first place.

 Nevertheless, some sanity check will have to be added to the
 to-be-written generic xnsynch support to catch missing thread handles.
 That make sense for checking future skin implementation as well.
 That is overhead in the hot path, whereas adding the thread registration
 takes place in a non hot path.
 
 It will be a debug check.

On the other hand: Once we really have generic fast xnsynch support,
this discussion should become pointless - RTDM will use them for its own
mutexes as well. So let's look at this patch only for the current
situation where there is no need for RTDM threads to have a handle as
they must not mess with foreign skin objects.

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