Re: [Xenomai-core] [RFC][PATCH] Factor out xnsynch_acquire/release

[Jan Kiszka <jan.kiszka@domain.hid>]

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