Gilles Chanteperdrix wrote:
> Wolfgang Mauerer wrote:
>> Gilles Chanteperdrix wrote:
>>> the hostrt stuff did not compile on ARM, at this chance, I had a look 
>>> and found a few things which I did not like:
>>> - the kernel-space and user-space did not use seqcount implementations 
>>> from the same header, and what is more, seqcount is not available on 2.4
>>> kernels. Since we provide an implementation anyway, I changed the code 
>>> so that only this implementation is used both in kernel-space and 
>>> user-space. Prefixing everything with xn to avoid namespace clashes.
>> that makes sense, good point!
>>> - the xnarch_hostrt_data also had two different definitions, one for 
>>> user-space, one for kernel-space, since this definition is supposed to 
>>> come from newer I-pipe in kernel-space, the code did not compile with 
>>> older I-pipe patches. I separated xnarch_hostrt_data, the structure 
>>> passed by the I-pipe to do_hostrt_event, from xnvdso_hostrt_data, the 
>>> structure, part of the vdso used for exchanges between user-space and 
>>> kernel-space. We can now get xnvdso_hostrt_data defined all the time. We
>>> can probably even move xnvdso_hostrt_data definition to vdso.h
>> I assume your main intention is to fix the Xenomai compile when the
>> underlying ipipe does not support hostrt? But when the ipipe layer is
>> too old for hostrt, it does not really make sense to include hostrt
>> support in the vdso -- it may be cleaner to define an empty
>> stub for struct xnarch_hostrt when ipipe does not provide hostrt info.
> No. The point is to avoid the comment: "hell will break loose if this
> struct is not keep in sync with one defined by the I-pipe". Having two
> pieces of code relying on different headers to get the same structure
> defined is really shooting ourself in the foot and asking for more. The
> only case where we can admit that is when one piece of code is assembly.
> And even then, it is dangerous (look at the linux kernel code, the
> developers devised asm-offsets.h to solve exactly this issue).
> Besides, having two structures defined for the two different layers
> allows us to modify one structure without any impact on the other.
> Keeping the structure defined even if the I-pipe does not define it is
> just a side effect which avoids #ifdefery. The vdso is part of a heap
> mapped in user-space, so the granularity is 4 Kbi, we do not really care
> about a structure we do not use. But this is debatable, I agree.
It'd be nicer to eliminate it when there's no support for it, but no
religious issue here.
>>> - avoided cycle_t and u32 in order to avoid user-space namespace 
>>> pollution and need for many typedefs with 2.4 kernels. Using unsigned 
>>> long long directly instead of cycle_t revealed a bug in clocktest.c.
>> While typedefs are generally debatable, the intention here was to
>> create as much symmetry with the Linux kernel implementation as
>> possible since this code is fairly well tested. I cannot
>> point to any particular problem with the non-fixed-width definitions,
>> but with them, we do at least avoid the possibility of introducing
>> any scaled math issues.
> unsigned long long is 64 bits
> unsigned is 32 bits
> unsigned long has the natural processor word size.
> So, in reality, they are fixed width definitions. Portable accross all
> architectures supported by Linux. No typedefs needed.

that's correct for almost all programming models (and all reasonable
ones, and certainly for everything the kernel runs on AFIK), but I still
don't see a benefit in removing analogies to the kernel for no
particular gain. A few lines before, you argued that things should
resemble the kernel approach.

Best regards,

>>> Here is the patch.
>>> diff --git a/include/nucleus/ b/include/nucleus/
>>> index 9a72b24..5fc1c21 100644
>>> diff --git a/include/nucleus/ b/include/nucleus/
>>> index 5e93dc9..a6fa801 100644
>>> diff --git a/include/nucleus/hostrt.h b/include/nucleus/hostrt.h
>>> index 70ffbfe..85b8e88 100644
>>> --- a/include/nucleus/hostrt.h
>>> +++ b/include/nucleus/hostrt.h
>>> @@ -23,32 +23,24 @@
>>>   * 02111-1307, USA.
>>>   */
>>> -#include <nucleus/types.h>
>>> -#ifdef __KERNEL__
>>> -#include <asm-generic/xenomai/system.h>
>>> -#else /* !__KERNEL__ */
>>> +#ifndef __KERNEL__
>>>  #include <time.h>
>>>  #include <sys/types.h>
>>> -#include <nucleus/seqlock_user.h>
>>> -typedef u_int32_t u32;
>>> -typedef u_int64_t cycle_t;
>>> +#else /* __KERNEL__ */
>>> +#include <asm-generic/xenomai/system.h>
>>> +#endif /* __KERNEL__ */
>>> +#include <nucleus/seqlock.h>
>>> -/*
>>> - * xnarch_hostrt_data must be kept in sync with the corresponding ipipe
>>> - * definitions in ipipe_tickdev.h We cannot directly include the file
>>> - * here because the definitions are also required in Xenomai userland.
>>> - */
>> I think this comment may have its value.
> No. For the reason explained above.

Xenomai-core mailing list

Reply via email to