[Xenomai-core] [PATCH] Document -EWOULDBLOCK for rtdm_event_wait
From: Wolfgang Mauerer w...@linux-kernel.net rtdm_event_wait can return -EWOULDBLOCK when a negative (i.e., non-blocking) timeout has been specified, but this is not covered in the documentation. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reported-by: michael.schwertfe...@astrum-it.de --- ksrc/skins/rtdm/drvlib.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/ksrc/skins/rtdm/drvlib.c b/ksrc/skins/rtdm/drvlib.c index 69570d0..8177f7d 100644 --- a/ksrc/skins/rtdm/drvlib.c +++ b/ksrc/skins/rtdm/drvlib.c @@ -928,6 +928,9 @@ EXPORT_SYMBOL(rtdm_event_wait); * - -EPERM @e may be returned if an illegal invocation environment is * detected. * + * - -EWOULDBLOCK is returned if a negative @a timeout (i.e., non-blocking + * operation) has been specified. + * * Environments: * * This service can be called from: -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH] Make marker string for faults less ambiguous
From: Wolfgang Mauerer w...@linux-kernel.net The trace mark string for faults uses an address element to notify about the location of the instruction that caused the fault. address is a bit dubious in this context because it can mean either faulting address or IP address. Replacing address with ip also makes the message more consistent with Linux' page_fault_entry trace point. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ksrc/nucleus/pod.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c index 21763d5..37d35f5 100644 --- a/ksrc/nucleus/pod.c +++ b/ksrc/nucleus/pod.c @@ -2511,7 +2511,7 @@ int xnpod_trap_fault(xnarch_fltinfo_t *fltinfo) thread = xnpod_current_thread(); trace_mark(xn_nucleus, thread_fault, - thread %p thread_name %s address %lu type %d, + thread %p thread_name %s ip %p type %d, thread, xnthread_name(thread), xnarch_fault_pc(fltinfo), xnarch_fault_trap(fltinfo)); -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH] Fix handling of deleted message queue objects
From: Wolfgang Mauerer wolfgang.maue...@siemens.com When a Xenomai thread tries to access a deleted message queue, it should obtain -ESRCH as error code both from within the kernel and for userland applications. However, userland sometimes receives wrong error codes because failure of the handle lookup operation is not correctly handled, which leads to subsequently different errors. This patch fixes the faulty patterns, and also provides a documentation update. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reported-by: Victor Metsch victor.met...@siemens.com --- ksrc/skins/native/queue.c |2 ++ ksrc/skins/native/syscall.c |6 ++ 2 files changed, 8 insertions(+), 0 deletions(-) diff --git a/ksrc/skins/native/queue.c b/ksrc/skins/native/queue.c index 9878562..c2b05ae 100644 --- a/ksrc/skins/native/queue.c +++ b/ksrc/skins/native/queue.c @@ -721,6 +721,8 @@ int rt_queue_send(RT_QUEUE *q, void *mbuf, size_t size, int mode) * defined for the queue at creation, or if no memory can be obtained * to convey the message data internally. * + * - -ESRCH is returned if a @a q represents a stale userland handle + * * Environments: * * This service can be called from: diff --git a/ksrc/skins/native/syscall.c b/ksrc/skins/native/syscall.c index 0860447..f6a4de5 100644 --- a/ksrc/skins/native/syscall.c +++ b/ksrc/skins/native/syscall.c @@ -2336,6 +2336,9 @@ static int __rt_queue_write(struct task_struct *curr, struct pt_regs *regs) q = (RT_QUEUE *)xnregistry_fetch(ph.opaque); + if (!q) + return -ESRCH; + /* Buffer to write to the queue. */ buf = (void __user *)__xn_reg_arg2(regs); @@ -2453,6 +2456,9 @@ static int __rt_queue_read(struct task_struct *curr, struct pt_regs *regs) q = (RT_QUEUE *)xnregistry_fetch(ph.opaque); + if (!q) + return -ESRCH; + /* Address of message space to write to. */ buf = (void __user *)__xn_reg_arg2(regs); -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] Realtime gettimeofday()
Hi, we'd like to implement a gettimeofday() mechanism that works in realtime context, both from user- and kernelland. Most importantly, the correctins made to the wall time by the NTP protcol on Linux must be transferred into the Xenomai domain. We have a preliminary implementation (with several weaknesses) ready, but before we go into the details of writing a complete solution, it would be great to have the community's opinion on how to proceed best -- there are a number of alternatives, each with specific strenghts and weaknesses. Any comments on the following proposals are very welcome! Essentially, getting the NTP-corrected time requires three ingredients: - A clock source. - A wall time t_{0} as basis, together with the value of the clocksource at that moment. - An NTP-corrected conversion factor between clock ticks and walltime. Since the NTP correction information holds wrt. a specific clock source, we have to use the clock source used by Linux. The Linux kernel provides a vsyscall based solution to the problem: The base data are periodically copied to a userland-accessible page, and a vsyscall reads the information using seqlock protection (i.e., rereading the information when it was changed during the read operation). This way, the data can be locklessly relayed into userland, and perturbation for the kernel is minimal as no lock to write the data is required. However, things get a little more involved when realtime comes into play: Using a simple seqlock that might require rereading the data an arbitrary number of times contrary to the determinism requirements of realtime. So far, we can think of three alternatives: 1.) Re-use the vsyscall data from the kernel with a buffered seqlock replacement. We cannot directly use the data provided by the Linux kernel because Xenomai might have interrupted the update process, which leads to an inconsistent state. Instead, we would use a buffered seqlock that uses two more copies of the data that are accompanied by a flag which indicates if some other Xenomai thread is currently reading or writing to the data: They are both filled with an initial valid copy of the data during Xenomai initialisation. When the data are requested in primary mode, Xenomai first checks if the Linux seqlock is taken: - If the lock is taken, the data provided by Linux may be in an inconsistent state, and the backup copies are used. - If the lock is not taken, check with the aforementioned flag if another Xenomai task is currently updating one of the copies. - (A) If yes, then read the data from the other copy. - (B) If no, perform at most one read try of the Linux data -- another CPU running the Linux kernel might update them in the meantime. If they have been read without perturbation, mark one copy as being currently updated (naturally, we need to flip the flag atomically), update the buffer, and use the data. If the data were modified during the read phase, discard the result and continue as in (A) Note that the flags may also be extended with version information to check which buffer has been updated more recently. While Xenomai might block the update process for a while, we always have at least a consistent copy of the data, and there is a fixed upper bound on the time required to compute the current NTP-corrected wall time. 2.) Use a transactional mechanism as outlined above to relay the data into userland, make sure that the triple (sec,nsec,ticks) is copied atomically to avoid an invalid state, and live with the fact that if the correction factor is not uptodate, we are only in danger of using outdated information, but won't get a completely bogous result (a userland application might start to read a buffer that is concurrently being updated by another CPU) 3.) Use explicit locking between the Linux kernel and Xenomai to update respectively access the data, and partially eliminate the performance advantages of the vsyscall mechanism. This would have the usual drawbacks associated with locks in primary mode. Besides, since a kernel lock is involved, it would not allow for a pure userland solution without kernel intervention. Are there preferences and/or arguments for/against any of these solutions? Or any better ideas? Thanks in advance for your comments! Best, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Realtime gettimeofday()
Gilles Chanteperdrix wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: I surely don't want to duplicate ntp code, just the results it spits out (e.g. into the vdso page). Ok, good point, we can avoid duplicating ntp code, but the vdso page trick only works on two architectures. So, IMO, the nucleus should get the infos, and copies them to the shared area. The question is to know if the infos are portable/in usable units such as nanoseconds or clock source ticks. Ok. Had a look, the info is pretty much generic. We can use it on all architectures. The point which would generate the I-pipe event is vsyscall_update. completely agreed. But this required arch-specific changes to the I-pipe patch and cannot be done in a generic fashion. The kernel need to synchronize with user land, so the problem is (almost) the same as with obtaining the original data from Linux directly: user land can only use a retry or a smart fall-back mechanism to obtain consistent data. Well, ok, I imagine something like a revision counter. But I do not see how it would work on an SMP system (kernel writing on one CPU, user-space reading on another cpu). Ok, I had a look at seqlock.h, we can do it the same way, in an area of the global heap. We will have to provide our own implementation since we can not include linux/seqlock.h in user-space. update_vsyscall on x86_64 turns off the irqs, to protect against the same code or some reader being called in an interrupt handler. We will have to do the same. But then the code can loop potentially unbounded, can't it? It may be that I'm missing something, but this was the initial reasaon for the three-buffer idea I've presented initially. Best, Wolfgang For the HPET clocksource, well, I do not share your enthusiasm, Linux keeps complaining about the stability of the tsc of my laptop, though it is a fairly recent core2 with the constant_tsc flag. My point is that we offer Xenomai without a real alternative to tsc for many moons, and no one really stood up so far and complained about missing hpet support. Just like you cannot use every SMI-infected box for RT, you can't do so when the tsc is unfixably broken. I'm not sure how hard hpet addition and maintenance would actually be, if it's trivial, I'm fine. But I think we have more important issues to solve. If we do not support HPET, we should have a way to know that Linux is currently not using tsc as its clock source, and so that the nucleus should ignore the corrections, otherwise we may end up with a system drifting even more than if Linux was not running ntp. From the ABI point of view, we can add a member in the sysinfo structure, giving the address where the kernel updates the clock related data, if that member is NULL, the kernel does not update the clock related data (it can be a compile-time decision because using a too old I-pipe patch, or a run-time decision because Linux does not use the tsc as its clocksource), and clock_gettime(CLOCK_REALTIME) uses the syscall. We would then postpone the implementation of the non-NULL case (both in user-space and kernel-space) to after the initial 2.5 release. Mixing kernel and user from either version would not cause any issue. This still stands. But we have to handle the disabled tsc differently, probably with a flag in the shared area. Because this can happen after the sys info have been retrieved. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Realtime gettimeofday()
Hi, On 02.12.2009, at 23:20, Gilles Chanteperdrix gilles.chanteperd...@xenomai.org wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: I surely don't want to duplicate ntp code, just the results it spits out (e.g. into the vdso page). Ok, good point, we can avoid duplicating ntp code, but the vdso page trick only works on two architectures. So, IMO, the nucleus should get the infos, and copies them to the shared area. The question is to know if the infos are portable/in usable units such as nanoseconds or clock source ticks. Ok. Had a look, the info is pretty much generic. We can use it on all architectures. The point which would generate the I-pipe event is vsyscall_update. completely agreed. But this required arch-specific changes to the I-pipe patch and cannot be done in a generic fashion. Well, it can be done in many ways. One of them is to create a function static inline ipipe_vsyscall_update(whatever) { vsyscall_update(whatever); ipipe_dispatch_event(whatever); } in linux/clocksource.h and replace calls to vsyscall_update in generic code with a call to ipipe_vsyscall_update. So, no, this does not necessarily requires arch-specific changes. However, changing it in the arch-specific way is probably the easier to maintain on the long run (if we use the method just sketched, we will have to check with every release if a new call to vsyscall_update appears). Besides, for the architectures which do not implement vsyscall_update (the majority, xenomai-wise, only x86 and ppc implement vsyscall_update), the call to ipipe_dispatch_event may be put in the empty implementation which may be found in linux/clocksource.h. The kernel need to synchronize with user land, so the problem is (almost) the same as with obtaining the original data from Linux directly: user land can only use a retry or a smart fall-back mechanism to obtain consistent data. Well, ok, I imagine something like a revision counter. But I do not see how it would work on an SMP system (kernel writing on one CPU, user-space reading on another cpu). Ok, I had a look at seqlock.h, we can do it the same way, in an area of the global heap. We will have to provide our own implementation since we can not include linux/seqlock.h in user-space. update_vsyscall on x86_64 turns off the irqs, to protect against the same code or some reader being called in an interrupt handler. We will have to do the same. But then the code can loop potentially unbounded, can't it? It may be that I'm missing something, but this was the initial reasaon for the three-buffer idea I've presented initially. Well, on a normally running system, the update takes place for every Linux timer interrupt (or is it only for the periodic tick ?), anyway, if real-time code is running, there is no chance that it can be preempted by a Linux timer interrupt, and if the code is called from non real-time context, we do not care much. For a tickless System, it's only the periodic Tick, Not every Timer Interrupt. Note that we already use such code for tsc emulation on some platforms, and well, it works. And this is normal, the chances to get preempted several times in a row by a timer interrupt without being able to complete the loop are pretty low. Otherwise it would mean that there are plenty of timer interrupts. So that means, in essence, that you would accept probabilistic algorithms in realtime context? Thanks, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Realtime gettimeofday()
Hi, Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: So that means, in essence, that you would accept probabilistic algorithms in realtime context? Ah, today's troll! though it seems that I have to replace Jan this time ;-) As I think I explained, the use of a seqlock in real-time context when the seqlock writer only happens in linux context is not probabilistic. It will work every time the first pass. I still don't see why it should succeed every time: What about the case that the Linux kernel on CPU0 updates the data, while Xenomai accesses them on another CPU? This can lead to inconsistent data, and they must be reread on the Xenomai side. I'm asking because if this case can not happen, then there's nothing left to to as I have the code already at hand. Thanks, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Realtime gettimeofday()
Hi, On 03.12.2009, at 14:14, Gilles Chanteperdrix gilles.chanteperd...@xenomai.org wrote: Wolfgang Mauerer wrote: Hi, Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: So that means, in essence, that you would accept probabilistic algorithms in realtime context? Ah, today's troll! though it seems that I have to replace Jan this time ;-) As I think I explained, the use of a seqlock in real-time context when the seqlock writer only happens in linux context is not probabilistic. It will work every time the first pass. I still don't see why it should succeed every time: What about the case that the Linux kernel on CPU0 updates the data, while Xenomai accesses them on another CPU? This can lead to inconsistent data, and they must be reread on the Xenomai side. Yeah, right. I was not thinking about SMP. But admit that in this case, there will be only one retry, there is nothing pathological. I'm asking because if this case can not happen, then there's nothing left to to as I have the code already at hand. You have reworked the nucleus timers handling to adapt to this new real-time clock ? Nope. Sorry, I was a bit unclear: I'm just referring to the gtod syscall that does the timer handling, Not any other adaptions. Thanks, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Realtime gettimeofday()
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Hi, On 03.12.2009, at 14:14, Gilles Chanteperdrix gilles.chanteperd...@xenomai.org wrote: Wolfgang Mauerer wrote: Hi, Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: So that means, in essence, that you would accept probabilistic algorithms in realtime context? Ah, today's troll! though it seems that I have to replace Jan this time ;-) As I think I explained, the use of a seqlock in real-time context when the seqlock writer only happens in linux context is not probabilistic. It will work every time the first pass. I still don't see why it should succeed every time: What about the case that the Linux kernel on CPU0 updates the data, while Xenomai accesses them on another CPU? This can lead to inconsistent data, and they must be reread on the Xenomai side. Yeah, right. I was not thinking about SMP. But admit that in this case, there will be only one retry, there is nothing pathological. that's right. Which makes it bounded again, so it's maybe the best way to go. I'm asking because if this case can not happen, then there's nothing left to to as I have the code already at hand. You have reworked the nucleus timers handling to adapt to this new real-time clock ? Nope. Sorry, I was a bit unclear: I'm just referring to the gtod syscall that does the timer handling, Not any other adaptions. Ok, but what good is the gtod syscall if you can not use it as a time reference for other timing related services? it suffices for our project's current purposes ;-) But it's certainly not the full solution. Before, we should have a decision wrt. the design issues, but I won't be able to continue working on this before mid of next week to look at the changed required for timer handling and come up with code. Cheers, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Realtime gettimeofday()
Quoting Philippe Gerum r...@xenomai.org: On Mon, 2009-12-07 at 14:43 +0100, Gilles Chanteperdrix wrote: Jan Kiszka wrote: Philippe Gerum wrote: On Thu, 2009-12-03 at 22:42 +0100, Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: We define a structure which will be shared between kernel and user, which contains all data exported by kernel to user, as well as flags indicating which member of the structure is available. The definition of the struct for 2.5.0 will be: struct xnshared { unsigned long long features; }; This struct will be allocated in the global sem heap, and features will be null for the time being. Every time we need to share some data between kernel and user (including for the ntp support), we will add data to the structure, and use a features bit to mean that the data are available from kernel. It will work as long as we add data from release to release and never remove it. So, for 2.5.0, the xnshared structure will be allocated, but the features member will be null. We will pass the offset of this structure in the global sem heap in the sysinfo structure. Go for this. Ack. That's also fine for me. Best, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH] Add support for sharing kernel/userland data between Xenomai and Linux
From: Wolfgang Mauerer wolfgang.maue...@siemens.com A new structure (struct xnshared) is introduced. It allows us to share generic data between user and kernel of Linux and Xenomai; a bitmap of feature flags located at the beginning of the structure identifies which data are shared. The structure is allocated in the global semaphore heap, and xnsysinfo.xnshared_offset identifies the offset of the structure within the heap. Currently, no shared features are yet supported - the patch only introduces the necessary ABI changes. When the need arises to share data between said entities, the structure must be accordingly extended, and a new feature bit must be added to the flags. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-generic/syscall.h |1 + include/nucleus/xnshared.h| 33 + ksrc/nucleus/module.c | 23 +++ ksrc/nucleus/shadow.c |4 4 files changed, 61 insertions(+), 0 deletions(-) create mode 100644 include/nucleus/xnshared.h diff --git a/include/asm-generic/syscall.h b/include/asm-generic/syscall.h index 483b99f..7d68580 100644 --- a/include/asm-generic/syscall.h +++ b/include/asm-generic/syscall.h @@ -53,6 +53,7 @@ typedef struct xnsysinfo { unsigned long long cpufreq; /* CPU frequency */ unsigned long tickval; /* Tick duration (ns) */ + unsigned long xnshared_offset; /* Offset of xnshared in the sem heap */ } xnsysinfo_t; #define SIGSHADOW SIGWINCH diff --git a/include/nucleus/xnshared.h b/include/nucleus/xnshared.h new file mode 100644 index 000..4293cda --- /dev/null +++ b/include/nucleus/xnshared.h @@ -0,0 +1,33 @@ +#ifndef XNSHARED_H +#define XNSHARED_H + +/* + * Data shared between Xenomai kernel/userland and the Linux kernel/userland + * on the global semaphore heap. The features element indicates which data are + * shared. Notice that xnshared may only grow, but never shrink. + */ +struct xnshared { + unsigned long long features; + + /* Embed domain specific structures that describe the +* shared data here */ +}; + +/* + * For each shared feature, add a flag below. For now, it is still + * empty. + */ +enum xnshared_features { +/* XNSHARED_FEAT_A = 1, + XNSHARED_FEAT_B, */ + XNSHARED_MAX_FEATURES, +}; + +extern struct xnshared *xnshared; + +static inline int xnshared_test_feature(enum xnshared_features feature) +{ + return xnshared xnshared-features (1 feature); +} + +#endif diff --git a/ksrc/nucleus/module.c b/ksrc/nucleus/module.c index 5404182..3963fbd 100644 --- a/ksrc/nucleus/module.c +++ b/ksrc/nucleus/module.c @@ -34,6 +34,7 @@ #include nucleus/pipe.h #endif /* CONFIG_XENO_OPT_PIPE */ #include nucleus/select.h +#include nucleus/xnshared.h #include asm/xenomai/bits/init.h MODULE_DESCRIPTION(Xenomai nucleus); @@ -51,6 +52,9 @@ u_long xnmod_sysheap_size; int xeno_nucleus_status = -EINVAL; +struct xnshared *xnshared; +EXPORT_SYMBOL_GPL(xnshared); + struct xnsys_ppd __xnsys_global_ppd; EXPORT_SYMBOL_GPL(__xnsys_global_ppd); @@ -82,6 +86,23 @@ void xnmod_alloc_glinks(xnqueue_t *freehq) } EXPORT_SYMBOL_GPL(xnmod_alloc_glinks); +/* + * We re-use the global semaphore heap to provide a multi-purpose shared + * memory area between Xenomai and Linux - for both kernel and userland + */ +void __init xnheap_init_shared(void) +{ + xnshared = (struct xnshared *)xnheap_alloc(__xnsys_global_ppd.sem_heap, + sizeof(struct xnshared)); + + if (!xnshared) + xnpod_fatal(Xenomai: cannot allocate memory for xnshared!\n); + + /* Currently, we don't support any sharing, but later versions will +* add flags here */ + xnshared-features = 0ULL; +} + int __init __xeno_sys_init(void) { int ret; @@ -106,6 +127,8 @@ int __init __xeno_sys_init(void) goto cleanup_arch; xnheap_set_label(__xnsys_global_ppd.sem_heap, global sem heap); + + xnheap_init_shared(); #endif #ifdef __KERNEL__ diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index 0c94a60..0373a8d 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -50,6 +50,7 @@ #include nucleus/trace.h #include nucleus/stat.h #include nucleus/sys_ppd.h +#include nucleus/xnshared.h #include asm/xenomai/features.h #include asm/xenomai/syscall.h #include asm/xenomai/bits/shadow.h @@ -1746,6 +1747,9 @@ static int xnshadow_sys_info(struct pt_regs *regs) info.cpufreq = xnarch_get_cpu_freq(); + info.xnshared_offset = + xnheap_mapped_offset(xnsys_ppd_get(1)-sem_heap, xnshared); + return __xn_safe_copy_to_user((void __user *)infarg, info, sizeof(info)); } -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] Add support for sharing kernel/userland data between Xenomai and Linux
Gilles Chanteperdrix wrote: wolfgang.maue...@siemens.com wrote: From: Wolfgang Mauerer wolfgang.maue...@siemens.com A new structure (struct xnshared) is introduced. It allows us to share generic data between user and kernel of Linux and Xenomai; a bitmap of feature flags located at the beginning of the structure identifies which data are shared. The structure is allocated in the global semaphore heap, and xnsysinfo.xnshared_offset identifies the offset of the structure within the heap. Currently, no shared features are yet supported - the patch only introduces the necessary ABI changes. When the need arises to share data between said entities, the structure must be accordingly extended, and a new feature bit must be added to the flags. Hi, it is nice that you proposed that patch, I wanted to do it, but did not yet. Some comments however. you're welcome ;-) I have some more patches pending that I have not sent yet because they need some polishing, so it might make sense to coordinate things first before you start to work on any of the issues. A patch including the changes you requested is attached. First, I think xnshared/xnshared_offset is a bit of a long name. I was thinking about xnvdso, but is is only slightly shorter. Any other idea, anyone? we can use xnvdso and xnvdso_off to save a few chars, I don't have any real preferences wrt. naming. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-generic/syscall.h |1 + include/nucleus/xnshared.h| 33 + ksrc/nucleus/module.c | 23 +++ ksrc/nucleus/shadow.c |4 4 files changed, 61 insertions(+), 0 deletions(-) create mode 100644 include/nucleus/xnshared.h diff --git a/include/asm-generic/syscall.h b/include/asm-generic/syscall.h index 483b99f..7d68580 100644 --- a/include/asm-generic/syscall.h +++ b/include/asm-generic/syscall.h @@ -53,6 +53,7 @@ typedef struct xnsysinfo { unsigned long long cpufreq; /* CPU frequency */ unsigned long tickval; /* Tick duration (ns) */ +unsigned long xnshared_offset; /* Offset of xnshared in the sem heap */ } xnsysinfo_t; #define SIGSHADOW SIGWINCH diff --git a/include/nucleus/xnshared.h b/include/nucleus/xnshared.h new file mode 100644 index 000..4293cda --- /dev/null +++ b/include/nucleus/xnshared.h @@ -0,0 +1,33 @@ +#ifndef XNSHARED_H +#define XNSHARED_H + +/* + * Data shared between Xenomai kernel/userland and the Linux kernel/userland + * on the global semaphore heap. The features element indicates which data are + * shared. Notice that xnshared may only grow, but never shrink. + */ +struct xnshared { +unsigned long long features; + +/* Embed domain specific structures that describe the + * shared data here */ +}; + +/* + * For each shared feature, add a flag below. For now, it is still + * empty. + */ +enum xnshared_features { +/* XNSHARED_FEAT_A = 1, +XNSHARED_FEAT_B, */ +XNSHARED_MAX_FEATURES, +}; Xenomai style is to use defines bit with the bit shifted directly, so, XNSHARED_FEAT_A would rather be 0x0001, and I am not sure an enum can be 64 bits. oh yeah, enums are always ints, as a superficial look into the ANSI standard tells me. Wasn't thinking about that. Using your direct definition makes sense (although it's a bit unlikely that there will ever be more than 2^32 shared features...) + +extern struct xnshared *xnshared; + +static inline int xnshared_test_feature(enum xnshared_features feature) +{ +return xnshared xnshared-features (1 feature); +} xnshared can not be null (you panic when allocation fails), and please use testbits for this function. okay + +#endif diff --git a/ksrc/nucleus/module.c b/ksrc/nucleus/module.c index 5404182..3963fbd 100644 --- a/ksrc/nucleus/module.c +++ b/ksrc/nucleus/module.c @@ -34,6 +34,7 @@ #include nucleus/pipe.h #endif /* CONFIG_XENO_OPT_PIPE */ #include nucleus/select.h +#include nucleus/xnshared.h #include asm/xenomai/bits/init.h MODULE_DESCRIPTION(Xenomai nucleus); @@ -51,6 +52,9 @@ u_long xnmod_sysheap_size; int xeno_nucleus_status = -EINVAL; +struct xnshared *xnshared; +EXPORT_SYMBOL_GPL(xnshared); + struct xnsys_ppd __xnsys_global_ppd; EXPORT_SYMBOL_GPL(__xnsys_global_ppd); @@ -82,6 +86,23 @@ void xnmod_alloc_glinks(xnqueue_t *freehq) } EXPORT_SYMBOL_GPL(xnmod_alloc_glinks); +/* + * We re-use the global semaphore heap to provide a multi-purpose shared + * memory area between Xenomai and Linux - for both kernel and userland + */ +void __init xnheap_init_shared(void) +{ +xnshared = (struct xnshared *)xnheap_alloc(__xnsys_global_ppd.sem_heap, + sizeof(struct xnshared)); + +if (!xnshared) +xnpod_fatal(Xenomai
Re: [Xenomai-core] [PATCH] Add support for sharing kernel/userland data between Xenomai and Linux
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: +enum xnshared_features { +/*XNSHARED_FEAT_A = 1, + XNSHARED_FEAT_B, */ + XNSHARED_MAX_FEATURES, +}; Xenomai style is to use defines bit with the bit shifted directly, so, XNSHARED_FEAT_A would rather be 0x0001, and I am not sure an enum can be 64 bits. oh yeah, enums are always ints, as a superficial look into the ANSI standard tells me. Wasn't thinking about that. Using your direct definition makes sense (although it's a bit unlikely that there will ever be more than 2^32 shared features...) Err, since features are bits, the limit is 32 features, not 2^32, and if we make the features member an unsigned long long in the first place, it is because we expect to be able to use the 64 bits, no? Argl. Yes. /me realises that christmas holidays are severely required... Ok. There is a real question here. Whether we want to put this code in module.c or shadow.c. I have no definite answer. Arguments for each file: module.c: the shared area will be used for NTP by the clock/timer subsystem, so will be used in kernel-space, even without CONFIG_XENO_OPT_PERVASIVE. shadow.c: global shared heap is uncached on ARM, so, I am not sure we really want the timer/clock system to use the same area as user-space. So, we are probably better off using different copies of the same data in kernel-space and user-space (with a price of a copy every time the data change and CONFIG_XENO_OPT_PERVASIVE is enabled). This would allow us to move that code in shadow.c Is there any compelling advantage for shadow.c? Using a copy operation every time the data change seems a bit contrary to the reason for the new mechanism in the first place, namely to have a light-weight means of sharing data. The way I see it, the NTP data will be used by the nucleus clock and timer subsystem, so several times for each timer tick, so having them on an uncached memory will have a performance hit. The copy, on the other hand, would take place over a non real-time context, only once every update of NTP data, i.e. once every 10ms. So, let us move it to shadow.c. I see your point, but it has the disadvantage that the time spent in the Linux kernel for the copy operations in the vsyscall is increased -- which might affect the Linux domain, since this is an optimised hot path. Naturally, in the absence of any quantitative measurements, this is all just speculation, but if access to the global semaphore heap is unchached only on ARM, we provide an unnecessary optimisation for n-1 out of n architectures supported by Xenomai. Having the time-keeping code in the Xenomai kernel benefit from NTP corrections provided by the Linux side is a bit separate from the ABI change, which would be the same irregardless of whether we use a second copy for kernel-side NTP operations or not. Would it therefore make sense to restrict the data exchange to the global heap semaphore, and add an architecture-specific hook for ARM later on to generate an extra kernel space copy of the NTP data? The Linux vsyscall time keeping information is architecture-specific, so arch specific hooks make sense in this area anyway. We could therefore restrict the second copy to ARM without much effort. Or am I missing something in your considerations? #ifdef __KERNEL__ diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index 0c94a60..0373a8d 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -50,6 +50,7 @@ #include nucleus/trace.h #include nucleus/stat.h #include nucleus/sys_ppd.h +#include nucleus/xnshared.h #include asm/xenomai/features.h #include asm/xenomai/syscall.h #include asm/xenomai/bits/shadow.h @@ -1746,6 +1747,9 @@ static int xnshadow_sys_info(struct pt_regs *regs) info.cpufreq = xnarch_get_cpu_freq(); + info.xnshared_offset = +xnheap_mapped_offset(xnsys_ppd_get(1)-sem_heap, xnshared); + return __xn_safe_copy_to_user((void __user *)infarg, info, sizeof(info)); } And the user-space part? It would be nice to at least define the structure in user-space and read the features member, to check that the inclusion of the nucleus/xnshared header works and that we read 0 and there is no gag on all architectures We can not expect to read 0, since we want this user-space to continue working with later versions of the ABI. Actually, we can not expect features to be of an specific form (I was thinking (1 max_bit) - 1, but it would mean that we can not remove feature bits). I haven't included the userland part on purpose because I wanted to make sure that the ABI change is merged for 2.5 - all the rest can be added later without problems. It will follow once I've polished the code (if having the userland part should be a prerequisite for merging the ABI change, I can try to speed things up a bit; otherwise, I'd submit the userland read side together with the NTP base mechanics
Re: [Xenomai-core] [PATCH] Add support for sharing kernel/userland data between Xenomai and Linux
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: +enum xnshared_features { +/* XNSHARED_FEAT_A = 1, +XNSHARED_FEAT_B, */ +XNSHARED_MAX_FEATURES, +}; Xenomai style is to use defines bit with the bit shifted directly, so, XNSHARED_FEAT_A would rather be 0x0001, and I am not sure an enum can be 64 bits. oh yeah, enums are always ints, as a superficial look into the ANSI standard tells me. Wasn't thinking about that. Using your direct definition makes sense (although it's a bit unlikely that there will ever be more than 2^32 shared features...) Err, since features are bits, the limit is 32 features, not 2^32, and if we make the features member an unsigned long long in the first place, it is because we expect to be able to use the 64 bits, no? Argl. Yes. /me realises that christmas holidays are severely required... Ok. There is a real question here. Whether we want to put this code in module.c or shadow.c. I have no definite answer. Arguments for each file: module.c: the shared area will be used for NTP by the clock/timer subsystem, so will be used in kernel-space, even without CONFIG_XENO_OPT_PERVASIVE. shadow.c: global shared heap is uncached on ARM, so, I am not sure we really want the timer/clock system to use the same area as user-space. So, we are probably better off using different copies of the same data in kernel-space and user-space (with a price of a copy every time the data change and CONFIG_XENO_OPT_PERVASIVE is enabled). This would allow us to move that code in shadow.c Is there any compelling advantage for shadow.c? Using a copy operation every time the data change seems a bit contrary to the reason for the new mechanism in the first place, namely to have a light-weight means of sharing data. The way I see it, the NTP data will be used by the nucleus clock and timer subsystem, so several times for each timer tick, so having them on an uncached memory will have a performance hit. The copy, on the other hand, would take place over a non real-time context, only once every update of NTP data, i.e. once every 10ms. So, let us move it to shadow.c. I see your point, but it has the disadvantage that the time spent in the Linux kernel for the copy operations in the vsyscall is increased -- which might affect the Linux domain, since this is an optimised hot path. Naturally, in the absence of any quantitative measurements, this is all just speculation, but if access to the global semaphore heap is unchached only on ARM, we provide an unnecessary optimisation for n-1 out of n architectures supported by Xenomai. Having the time-keeping code in the Xenomai kernel benefit from NTP corrections provided by the Linux side is a bit separate from the ABI change, which would be the same irregardless of whether we use a second copy for kernel-side NTP operations or not. Exactly, the issue are distinct, the ABI change is a pure user-space issue, so, should only be compiled if CONFIG_XENO_OPT_PERVASIVE is defined. okay, that makes sense. Would it therefore make sense to restrict the data exchange to the global heap semaphore, and add an architecture-specific hook for ARM later on to generate an extra kernel space copy of the NTP data? The Linux vsyscall time keeping information is architecture-specific, so No. We will make it generic. Nothing arch specific is needed. We will not copy the vsyscall time keeping information, we will copy the data passed to update_vsyscall, which are the same on all architectures. Having the possibility to use gettimeofday() without switching to kernel mode is all about speed, and I don't see why re-using the optimised data structures offered by the Linux kernel, together with the already existing optimised read routines for this purpose should not be an option for the Xenomai userland. But again, the difference between the alternatives is hard to judge without quantitative measurements. I'll maybe come back to this. arch specific hooks make sense in this area anyway. We could therefore restrict the second copy to ARM without much effort. Or am I missing something in your considerations? This xnvdso feature is needed for user-space only. Let us move it to shadow.c. okay. Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] Add support for sharing kernel/userland data between Xenomai and Linux
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: (...) Would it therefore make sense to restrict the data exchange to the global heap semaphore, and add an architecture-specific hook for ARM later on to generate an extra kernel space copy of the NTP data? The Linux vsyscall time keeping information is architecture-specific, so No. We will make it generic. Nothing arch specific is needed. We will not copy the vsyscall time keeping information, we will copy the data passed to update_vsyscall, which are the same on all architectures. Having the possibility to use gettimeofday() without switching to kernel mode is all about speed, and I don't see why re-using the optimised data structures offered by the Linux kernel, together with the already existing optimised read routines for this purpose should not be an option for the Xenomai userland. But again, the difference between the alternatives is hard to judge without quantitative measurements. I'll maybe come back to this. I thought we had been through this already and that we agreed. So, let us make things clear, for once, and I will not be able to exchange much further mails this afternoon. well, the agreement was on 3- implement the nucleus callback for the I-pipe event which copies relevant data, and obviously we were taking different things to be the relevant data ;-) We are not talking about speed. We already have speed: clock_gettime(CLOCK_MONOTONIC) just reads the tsc, and converts the result to a struct timespec, without a syscall, without even a division. This is about as fast as it can be, and enough for 99% of real-time applications. What we are talking about, is synchronizing CLOCK_MONOTONIC and CLOCK_REALTIME with NTP corrections, without sacrificing too much performance. Because, yes, we are going to sacrifice performance, the computations using NTP data will be slower than our current multiplication only conversion. This, for a very specific kind of application. Of the 5 architectures on which Xenomai is currently ported, only two implement the update_vsyscall() function. That is a minority. And we are not interested in yet-another x86-only hack. What we want, because we want the same level of support for all architectures, that will be easier to maintain, is a solution as much generic as possible. Minimal #ifdefs, minimal architecture code. After all, this feature is only needed for a few specific applications, so, there is no reason for it to become a maintenance burden. Let us compare what we are talking about. My idea, is to get the I-pipe patches emit an event when update_vsyscall is called, and in the xenomai handler for this event, do the computations of the constants which will be used by the clock and timer operations until next update. This computation will happen with a xeno-seqlock locked irqs off. Actually, I thought there were some computations when starting this mail, but the computations we are talking about are in fact the copy of a handful of 32 bits and 64 bits variables. What you are talking about is, in the xenomai handler for the NTP I-pipe event, call an arch dependent hook which will copy the data from the x86 specific vsyscall_gtod_data structure. Unless I misunderstood you, there is no difference in terms of performance between the two implementations. And even if there was a difference, we are talking about code which is run as part of Linux timer handler, that is, when Xenomai tasks have relinquished the system. This only adds to the overhead of something that is already of some importance. there are indeed no calculations for AMD64, just for PPC. Since some architectures also require arch-specific hooks for gettimeofday() (albeit non of those with vsyscall and supported by Xenomai), I was trying to design a solution that could also cope with future archs that would require such a hook. But when easier maintainability is more important than optimal performance, I agree that the generic solution is by far better. So let's do it this way. Best, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 1/2] Add support for sharing kernel/userland data between Xenomai and Linux
A new structure (struct xnshared) is introduced. It allows us to share generic data between user and kernel of Linux and Xenomai; a bitmap of feature flags located at the beginning of the structure identifies which data are shared. The structure is allocated in the global semaphore heap, and xnsysinfo.xnshared_offset identifies the offset of the structure within the heap. Currently, no shared features are yet supported - the patch only introduces the necessary ABI changes. When the need arises to share data between said entities, the structure must be accordingly extended, and a new feature bit must be added to the flags. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-generic/syscall.h |1 + include/nucleus/Makefile.am |3 +- include/nucleus/xnvdso.h | 61 + ksrc/nucleus/module.c |7 + ksrc/nucleus/shadow.c | 22 +++ 5 files changed, 93 insertions(+), 1 deletions(-) create mode 100644 include/nucleus/xnvdso.h diff --git a/include/asm-generic/syscall.h b/include/asm-generic/syscall.h index 483b99f..8f1ddc6 100644 --- a/include/asm-generic/syscall.h +++ b/include/asm-generic/syscall.h @@ -53,6 +53,7 @@ typedef struct xnsysinfo { unsigned long long cpufreq; /* CPU frequency */ unsigned long tickval; /* Tick duration (ns) */ + unsigned long xnvdso_off; /* Offset of xnvdso in the sem heap */ } xnsysinfo_t; #define SIGSHADOW SIGWINCH diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am index 4be05f8..26d3fa2 100644 --- a/include/nucleus/Makefile.am +++ b/include/nucleus/Makefile.am @@ -34,4 +34,5 @@ includesub_HEADERS = \ trace.h \ types.h \ version.h \ - xenomai.h + xenomai.h \ + xnvdso.h diff --git a/include/nucleus/xnvdso.h b/include/nucleus/xnvdso.h new file mode 100644 index 000..fbff8fa --- /dev/null +++ b/include/nucleus/xnvdso.h @@ -0,0 +1,61 @@ +#ifndef _XENO_NUCLEUS_XNVDSO_H +#define _XENO_NUCLEUS_XNVDSO_H + +/*!\file xnvdso.h + * \brief Definitions for global semaphore heap shared objects + * \author Wolfgang Mauerer + * + * Copyright (C) 2009 Wolfgang Mauerer wolfgang.maue...@siemens.com. + * + * Xenomai is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * Xenomai is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Xenomai; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA + * 02111-1307, USA. + */ + +#include nucleus/types.h + +/* + * Data shared between Xenomai kernel/userland and the Linux kernel/userland + * on the global semaphore heap. The features element indicates which data are + * shared. Notice that struct xnvdso may only grow, but never shrink. + */ +struct xnvdso { + unsigned long long features; + + /* Embed domain specific structures that describe the +* shared data here */ +}; + +/* + * For each shared feature, add a flag below. For now, the set is still + * empty. + */ +/* +#define XNVDSO_FEAT_A 0x0001 +#define XNVDSO_FEAT_B 0x0002 +#define XNVDSO_FEAT_C 0x0004 +#define XNVDSO_FEATURES(XNVDSO_FEAT_A | XNVDSO_FEAT_B | XVDSO_FEAT_C) +*/ + +#define XNVDSO_FEATURES 0x + +extern struct xnvdso *xnvdso; + +static inline int xnvdso_test_feature(unsigned long long feature) +{ + return testbits(xnvdso-features, feature); +} + +extern void xnheap_init_vdso(void); +#endif /* _XENO_NUCLEUS_XNVDSO_H */ diff --git a/ksrc/nucleus/module.c b/ksrc/nucleus/module.c index 5404182..0a17661 100644 --- a/ksrc/nucleus/module.c +++ b/ksrc/nucleus/module.c @@ -35,6 +35,11 @@ #endif /* CONFIG_XENO_OPT_PIPE */ #include nucleus/select.h #include asm/xenomai/bits/init.h +#ifdef CONFIG_XENO_OPT_PERVASIVE +#include nucleus/xnvdso.h +#else +static inline void xnheap_init_vdso(void) { } +#endif /* CONFIG_XENO_OPT_PERVASIVE */ MODULE_DESCRIPTION(Xenomai nucleus); MODULE_AUTHOR(r...@xenomai.org); @@ -106,6 +111,8 @@ int __init __xeno_sys_init(void) goto cleanup_arch; xnheap_set_label(__xnsys_global_ppd.sem_heap, global sem heap); + + xnheap_init_vdso(); #endif #ifdef __KERNEL__ diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c index d0cb416..bff7dc5 100644 --- a/ksrc/nucleus/shadow.c +++ b/ksrc/nucleus/shadow.c @@ -50,6 +50,7 @@ #include nucleus/trace.h #include nucleus/stat.h #include
[Xenomai-core] [PATCH 0/2] xnvdso mechanism and unit test
Hi, the following two mails integrate Gilles' comments into the xnvdso patch and add a unit test. Notice that this submission does not consider efficient access of shared data from kernel space. Since this problem is independent from the xnvdso mechanism, it will be addressed when I submit the NTP time correction sharing code. Thanks, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism
This testcase checks if the value in xnvdso-features matches the value of XNVDSO_FEATURES, that is, if the information is correctly transferred from kernel to userland. Notice that the approach will fail once configurations are supported that know of multiple features and implement only some of them. In this case, the testcase needs to be extended accordingly to check that only the expected features are present. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/testsuite/unit/Makefile.am | 15 +- src/testsuite/unit/runinfo.in |1 + src/testsuite/unit/xnvdso.c| 43 3 files changed, 58 insertions(+), 1 deletions(-) create mode 100644 src/testsuite/unit/xnvdso.c diff --git a/src/testsuite/unit/Makefile.am b/src/testsuite/unit/Makefile.am index 24d077a..c77cc54 100644 --- a/src/testsuite/unit/Makefile.am +++ b/src/testsuite/unit/Makefile.am @@ -2,7 +2,8 @@ testdir = $(exec_prefix)/share/xenomai/testsuite/unit CCLD = $(top_srcdir)/scripts/wrap-link.sh $(CC) -bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native +bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native \ + xnvdso arith_SOURCES = arith.c arith-noinline.c arith-noinline.h @@ -53,6 +54,18 @@ mutex_torture_native_LDADD = \ ../../skins/native/libnative.la \ -lpthread -lm +xnvdso_SOURCES = xnvdso.c + +xnvdso_CPPFLAGS = \ + @XENO_USER_CFLAGS@ \ + -I$(top_srcdir)/include + +xnvdso_LDFLAGS = @XENO_USER_LDFLAGS@ + +xnvdso_LDADD = \ + ../../skins/native/libnative.la \ + -lpthread -lm + install-data-local: $(mkinstalldirs) $(DESTDIR)$(testdir) @sed -e's,@exec_prefix\@,$(exec_prefix),g' $(srcdir)/runinfo.in $(DESTDIR)$(testdir)/.runinfo diff --git a/src/testsuite/unit/runinfo.in b/src/testsuite/unit/runinfo.in index f4cd208..a22afc0 100644 --- a/src/testsuite/unit/runinfo.in +++ b/src/testsuite/unit/runinfo.in @@ -2,3 +2,4 @@ arith:native:!...@exec_prefix@/bin/arith;popall:control_c wakeup-time:native:!...@exec_prefix@/bin/wakeup-time;popall:control_c mutex-torture-posix:posix:!...@exec_prefix@/bin/mutex-torture-posix;popall:control_c mutex-torture-native:native:!...@exec_prefix@/bin/mutex-torture-native;popall:control_c +xnvdso:native:!...@exec_prefix@/bin/xnvdso;popall:control_c diff --git a/src/testsuite/unit/xnvdso.c b/src/testsuite/unit/xnvdso.c new file mode 100644 index 000..6d7d8e1 --- /dev/null +++ b/src/testsuite/unit/xnvdso.c @@ -0,0 +1,43 @@ +/* + * VDSO feature set testcase + * by Wolfgang Mauerer wolfgang.maue...@siemens.com + */ + +#include stdio.h +#include asm/xenomai/syscall.h +#include nucleus/xnvdso.h + +extern unsigned long xeno_sem_heap[2]; + +int main(int argc, char **argv) +{ + int err; + xnsysinfo_t sysinfo; + struct xnvdso *xnvdso; + + if (!xeno_sem_heap[1]) { + fprintf(stderr, Could not determine position of the + global semaphore heap\n); + return 1; + } + + /* The muxid is irrelevant for this test as long as it's valid */ + err = XENOMAI_SYSCALL2(__xn_sys_info, 1, sysinfo); + if (err 0) { + fprintf(stderr, sys_sys_info failed: %d\n, err); + return 1; + } + + printf(Address of the global semaphore heap: 0x%lx\n, + xeno_sem_heap[1]); + printf(Offset of xnvdso: %lu\n, sysinfo.xnvdso_off); + + xnvdso = (struct xnvdso *)(xeno_sem_heap[1] + sysinfo.xnvdso_off); + printf(Contents of the features flag: %llu\n, xnvdso-features); + + if (xnvdso-features == XNVDSO_FEATURES) + return 0; + + fprintf(stderr, error: xnvdso-features != XNVDSO_FEATURES\n); + return 1; +} -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: This testcase checks if the value in xnvdso-features matches the value of XNVDSO_FEATURES, that is, if the information is correctly transferred from kernel to userland. Notice that the approach will fail once configurations are supported that know of multiple features and implement only some of them. In this case, the testcase needs to be extended accordingly to check that only the expected features are present. Please allow to pass the value we want to check on the test command line. here you go. I suppose keeping XNVDSO_FEATURES as default value unless something different is specified is okay? Cheers, Wolfgang commit 0f31dc5c50f7034dddbac67c22e53823f1be72d3 Author: Wolfgang Mauerer wolfgang.maue...@siemens.com Date: Tue Dec 22 23:11:31 2009 +0100 Testcase for the xnvdso mechanism This testcase checks if the value in xnvdso-features matches the feature set specified on the command line. When no explicit feature test set is given, XNVDSO_FEATURES (i.e., the set of all features known to the current Xenomai revision) is used. Notice that the default approach will naturally fail once configurations are supported that know of multiple features and implement only some of them. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com diff --git a/src/testsuite/unit/Makefile.am b/src/testsuite/unit/Makefile.am index 24d077a..c77cc54 100644 --- a/src/testsuite/unit/Makefile.am +++ b/src/testsuite/unit/Makefile.am @@ -2,7 +2,8 @@ testdir = $(exec_prefix)/share/xenomai/testsuite/unit CCLD = $(top_srcdir)/scripts/wrap-link.sh $(CC) -bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native +bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native \ + xnvdso arith_SOURCES = arith.c arith-noinline.c arith-noinline.h @@ -53,6 +54,18 @@ mutex_torture_native_LDADD = \ ../../skins/native/libnative.la \ -lpthread -lm +xnvdso_SOURCES = xnvdso.c + +xnvdso_CPPFLAGS = \ + @XENO_USER_CFLAGS@ \ + -I$(top_srcdir)/include + +xnvdso_LDFLAGS = @XENO_USER_LDFLAGS@ + +xnvdso_LDADD = \ + ../../skins/native/libnative.la \ + -lpthread -lm + install-data-local: $(mkinstalldirs) $(DESTDIR)$(testdir) @sed -e's,@exec_prefix\@,$(exec_prefix),g' $(srcdir)/runinfo.in $(DESTDIR)$(testdir)/.runinfo diff --git a/src/testsuite/unit/runinfo.in b/src/testsuite/unit/runinfo.in index f4cd208..a22afc0 100644 --- a/src/testsuite/unit/runinfo.in +++ b/src/testsuite/unit/runinfo.in @@ -2,3 +2,4 @@ arith:native:!...@exec_prefix@/bin/arith;popall:control_c wakeup-time:native:!...@exec_prefix@/bin/wakeup-time;popall:control_c mutex-torture-posix:posix:!...@exec_prefix@/bin/mutex-torture-posix;popall:control_c mutex-torture-native:native:!...@exec_prefix@/bin/mutex-torture-native;popall:control_c +xnvdso:native:!...@exec_prefix@/bin/xnvdso;popall:control_c diff --git a/src/testsuite/unit/xnvdso.c b/src/testsuite/unit/xnvdso.c new file mode 100644 index 000..69305c2 --- /dev/null +++ b/src/testsuite/unit/xnvdso.c @@ -0,0 +1,52 @@ +/* + * VDSO feature set testcase + * by Wolfgang Mauerer wolfgang.maue...@siemens.com + */ + +#include stdio.h +#include stdlib.h +#include asm/xenomai/syscall.h +#include nucleus/xnvdso.h + +extern unsigned long xeno_sem_heap[2]; + +int main(int argc, char **argv) +{ + int err; + xnsysinfo_t sysinfo; + struct xnvdso *xnvdso; + unsigned long long test_features; + + if (argc != 2) { + printf(No specific feature(s) given, using XNVDSO_FEATURE\n); + test_features = XNVDSO_FEATURES; + } else { + test_features = strtoull(argv[1], NULL, 0); + } + + if (!xeno_sem_heap[1]) { + fprintf(stderr, Could not determine position of the + global semaphore heap\n); + return 1; + } + + /* The muxid is irrelevant for this test as long as it's valid */ + err = XENOMAI_SYSCALL2(__xn_sys_info, 1, sysinfo); + if (err 0) { + fprintf(stderr, sys_sys_info failed: %d\n, err); + return 1; + } + + printf(Address of the global semaphore heap: 0x%lx\n, + xeno_sem_heap[1]); + printf(Offset of xnvdso: %lu\n, sysinfo.xnvdso_off); + + xnvdso = (struct xnvdso *)(xeno_sem_heap[1] + sysinfo.xnvdso_off); + printf(Contents of the features flag: %llu\n, xnvdso-features); + + if (xnvdso-features == test_features) + return 0; + + fprintf(stderr, error: xnvdso-features != %llu\n, test_features); + return 1; +} ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] Fwd: Re: [RFC][PATCH] x86: Fix root domain state restoring on exception return
(sorry if you get this twice, the cc addresses were screwed up by copy and paste last time) Kiszka, Jan wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Actually, we only need to save the state if we enter over the root domain and have to align its state to the hardware interrupt mask. Moreover, the x86-32 regs.eflags fix-up must happen based on the current root domain state to avoid more spurious corruptions. unfortunately, this won't boot on x86-32: During CPU bug checking in check_hlt, the kernel will really go into the halt state and never recover. By modifying __ipipe_handle_exception to use raw_irqs_disabled_flags as argument to __fixup_if instead of raw_irqs_disabled, everything is back to normal again. However, I'm not sure if this is a) the proper solution or b) won't cause problems somewhere else, so a discussion would be highly welcome... Cheers, Wolfgang diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c index 4442d96..5527e3c 100644 --- a/arch/x86/kernel/ipipe.c +++ b/arch/x86/kernel/ipipe.c @@ -702,19 +702,20 @@ static int __ipipe_xlate_signo[] = { int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) { - unsigned long flags; + bool restore_flags = false; + unsigned long flags = 0; /* Pick up the root domain state of the interrupted context. */ local_save_flags(flags); - if (ipipe_root_domain_p) { + if (ipipe_root_domain_p irqs_disabled_hw()) { /* * Replicate hw interrupt state into the virtual mask before * calling the I-pipe event handler over the root domain. Also * required later when calling the Linux exception handler. */ - if (irqs_disabled_hw()) - local_irq_disable(); + local_irq_save(flags); + restore_flags = true; } #ifdef CONFIG_KGDB /* catch exception KGDB is interested in over non-root domains */ @@ -725,7 +726,8 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) #endif /* CONFIG_KGDB */ if (unlikely(ipipe_trap_notify(vector, regs))) { - local_irq_restore_nosync(flags); + if (restore_flags) + local_irq_restore_nosync(flags); return 1; } @@ -734,9 +736,14 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) * handler, restore the original IF from exception entry as the * low-level return code will evaluate it. */ - __fixup_if(raw_irqs_disabled_flags(flags), regs); - - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* + * 32-bit: In case we migrated to root domain inside the event + * handler, align regs.flags with the root domain state as the + * low-level return code will evaluate it. + */ + __fixup_if(raw_irqs_disabled_flags(flags), regs); + } else { /* Detect unhandled faults over non-root domains. */ struct ipipe_domain *ipd = ipipe_current_domain; @@ -770,21 +777,26 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) * Relevant for 64-bit: Restore root domain state as the low-level * return code will not align it to regs.flags. */ - local_irq_restore_nosync(flags); + if (restore_flags) + local_irq_restore_nosync(flags); return 0; } int __ipipe_divert_exception(struct pt_regs *regs, int vector) { - unsigned long flags; - - /* Same root state handling as in __ipipe_handle_exception. */ - local_save_flags(flags); + bool restore_flags = false; + unsigned long flags = 0; if (ipipe_root_domain_p) { - if (irqs_disabled_hw()) - local_irq_disable(); + if (irqs_disabled_hw()) { + /* + * Same root state handling as in + * __ipipe_handle_exception. + */ + local_irq_save(flags); + restore_flags = true; + } } #ifdef CONFIG_KGDB /* catch int1 and int3 over non-root domains */ @@ -804,16 +816,20 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector) #endif /* CONFIG_KGDB */ if (unlikely(ipipe_trap_notify(vector, regs))) { - local_irq_restore_nosync(flags); + if (restore_flags) + local_irq_restore_nosync(flags); return 1; } + if (likely(ipipe_root_domain_p)) { + /* see __ipipe_handle_exception */ + __fixup_if(raw_irqs_disabled(), regs); + } + /* - * 32-bit: Due to possible migration inside the event handler, we have - * to restore IF so that low-level return code sets the root domain - * state correctly. + * No need to restore root state in the 64-bit case, the Linux handler + * and the return code will take care of it. */ - __fixup_if(raw_irqs_disabled_flags(flags), regs); return 0; } ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Fwd: Re: [RFC][PATCH] x86: Fix root domain state restoring on exception return
Jan Kiszka wrote: Wolfgang Mauerer wrote: Kiszka, Jan wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Actually, we only need to save the state if we enter over the root domain and have to align its state to the hardware interrupt mask. Moreover, the x86-32 regs.eflags fix-up must happen based on the current root domain state to avoid more spurious corruptions. unfortunately, this won't boot on x86-32: During CPU bug checking in check_hlt, the kernel will really go into the halt state and never recover. By modifying __ipipe_handle_exception to use raw_irqs_disabled_flags as argument to __fixup_if instead of raw_irqs_disabled, everything is back to normal again. However, That would reintroduce the bug we saw on 64 bit to 32 bit (in a slight variation). So we need to understand what goes wrong here. Can you generate an ipipe panic trace? the problem is that when Linux does the hlt check in check_hlt, it relies on interrupts to occur after the hlt instructions have been issued - otherwise, the computer will really halt. This happens during early boot before ftrace is enabled, which makes tracing a bit hard... Besides, since we are stuck after the first hlt, its also hard to find a good point where to start the trace ;-) One missing part of the problem seems to be to figure out when to _really_ use __fixup_if, I did some more testing on the effects wrt. the bug handled in Jan's recent patch. 1.) With if (restore_flags) __fixup_regs(raw_irqs_disabled_flags(regs) ,regs); being used in __ipipe_handle_exception, the kernel boots properly (so it seems to do as well if __fixup_regs is called unconditionally) 2.) With __fixup_regs(raw_irq_disable(), regs), the boot process stops in check_hlt() in the Linux kernel. There seems to be a disagreement about the state of the hardware interrupt flag right before the kernel executes the tests in check_halt(). With the following code in __ipipe_handle_exception if (restore_flags) { rawflags = __raw_local_save_flags(); printk(before fixup: rawflags: %lx, flags: %lx, regs-flags: %lx\n, rawflags, flags, regs-flags); __fixup_if(raw_irqs_disabled_flags(flags), regs); printk(after fixup: rawflags: %lx, flags: %lx, regs-flags: %lx\n, rawflags, flags, regs-flags); } we can distinguish between case 1 and 2 (X86_FLAGS_IF=0x200) right before check_hlt(): case 1 (raw_irqs_disable_flags): [0.339060] before fixup: rawflags: 0, flags: 200, regs-flags: 246 [0.340001] after fixup: rawflags: 0, flags: 200, regs-flags: 246 case 2 (raw_irqs_disable): [0.345352] before fixup: rawflags: 0, flags: 200, regs-flags: 246 [0.346582] after fixup: rawflags: 0, flags: 200, regs-flags: 46 Consider __ipipe_unstall_iret_root. When regs-flags | X86_FLAGS_IF - is 0, the pipeline is stalled, but regs.flags =| X86_EFLAGS_IF = Ipipe recives interrupts, but does not pass them on to the Linux domain. - is X86_FLAGS_IF, then the pipeline is unstalled and interrupts are implicitely enabled by iret. Everything works as expected even if rawflags and regs.flags have disagreed upon entry to __ipipe_handle_exception This explains why everything works fine in case 1 (despite rawflags and regs-flags disagree), but fails in case 2. However, since the IRQ state obtained from the raw hardware settings via pushf in raw_irqs_disabled() disagrees with what Ipipe believes to be the hardware state in regs-flags, it would seem that the consistency between both is destroyed somewhere in the early boot process, and inadvertently fixed by __fixup_regs. Does that make sense? Or does anyone else have another idea what might be going on? For the sake of completeness, I've also attached the patch used for testing as it is right now. Thanks, Wolfgang commit 86e5bcfd0a39960e68475a5a6108088176d98aae Author: Wolfgang Mauerer wolfgang.maue...@siemens.com Date: Sun Jan 24 23:13:30 2010 +0100 x86: Fix root domain state restoring on exception return diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c index 4442d96..b8a6ec7 100644 --- a/arch/x86/kernel/ipipe.c +++ b/arch/x86/kernel/ipipe.c @@ -702,19 +702,22 @@ static int __ipipe_xlate_signo[] = { int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) { - unsigned long flags; + bool restore_flags = false; + unsigned long flags = 0; + unsigned long regflags; + unsigned long rawflags; /* Pick up the root domain state of the interrupted context. */ local_save_flags(flags); - if (ipipe_root_domain_p) { + if (ipipe_root_domain_p irqs_disabled_hw()) { /* * Replicate hw interrupt state into the virtual
Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return
Jan Kiszka wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Instead, only save and restore them if we were entering over root. However, the x86-32 regs.flags fixup is required nevertheless to take care of mismatches between the root domain state and the hardware flags on entry. That may happen if we fault in the iret path. But also in this case we must not restore an invalid root domain state. So if we entered over non-root, pick up the input for __fixup_if from the root domain after running the ipipe handler. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- Next try. But this time I think I finally understood what scenario __fixup_if is actually fixing. Please correct me if I'm still missing one. looks good - it works for my test cases and solves the problems with the hw/pipeline state mismatch during early bootup. But do you happen to have any scenario at hand with ipipe_domain_root_p !root_entry? Couldn't trigger this one yet so only the raw_irqs_disabled_flags fixup is excercised, though I guess it can't do any harm to really ensure that the explanation fits reality this time... Wolfgang arch/x86/kernel/ipipe.c | 81 +- 1 files changed, 51 insertions(+), 30 deletions(-) diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c index 4442d96..b471355 100644 --- a/arch/x86/kernel/ipipe.c +++ b/arch/x86/kernel/ipipe.c @@ -702,19 +702,23 @@ static int __ipipe_xlate_signo[] = { int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) { - unsigned long flags; - - /* Pick up the root domain state of the interrupted context. */ - local_save_flags(flags); + bool root_entry = false; + unsigned long flags = 0; if (ipipe_root_domain_p) { - /* - * Replicate hw interrupt state into the virtual mask before - * calling the I-pipe event handler over the root domain. Also - * required later when calling the Linux exception handler. - */ - if (irqs_disabled_hw()) + root_entry = true; + + local_save_flags(flags); + + if (irqs_disabled_hw()) { + /* + * Replicate hw interrupt state into the virtual mask + * before calling the I-pipe event handler over the + * root domain. Also required later when calling the + * Linux exception handler. + */ local_irq_disable(); + } } #ifdef CONFIG_KGDB /* catch exception KGDB is interested in over non-root domains */ @@ -725,18 +729,22 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) #endif /* CONFIG_KGDB */ if (unlikely(ipipe_trap_notify(vector, regs))) { - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 1; } - /* - * 32-bit: In case we migrated to root domain inside the event - * handler, restore the original IF from exception entry as the - * low-level return code will evaluate it. - */ - __fixup_if(raw_irqs_disabled_flags(flags), regs); - - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* + * 32-bit: In case we faulted in the iret path, regs.flags do + * not match the root domain state as the low-level return + * code will evaluate it. Fix this up, either by the root + * state sampled on entry or, if we migrated to root, with the + * current state. + */ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); + } else { /* Detect unhandled faults over non-root domains. */ struct ipipe_domain *ipd = ipipe_current_domain; @@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) * Relevant for 64-bit: Restore root domain state as the low-level * return code will not align it to regs.flags. */ - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 0; } int __ipipe_divert_exception(struct pt_regs *regs, int vector) { - unsigned long flags; - - /* Same root state handling as in __ipipe_handle_exception. */ - local_save_flags(flags); + bool root_entry = false; + unsigned long flags = 0; if
Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return
Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Instead, only save and restore them if we were entering over root. However, the x86-32 regs.flags fixup is required nevertheless to take care of mismatches between the root domain state and the hardware flags on entry. That may happen if we fault in the iret path. But also in this case we must not restore an invalid root domain state. So if we entered over non-root, pick up the input for __fixup_if from the root domain after running the ipipe handler. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- Next try. But this time I think I finally understood what scenario __fixup_if is actually fixing. Please correct me if I'm still missing one. looks good - it works for my test cases and solves the problems with the hw/pipeline state mismatch during early bootup. But do you happen to have any scenario at hand with ipipe_domain_root_p !root_entry? Couldn't trigger this one yet so only the raw_irqs_disabled_flags fixup is excercised, though I guess it can't do any harm to really ensure that the explanation fits reality this time... You mean non-root entry - migration - __fixup_if? In that case we pick up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or what do you mean? (...) - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* +* 32-bit: In case we faulted in the iret path, regs.flags do +* not match the root domain state as the low-level return +* code will evaluate it. Fix this up, either by the root +* state sampled on entry or, if we migrated to root, with the +* current state. +*/ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); I'm referring to the case that evaluates to __fixup_if(raw_irqs_disabled(), regs); That is, something that triggers if (!root_entry) do_something(); Could be that we're talking about to the same case, although I'm not sure ;-) Cheers, Wolfgang + } else { /* Detect unhandled faults over non-root domains. */ struct ipipe_domain *ipd = ipipe_current_domain; @@ -770,21 +778,29 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector) * Relevant for 64-bit: Restore root domain state as the low-level * return code will not align it to regs.flags. */ - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 0; } int __ipipe_divert_exception(struct pt_regs *regs, int vector) { - unsigned long flags; - - /* Same root state handling as in __ipipe_handle_exception. */ - local_save_flags(flags); + bool root_entry = false; + unsigned long flags = 0; if (ipipe_root_domain_p) { - if (irqs_disabled_hw()) + root_entry = true; + + local_save_flags(flags); + + if (irqs_disabled_hw()) { + /* +* Same root state handling as in +* __ipipe_handle_exception. +*/ local_irq_disable(); + } } #ifdef CONFIG_KGDB /* catch int1 and int3 over non-root domains */ @@ -804,16 +820,21 @@ int __ipipe_divert_exception(struct pt_regs *regs, int vector) #endif /* CONFIG_KGDB */ if (unlikely(ipipe_trap_notify(vector, regs))) { - local_irq_restore_nosync(flags); + if (root_entry) + local_irq_restore_nosync(flags); return 1; } + if (likely(ipipe_root_domain_p)) { + /* see __ipipe_handle_exception */ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); + } + /* -* 32-bit: Due to possible migration inside the event handler, we have -* to restore IF so that low-level return code sets the root domain -* state correctly. +* No need to restore root state in the 64-bit case, the Linux handler +* and the return code will take care of it. */ - __fixup_if(raw_irqs_disabled_flags(flags), regs); return 0; } Jan ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return
Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Instead, only save and restore them if we were entering over root. However, the x86-32 regs.flags fixup is required nevertheless to take care of mismatches between the root domain state and the hardware flags on entry. That may happen if we fault in the iret path. But also in this case we must not restore an invalid root domain state. So if we entered over non-root, pick up the input for __fixup_if from the root domain after running the ipipe handler. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- Next try. But this time I think I finally understood what scenario __fixup_if is actually fixing. Please correct me if I'm still missing one. looks good - it works for my test cases and solves the problems with the hw/pipeline state mismatch during early bootup. But do you happen to have any scenario at hand with ipipe_domain_root_p !root_entry? Couldn't trigger this one yet so only the raw_irqs_disabled_flags fixup is excercised, though I guess it can't do any harm to really ensure that the explanation fits reality this time... You mean non-root entry - migration - __fixup_if? In that case we pick up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or what do you mean? (...) - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* + * 32-bit: In case we faulted in the iret path, regs.flags do + * not match the root domain state as the low-level return + * code will evaluate it. Fix this up, either by the root + * state sampled on entry or, if we migrated to root, with the + * current state. + */ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); I'm referring to the case that evaluates to __fixup_if(raw_irqs_disabled(), regs); That is, something that triggers if (!root_entry) do_something(); Could be that we're talking about to the same case, although I'm not sure ;-) Right, that's the case I described above. What problem do you precisely see or what concerns to you have about the suggested behavior? None. I'd just like to be able to trigger it to avoid that there are any unforseen problems we're still missing. Since this corner of Ipipe seems to have proven tricky before AFAIK, I thought it might perhaps be worth while to really excercise every possible code path. Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return
Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Instead, only save and restore them if we were entering over root. However, the x86-32 regs.flags fixup is required nevertheless to take care of mismatches between the root domain state and the hardware flags on entry. That may happen if we fault in the iret path. But also in this case we must not restore an invalid root domain state. So if we entered over non-root, pick up the input for __fixup_if from the root domain after running the ipipe handler. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- Next try. But this time I think I finally understood what scenario __fixup_if is actually fixing. Please correct me if I'm still missing one. looks good - it works for my test cases and solves the problems with the hw/pipeline state mismatch during early bootup. But do you happen to have any scenario at hand with ipipe_domain_root_p !root_entry? Couldn't trigger this one yet so only the raw_irqs_disabled_flags fixup is excercised, though I guess it can't do any harm to really ensure that the explanation fits reality this time... You mean non-root entry - migration - __fixup_if? In that case we pick up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or what do you mean? (...) - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* +* 32-bit: In case we faulted in the iret path, regs.flags do +* not match the root domain state as the low-level return +* code will evaluate it. Fix this up, either by the root +* state sampled on entry or, if we migrated to root, with the +* current state. +*/ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); I'm referring to the case that evaluates to __fixup_if(raw_irqs_disabled(), regs); That is, something that triggers if (!root_entry) do_something(); Could be that we're talking about to the same case, although I'm not sure ;-) Right, that's the case I described above. What problem do you precisely see or what concerns to you have about the suggested behavior? None. I'd just like to be able to trigger it to avoid that there are any unforseen problems we're still missing. Since this corner of Ipipe seems to have proven tricky before AFAIK, I thought it might perhaps be worth while to really excercise every possible code path. To trigger it, you need to recreate the scenario that our colleagues managed to generate on the MARS: Have Linux IRQs disabled (or enabled, to have both variations), schedule in a Xenomai task, let it raise a fault so that Xenomai migrates it back. Yep, but allocating a large amount of memory in non-rt, switching to rt and then accessing this and switching back to non-rt in the hope of generating many faults so that the scenario would be synthesised without expensive and hard to access hardware was unsuccessful, so my little innocent question was really just if you happened to have a testcase. But as I said in the very beginning, the patch nevertheless looks good to me. Cheers, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH v2] x86: Fix root domain state restoring on exception return
Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: Jan Kiszka wrote: If we enter __ipipe_handle_exception over a non-root domain and leave it due to migration in the event handler over root, we must not restore the root domain state so far saved on entry. This caused subtle pipeline state corruptions. Instead, only save and restore them if we were entering over root. However, the x86-32 regs.flags fixup is required nevertheless to take care of mismatches between the root domain state and the hardware flags on entry. That may happen if we fault in the iret path. But also in this case we must not restore an invalid root domain state. So if we entered over non-root, pick up the input for __fixup_if from the root domain after running the ipipe handler. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- Next try. But this time I think I finally understood what scenario __fixup_if is actually fixing. Please correct me if I'm still missing one. looks good - it works for my test cases and solves the problems with the hw/pipeline state mismatch during early bootup. But do you happen to have any scenario at hand with ipipe_domain_root_p !root_entry? Couldn't trigger this one yet so only the raw_irqs_disabled_flags fixup is excercised, though I guess it can't do any harm to really ensure that the explanation fits reality this time... You mean non-root entry - migration - __fixup_if? In that case we pick up the flags for fixup _after_ the migration (raw_irqs_disabled()). Or what do you mean? (...) - if (unlikely(!ipipe_root_domain_p)) { + if (likely(ipipe_root_domain_p)) { + /* + * 32-bit: In case we faulted in the iret path, regs.flags do + * not match the root domain state as the low-level return + * code will evaluate it. Fix this up, either by the root + * state sampled on entry or, if we migrated to root, with the + * current state. + */ + __fixup_if(root_entry ? raw_irqs_disabled_flags(flags) : + raw_irqs_disabled(), regs); I'm referring to the case that evaluates to __fixup_if(raw_irqs_disabled(), regs); That is, something that triggers if (!root_entry) do_something(); Could be that we're talking about to the same case, although I'm not sure ;-) Right, that's the case I described above. What problem do you precisely see or what concerns to you have about the suggested behavior? None. I'd just like to be able to trigger it to avoid that there are any unforseen problems we're still missing. Since this corner of Ipipe seems to have proven tricky before AFAIK, I thought it might perhaps be worth while to really excercise every possible code path. To trigger it, you need to recreate the scenario that our colleagues managed to generate on the MARS: Have Linux IRQs disabled (or enabled, to have both variations), schedule in a Xenomai task, let it raise a fault so that Xenomai migrates it back. Yep, but allocating a large amount of memory in non-rt, switching to rt and then accessing this and switching back to non-rt in the hope of generating many faults so that the scenario would be synthesised without expensive and hard to access hardware was unsuccessful, so my little innocent question was really just if you happened to have a testcase. OK, then I totally misunderstood your question. Seems we have nevertheless finally managed to reach an agreement on what we mean ;-) You do not need to generate a fixable fault, and unfixable one (*(int *)NULL = 0;) should suffice. yeah, although that would maybe be a bit ugly wrt. handling the SEGV and continuing to allow continuous faults without having to restart the program all the time. As we just discussed offline, this would anyway not yet trigger the fault in iret-scenario, and I don't think it's worth while spending the large extra effort for this considering that the code will sooner or later die anyway. Or does maybe anyone have an idea how to trigger such a fault with simple means? Cheers, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Potential heap corruption on thread cleanup
Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Hi Gilles, I'm pushing your findings to the list, also as my colleagues showed strong interest - this thing may explain rare corruptions for us as well. I thought a bit about that likely u_mode-related crash in your test case and have the following theory so far: If the xeno_current_mode storage is allocated on the application heap (!HAVE_THREAD, that's also what we are forced to use), it is automatically freed on thread termination in the context of the dying thread. If the thread is already migrated to secondary or if that happens while it is cleaned up (i.e. before calling for exit into the kernel), there is no problem, Xenomai will not touch the mode storage anymore. But if the thread happens to delete the storage silently, without any migration, the final exit will trigger one further access. And that takes place against an invalid head area at this point. Does this make sense? Yes, it is the issue we observed. If that is true, all we need to do is to force a migration before releasing the mode storage. Could you check this? No, that does not fly. Calling, for instance, __wrap_pthread_mutex_lock in another TSD cleanup function is which could be called after the current_mode TSD cleanup is allowed and could trigger a switch to primary mode and a write to the u_mode. Good point. Mmh. Another, but ABI-breaking, way would be to add a syscall for deregistering the u_mode pointer... That is the thing we did to verify that we had this bug. But this syscall would be also called too soon, and suffers from the TSD cleanup functions order again. Right, the only complete fix without losing functionality is to add an option to our ABI for requesting kernel-managed memory if dynamic allocation is necessary (i.e. no TLS is available). No. TLS may as well suffer from the same issue, since it is handled by the glibc or libgcc, over which we have no control. So yes, it may work by chance today, but may as well stop working tomorrow. We use kernel-managed memory all the time, final point. I think we are still in the solution finding process, no need for early conclusions. See, we actually do not need kernel-managed storage for u_mode at all. u_mode is an optimization, mostly for our fast user space mutexes. We can indeed switch off all updates by the kernel and will still be able to provide all required features - just less optimally. Adding a third state, invalid, we can make all mutex users assume they need the slow syscall path on uncontended acquisition. And assert_nrt will probably be happy about a syscall replacement for u_mode when it became invalid. Thinking about the fast part in fast userspace mutex: Would it be an argument in favour of not using the global semaphore heap that said memory is uncached on some architectures? Or is that irrelevant? Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH] Fix x86_64 builds with CC_STACKPROTECTOR enabled
From: Wolfgang Mauerer (none) wolfg...@dirichlet The per_cpu__ prefix for per-cpu-variables in the Linux kernel is gone since dd17c8f72993f. This patch adapts the x86_64 task switching code to cope with kernels before and after this commit. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Acked-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-x86/switch_64.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/include/asm-x86/switch_64.h b/include/asm-x86/switch_64.h index a716ad6..c2df7d8 100644 --- a/include/asm-x86/switch_64.h +++ b/include/asm-x86/switch_64.h @@ -37,6 +37,10 @@ struct xnarch_x8664_initstack { }; #ifdef CONFIG_CC_STACKPROTECTOR +#include asm/percpu.h +#ifndef per_cpu_var +#define per_cpu_var(x) x +#endif /* * We have an added complexity with -fstack-protector, due to the * hybrid scheduling between user- and kernel-based Xenomai threads, -- 1.7.0.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH] Fix x86_64 builds with CC_STACKPROTECTOR enabled
The per_cpu__ prefix for per-cpu-variables in the Linux kernel is gone since dd17c8f72993f. This patch adds a corresponding wrapper. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Acked-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-generic/wrappers.h |4 include/asm-x86/switch_64.h|3 +++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/include/asm-generic/wrappers.h b/include/asm-generic/wrappers.h index ecc1867..3827492 100644 --- a/include/asm-generic/wrappers.h +++ b/include/asm-generic/wrappers.h @@ -598,4 +598,8 @@ static inline void wrap_proc_dir_entry_owner(struct proc_dir_entry *entry) raw_spin_unlock_irqrestore(rthal_irq_descp(irq)-lock, flags) #endif /* LINUX_VERSION_CODE = KERNEL_VERSION(2,6,33) */ +#if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,34) +#define per_cpu_var(x) x +#endif + #endif /* _XENO_ASM_GENERIC_WRAPPERS_H */ diff --git a/include/asm-x86/switch_64.h b/include/asm-x86/switch_64.h index a716ad6..a436917 100644 --- a/include/asm-x86/switch_64.h +++ b/include/asm-x86/switch_64.h @@ -25,6 +25,9 @@ #error Pure kernel header included from user-space! #endif +#include asm/percpu.h +#include asm/xenomai/wrappers.h + struct xnarch_x8664_initstack { #ifdef CONFIG_CC_STACKPROTECTOR -- 1.7.0.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Question about getting system time
[moved to xenomai-core] Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: - gettimeofday should not have another timebase than clock_gettime(CLOCK_REALTIME): in other word, the whole clock system should be based on the ntp clock. Sorry, I'm not quite sure what you are talking about here. The NTP corrections are provided for the clock offered in the vsyscall page by Linux, so the clock is based on the NTP clock, isn't it? Or am I misparsing your statement? Unless I misunderstood your patch, what you provide is: gettimeofday which uses linux timebase clock_gettime which uses Xenomai timebase unrelated to linux' timebase This is unacceptable. What we want is a unique timebase. okay, I see your point now. But this sound more like 3.0 stuff, doesn't it? The goal right now is just to have some code that generates synchronised, NTP corrected timestamps on Linux and Xenomai, not to rework the base timer handling. And since this will have a cost not everyone is willing to pay, I have another requirement: this code should be compiled conditionally. - you should not use vread, you should use __xn_rdtsc directly in user-space, otherwise, your code would only work on x86. Of course this means that the I-pipe should create a clocksource with whatever hardware counter the architecture is providing to Xenomai. This also means that the I-pipe should declare a clocksource using whatever hardware counter is provided by the architecture with highest priority than other clocksources, to ensure that Linux is using the same clock source as Xenomai, and that NTP is correcting that clock source. This would also remove the issue with Linux declaring the tsc unstable. the reason why it's based on vread() is because the Linux kernel automatically makes sure that it points to a function that can be called from userland, so why would it only run on x86? Currently, the userland patch is limited to x86 because I've only adapted the sequence counter for this arch. Besides, vread() could also work with a different source than the TSC as long as it's accessible from userland. vread is a function pointer call, which: - requires vsyscalls, currently only implemented on x86 (and maybe ppc) - is function pointer call, so damn slow on low end hardware. ... and fast as lightning on x86 when the call is made often, which is the important case when speed matters. So it makes sense on this platform. Xenomai has __xn_rdtsc on all architectures, so, we should be based on that. Since what we want is Xenomai to use the same clock source as Linux, and anything else than tsc is not implemented for Xenomai, we should implement tsc first and keep other clock sources for later. And when we use another clock, __xn_rdtsc will use that clock anyway. - why the transactionnal mechanism at all? Why not simply using seqlocks with an ipipe_spinlock, and do the update with irqs off? the locking section is much shorter than, say, xnpod_schedule, so it will not have any influence on the worst case latency, and the reader side will still be lockless. Because even if we don't increase the peak latency, we'll still increase the average latency. Additionally, it would also be possible to extend the base mechanism to an RCU-style communication channel between the kernel and Xenomai in the long run, so I'd argue that the lock-less solution is nicer. It is a useless optimization, it is a lot of code to avoid shutting interrupts of for a handful of assignments (since you can avoid copying vread, and mult and shift which never change if we enfore the clock source). There are many more, longer masking sections everywhere in the I-pipe patch and in Xenomai code. On the one hand you make complicated code (which will be costly on low end hardware) to avoid shutting interrupts around a few assignments, but on the other hand you leave an architecture specific function pointer call where we want a fast behaviour on average (remember, we do all this to avoid a system call, which is only a few hundreds nanoseconds on your big iron x86), and where we have a generic fast replacement. Sometimes, I do not understand your logic. But using the same argument, you could get rid of Linux vsyscall based gettimeofday()... Would having the synchronised time stamp feature (maybe by another name than gettimeofday()) conditionally compiled on x86 be acceptable? Best, Wolfgang - the NTP event should trigger an ipipe event with ipipe_dispatch_event. could do, but I was following Gilles' suggestion ;-) to use an arch-specific hook since it's easier to maintain in the long run than writing a generic function and replacing every call to vsyscall_update(), also the future. (http://www.opensubscriber.com/message/xenomai-core@gna.org/13126830.html, although I can change that, certainly no religious issues here) What I mean is that vsyscall_update should trigger ipipe_dispatch_event. This is what I meant from
Re: [Xenomai-core] Question about getting system time
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: On the one hand you make complicated code (which will be costly on low end hardware) to avoid shutting interrupts around a few assignments, but on the other hand you leave an architecture specific function pointer call where we want a fast behaviour on average (remember, we do all this to avoid a system call, which is only a few hundreds nanoseconds on your big iron x86), and where we have a generic fast replacement. Sometimes, I do not understand your logic. But using the same argument, you could get rid of Linux vsyscall based gettimeofday()... I do not see your point, the Linux code does not go a long way to make lockless code, it simply turns off interrupts around the gtod data update, which is really reasonable given the size of the masking section. The reading is lockless, the writing is not. I was referring to the argument that system calls are so fast that replacing gtod with a syscall-less version that uses function pointer dereferencing instead does not make much of a difference. Be it as it may, I need to check how far our budget can cover the (much more comprehensive) modifications required for the solution suggested by you. Let's see. Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] Question about getting system time
Gilles Chanteperdrix wrote: Jan Kiszka wrote: Gilles Chanteperdrix wrote: Jan Kiszka wrote: Just like it seems to be the case for Steve (unless I misunderstood his reply), it is very useful for us being able to time-stamp events in RT context that need to be correlated with events stamped in non-RT (including non-Xenomai) parts or even on other systems: (offline) data fusion, logging, tracing. I even bet that this is currently the major use case for synchronized clocks, only a smaller part already has the need to synchronize timed activities on a common clock source. But there is huge potential in the second part once we can provide a stable infrastructure. I already had such issues, and I did not solve them by modifying Xenomai core. Even a third clock would have to be delivered for more archs than x86, no question. We would basically need a generic but slow syscall variant and per-arch syscall-less optimizations (where feasible). So, you would add a syscall which would becomre useless when you have implemented synchronized clocks properly? Yet another reason for avoiding this solution. Could be CLOCK_LINUX - ie. no need for a new syscall. I am Ok for this solution (and now that I think about it, I wonder if we did not already have this discussion). Anyway, I would go for CLOCK_HOST_REALTIME, in case someone wants to implement CLOCK_HOST_MONOTONIC. The advantage is that we can return EINVAL in the timer_create or clock_settime calls, to indicate clearly that using this clock for timing services is verboten. And when/if the full synchronization is implemented, the ID simply becomes a #define. okay, so let's pursue this path. Patches will follow. Best, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH, RFC] Instrumentation to detect delayed timers
Hi, the attached patch augments the timer expiration handler with some checks for excessive timer latencies. Depending on the configuration options, it freezes the ipipe tracer and creates a list of delayed timers. This code was useful for us in debugging some problems with faulty hardware. Would such a thing be suitable for mainline? Regards, Wolfgang --- commit 9c2a3908ac4aff21022a577552002fc2622d3670 Author: Wolfgang Mauerer wolfgang.maue...@siemens.com Date: Mon Jun 7 15:34:39 2010 +0200 This patch provides the possibility to check upon timer expiration how much a timer has been delayed. When the delay surpasses a configurable threshold, the ipipe tracer is frozen. Additionally, the patch allows for the creation of a list of timers that have suffered from excessive delays. The mechanism can be disabled at both, compile- and runtime. There is no overhead in the former case. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com diff --git a/ksrc/nucleus/Kconfig b/ksrc/nucleus/Kconfig index 211a4ad..ffed3f8 100644 --- a/ksrc/nucleus/Kconfig +++ b/ksrc/nucleus/Kconfig @@ -293,6 +293,24 @@ config XENO_OPT_DEBUG_TIMERS This option activates debugging output for critical timer-related operations performed by the Xenomai core. +config XENO_OPT_DEBUG_TIMER_LATENCY + bool Log excessive timer latencies + depends on XENO_OPT_DEBUG (XENO_OPT_STATS || IPIPE_TRACE) + help + + This option activates recording of excessive timer latencies. + In contrast to the userland latency checker which is statistical + in nature, this test checks every expiring timer and is thus + more precise. However, it adds some runtime overhead when + activated. + + When the ipipe tracer is enabled, the trace is frozen + when a latency maximum above the threshold set in + the debugfs file timer_lattest/lat_threshold_us is detected. + When Xenomai statistics collection is enabled, a list of + excessive latencies together with the names of the timers + is kept in the debugfs file timer_lattest/exc_timer_list. + config XENO_OPT_DEBUG_SYNCH_RELAX bool Detect mutexes held in relaxed sections depends on XENO_OPT_PERVASIVE XENO_OPT_DEBUG diff --git a/ksrc/nucleus/timer.c b/ksrc/nucleus/timer.c index d813c4f..01dad44 100644 --- a/ksrc/nucleus/timer.c +++ b/ksrc/nucleus/timer.c @@ -36,11 +36,37 @@ * *...@{*/ +#include linux/debugfs.h #include nucleus/pod.h #include nucleus/thread.h #include nucleus/timer.h +#include nucleus/trace.h #include asm/xenomai/bits/timer.h +#ifdef CONFIG_XENO_OPT_DEBUG_TIMER_LATENCY +#define NANO 10ll +#define MU 100ll +#define NANO_PER_MU (NANO/MU) + +static u32 lat_thresh = 0; /* in \mu s, set to zero to disable */ + +#ifdef CONFIG_XENO_OPT_STATS +#include linux/seq_file.h + +#define NUM_REC_LATENCIES 500 +static int num_lat_exc_timers = -1; +static bool lat_rec_overflow = false; /* Set when more than NUM_REC_LATENCIES + * excessive latencies have been + * observed */ +struct lat_rec { + char name[XNOBJECT_NAME_LEN]; + xnsticks_t delta; +}; + +static struct lat_rec delayed_timers[NUM_REC_LATENCIES]; +#endif // CONFIG_XENO_OPT_STATS +#endif // XENO_OPT_DEBUG_TIMER_LATENCY + static inline void xntimer_enqueue_aperiodic(xntimer_t *timer) { xntimerq_t *q = timer-sched-timerqueue; @@ -368,6 +394,32 @@ void xntimer_tick_aperiodic(void) if (delta (xnsticks_t)nklatency) break; +#ifdef CONFIG_XENO_OPT_DEBUG_TIMER_LATENCY + /* +* Check for severly delayed timers, but omit the host tick +* timer as it can be delayed intentionally, thus causing +* spurious excessive latencies (see +* xntimer_next_local_shot()). +*/ + if (lat_thresh (timer != sched-htimer)) { + delta = xnarch_tsc_to_ns(-delta); + if (delta ((xnsticks_t)lat_thresh)*NANO_PER_MU) { + xnarch_trace_user_freeze(delta, 1); +#ifdef CONFIG_XENO_OPT_STATS + if (num_lat_exc_timers NUM_REC_LATENCIES - 1) { + num_lat_exc_timers++; + } else { + lat_rec_overflow = true; + num_lat_exc_timers = 0; + } + xnobject_copy_name(delayed_timers[num_lat_exc_timers].name, + timer-name); + delayed_timers[num_lat_exc_timers].delta = delta; +#endif // XENO_OPT_STATS + } + } +#endif // XENO_OPT_DEBUG_TIMER_LATENCY
Re: [Xenomai-core] [PATCH, RFC] Instrumentation to detect delayed timers
Philippe Gerum wrote: On Tue, 2010-06-08 at 16:21 +0200, Wolfgang Mauerer wrote: Hi, the attached patch augments the timer expiration handler with some checks for excessive timer latencies. Depending on the configuration options, it freezes the ipipe tracer and creates a list of delayed timers. This code was useful for us in debugging some problems with faulty hardware. Would such a thing be suitable for mainline? No, I don't think so, albeit the debug feature it brings in makes sense. - the code, as is, adds significant cruft to the timer code, which is per se a problem. Debug is ok, but I really would like that code to have a much lower impact source-wise. so something along the lines of xntimer_enqueue_aperiodic(...) { ... large_latency_hook_whatever(args); ... } where large_latency_hook_whatever() is #ifdef'd to nop if not compile-time configured would be acceptable I suppose? - dependency on debugfs is wrong; we still have to take care of legacy 2.4 kernels. I know, this is a real PITA, and I'm more than willing to get rid of that heritage in 3.x, but for now, 2.x still requires to be 2.4-compatible. oh, yeah. procfs would be a replacement, but using debugfs via ipipe is much nicer since this is definitely debugging stuff. - but, beyond this, this is something that should go into the I-pipe tracer instead, exporting a new dedicated interface to log a high latency event. There, you could use whatever fits to implement it, since there is by definition no consideration for compat with legacy kernels. I'm thinking of having ipipe_trace_latency(unsigned long delta, const char *source). That would be reusable in other, non-Xenomai contexts as well. Sounds reasonable. Would it make sense to augment ipipe_trace_latency() with an additional type argument to distinguish different latency sources, or is this overkill? Best, Wolfgang Additionally, the latency spots could appear as events in the tracer log as well, which may help debugging them a lot, since you would know about the actual context. Regards, Wolfgang --- commit 9c2a3908ac4aff21022a577552002fc2622d3670 Author: Wolfgang Mauerer wolfgang.maue...@siemens.com Date: Mon Jun 7 15:34:39 2010 +0200 This patch provides the possibility to check upon timer expiration how much a timer has been delayed. When the delay surpasses a configurable threshold, the ipipe tracer is frozen. Additionally, the patch allows for the creation of a list of timers that have suffered from excessive delays. The mechanism can be disabled at both, compile- and runtime. There is no overhead in the former case. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com diff --git a/ksrc/nucleus/Kconfig b/ksrc/nucleus/Kconfig index 211a4ad..ffed3f8 100644 --- a/ksrc/nucleus/Kconfig +++ b/ksrc/nucleus/Kconfig @@ -293,6 +293,24 @@ config XENO_OPT_DEBUG_TIMERS This option activates debugging output for critical timer-related operations performed by the Xenomai core. +config XENO_OPT_DEBUG_TIMER_LATENCY +bool Log excessive timer latencies +depends on XENO_OPT_DEBUG (XENO_OPT_STATS || IPIPE_TRACE) +help + +This option activates recording of excessive timer latencies. +In contrast to the userland latency checker which is statistical +in nature, this test checks every expiring timer and is thus +more precise. However, it adds some runtime overhead when +activated. + +When the ipipe tracer is enabled, the trace is frozen +when a latency maximum above the threshold set in +the debugfs file timer_lattest/lat_threshold_us is detected. +When Xenomai statistics collection is enabled, a list of +excessive latencies together with the names of the timers +is kept in the debugfs file timer_lattest/exc_timer_list. + config XENO_OPT_DEBUG_SYNCH_RELAX bool Detect mutexes held in relaxed sections depends on XENO_OPT_PERVASIVE XENO_OPT_DEBUG diff --git a/ksrc/nucleus/timer.c b/ksrc/nucleus/timer.c index d813c4f..01dad44 100644 --- a/ksrc/nucleus/timer.c +++ b/ksrc/nucleus/timer.c @@ -36,11 +36,37 @@ * *...@{*/ +#include linux/debugfs.h #include nucleus/pod.h #include nucleus/thread.h #include nucleus/timer.h +#include nucleus/trace.h #include asm/xenomai/bits/timer.h +#ifdef CONFIG_XENO_OPT_DEBUG_TIMER_LATENCY +#define NANO 10ll +#define MU 100ll +#define NANO_PER_MU (NANO/MU) + +static u32 lat_thresh = 0; /* in \mu s, set to zero to disable */ + +#ifdef CONFIG_XENO_OPT_STATS +#include linux/seq_file.h + +#define NUM_REC_LATENCIES 500 +static int num_lat_exc_timers = -1; +static bool lat_rec_overflow = false; /* Set when more than NUM_REC_LATENCIES + * excessive latencies have been + * observed */ +struct lat_rec { +char name[XNOBJECT_NAME_LEN
[Xenomai-core] [PATCH 2/7] nucleus: Sanity check for vdso.h
Including the file only makes sense when support for pervasive rt is enabled, so add a corresponding check. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/nucleus/vdso.h |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/include/nucleus/vdso.h b/include/nucleus/vdso.h index c431f88..2343e5d 100644 --- a/include/nucleus/vdso.h +++ b/include/nucleus/vdso.h @@ -23,6 +23,10 @@ * 02111-1307, USA. */ +#if defined(__KERNEL__) !defined(CONFIG_XENO_OPT_PERVASIVE) +#error vdso.h included in a kernel configuration without pervasive rt support! +#endif + #include nucleus/types.h /* -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 1/7] nucleus: Spelling fix for check-vdso.c
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/testsuite/unit/check-vdso.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/testsuite/unit/check-vdso.c b/src/testsuite/unit/check-vdso.c index 8f90420..17684de 100644 --- a/src/testsuite/unit/check-vdso.c +++ b/src/testsuite/unit/check-vdso.c @@ -15,7 +15,7 @@ int main(int argc, char **argv) unsigned long long test_features; if (argc != 2) { - printf(No specific feature(s) given, using XNVDSO_FEATURE\n); + printf(No specific feature(s) given, using XNVDSO_FEATURES\n); test_features = XNVDSO_FEATURES; } else { test_features = strtoull(argv[1], NULL, 0); -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 4/7] nucleus: Add CLOCK_HOST_REALTIME bits to nkvdso
Augment the shared vdso page with all data required to implement a clock CLOCK_HOST_REALTIME that provides time information synchronised with the NTP-corrected time in the Linux kernel. The definition of the hostrt data is placed in a separate head file so that we can use it irregardless of pervasive rt support is compiled in or not. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-generic/hal.h|2 + include/asm-generic/system.h |4 ++ include/asm-sim/system.h |4 ++ include/nucleus/Makefile.am |1 + include/nucleus/hostrt.h | 65 ++ include/nucleus/vdso.h | 16 -- ksrc/nucleus/Kconfig |4 ++ ksrc/nucleus/module.c| 25 8 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 include/nucleus/hostrt.h diff --git a/include/asm-generic/hal.h b/include/asm-generic/hal.h index f90cafa..4203a3b 100644 --- a/include/asm-generic/hal.h +++ b/include/asm-generic/hal.h @@ -680,6 +680,8 @@ static inline int rthal_trace_panic_dump(void) #endif /* CONFIG_IPIPE_TRACE */ +#define rthal_set_hostrt_data ipipe_set_hostrt_data +#define rthal_hostrt_data ipipe_hostrt_data #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/include/asm-generic/system.h b/include/asm-generic/system.h index a2c8fb9..a4e278a 100644 --- a/include/asm-generic/system.h +++ b/include/asm-generic/system.h @@ -490,4 +490,8 @@ static inline void xnarch_hisyscall_entry(void) { } #define xnarch_post_graph(obj,state) #define xnarch_post_graph_if(obj,state,cond) +/* Synchronised realtime clock*/ +#define xnarch_set_hostrt_data rthal_set_hostrt_data +#define xnarch_hostrt_data rthal_hostrt_data + #endif /* !_XENO_ASM_GENERIC_SYSTEM_H */ diff --git a/include/asm-sim/system.h b/include/asm-sim/system.h index 1a5a875..56af408 100644 --- a/include/asm-sim/system.h +++ b/include/asm-sim/system.h @@ -598,4 +598,8 @@ static inline long IS_ERR(const void *ptr) return IS_ERR_VALUE((unsigned long)ptr); } +/* Host realtime support is not supported in the simulator */ +#define xnarch_set_hostrt_data { } +struct xnarch_hostrt_data { }; + #endif /* !_XENO_ASM_SIM_SYSTEM_H */ diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am index 3a6a024..b04d7f6 100644 --- a/include/nucleus/Makefile.am +++ b/include/nucleus/Makefile.am @@ -6,6 +6,7 @@ includesub_HEADERS = \ bufd.h \ compiler.h \ heap.h \ + hostrt.h \ intr.h \ jhash.h \ map.h \ diff --git a/include/nucleus/hostrt.h b/include/nucleus/hostrt.h new file mode 100644 index 000..5230165 --- /dev/null +++ b/include/nucleus/hostrt.h @@ -0,0 +1,65 @@ +#ifndef _XENO_NUCLEUS_HOSTRT_H +#define _XENO_NUCLEUS_HOSTRT_H + +/*!\file hostrt.h + * \brief Definitions for global semaphore heap shared objects + * \author Wolfgang Mauerer + * + * Copyright (C) 2010 Wolfgang Mauerer wolfgang.maue...@siemens.com. + * + * Xenomai is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2 of the License, + * or (at your option) any later version. + * + * Xenomai is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Xenomai; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA + * 02111-1307, USA. + */ + +#include nucleus/types.h +#ifdef __KERNEL__ +#include linux/time.h +#include linux/ipipe_tickdev.h +#include asm-generic/xenomai/system.h + +#ifndef CONFIG_XENO_OPT_PERVASIVE +extern struct xnarch_hostrt_data *nkhostrt_data; + +static inline struct xnarch_hostrt_data *get_hostrt_data(void) +{ + return nkhostrt_data; +} +#endif /* !CONFIG_XENO_OPT_PERVASIVE */ +#else /* !__KERNEL__ */ +#include time.h +#include sys/types.h +#include nucleus/seqlock_user.h +typedef u_int32_t u32; +typedef u_int64_t cycle_t; + +/* + * 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. + */ +struct xnarch_hostrt_data { + short live; + seqcount_t seqcount; + time_t wall_time_sec; + u32 wall_time_nsec; + struct timespec wall_to_monotonic; + cycle_t cycle_last; + cycle_t mask; + u32 mult; + u32 shift; +}; +#endif /* !__KERNEL__ */ + +#endif diff --git a/include/nucleus/vdso.h b/include/nucleus/vdso.h index 2343e5d..57f2ee7 100644 --- a/include/nucleus/vdso.h +++ b/include/nucleus/vdso.h
[Xenomai-core] [PATCH 3/7] nucleus: Add userland cpu_relax() definition for x86
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-x86/atomic.h |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/include/asm-x86/atomic.h b/include/asm-x86/atomic.h index bb3ce46..16bc990 100644 --- a/include/asm-x86/atomic.h +++ b/include/asm-x86/atomic.h @@ -71,6 +71,11 @@ typedef struct { unsigned long counter; } xnarch_atomic_t; #define xnarch_write_memory_barrier() xnarch_memory_barrier() +static inline void cpu_relax(void) +{ + asm volatile(rep; nop ::: memory); +} + #ifdef __i386__ struct __xeno_xchg_dummy { unsigned long a[100]; }; -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 5/7] posix: Support reading the host realtime clock in realtime context
Wall time management is typically assisted by the NTP protocol in the Linux context, but this information is not propagated to Xenomai. This patch adds support for a CLOCK_HOST_REALTIME clock id that is coupled to the host operating system's realtime clock. The required information from the Kernel into Xenomai. The data exchange is designed to allow for lockless reading from userland. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/posix/time.h |2 + ksrc/skins/posix/clock.c | 90 +- 2 files changed, 91 insertions(+), 1 deletions(-) diff --git a/include/posix/time.h b/include/posix/time.h index 4f2d760..938feb6 100644 --- a/include/posix/time.h +++ b/include/posix/time.h @@ -50,6 +50,8 @@ #define CLOCK_MONOTONIC 1 #endif /* CLOCK_MONOTONIC */ +#define CLOCK_HOST_REALTIME 16 + #if defined(__KERNEL__) || defined(__XENO_SIM__) struct sigevent; diff --git a/ksrc/skins/posix/clock.c b/ksrc/skins/posix/clock.c index 553e123..f5a789b 100644 --- a/ksrc/skins/posix/clock.c +++ b/ksrc/skins/posix/clock.c @@ -50,6 +50,13 @@ *...@{*/ #include posix/thread.h +#include linux/ipipe_tickdev.h +#include linux/math64.h +#include nucleus/hostrt.h +#ifdef CONFIG_XENO_OPT_PERVASIVE +#include nucleus/vdso.h +#endif +#include asm-generic/xenomai/system.h /** * Get the resolution of the specified clock. @@ -88,6 +95,76 @@ int clock_getres(clockid_t clock_id, struct timespec *res) } /** + * Read the host-synchronised realtime clock. + * + * Obtain the current time with NTP corrections from the Linux domain + * + * @param tp pointer to a struct timespec + * + * @retval 0 on success; + * @retval -1 if no suitable NTP-corrected clocksource is availabel + * + * @see + * a href=http://www.opengroup.org/onlinepubs/95399/functions/gettimeofday.html; + * Specification./a + * + */ +static int do_clock_host_realtime(struct timespec *tp) +{ +#ifdef CONFIG_XENO_OPT_HOSTRT + cycle_t now, base, mask, cycle_delta; + unsigned long mult, shift, nsec, rem; + struct xnarch_hostrt_data *hostrt_data; + unsigned int seq; + + hostrt_data = get_hostrt_data(); + BUG_ON(!hostrt_data); + + if (unlikely(!hostrt_data-live)) + return -1; + + /* +* Note: Disabling HW interrupts around writes to hostrt_data ensures +* that a reader (on the Xenomai side) cannot interrupt a writer (on +* the Linux kernel side) on the same CPU. The sequence counter is +* required when a reader is interleaved by a writer on a different +* CPU. This follows the approach from userland, where tasking the +* spinlock is not possible. +*/ +retry: + seq = read_seqcount_begin(hostrt_data-seqcount); + + now = xnarch_get_cpu_tsc(); + base = hostrt_data-cycle_last; + mask = hostrt_data-mask; + mult = hostrt_data-mult; + shift = hostrt_data-shift; + tp-tv_sec = hostrt_data-wall_time_sec; + nsec = hostrt_data-wall_time_nsec; + + if (read_seqcount_retry(hostrt_data-seqcount, seq)) + goto retry; + + /* +* At this point, we have a consistent copy of the fundamental +* data structure - calculate the interval between the current +* and base time stamp cycles, and convert the difference +* to nanoseconds. +*/ + cycle_delta = (now - base) mask; + nsec += (cycle_delta * mult) shift; + + /* Convert to the desired sec, usec representation */ + tp-tv_sec += xnarch_divrem_billion(nsec, rem); + tp-tv_nsec = rem; + + return 0; +#else /* CONFIG_XENO_OPT_HOSTRT */ + return -1; +#endif +} + +/** * Read the specified clock. * * This service returns, at the address @a tp the current value of the clock @a @@ -97,8 +174,12 @@ int clock_getres(clockid_t clock_id, struct timespec *res) * - CLOCK_MONOTONIC, the clock value is given by an architecture-dependent high * resolution counter, with a precision independent from the system clock tick * duration. + * - CLOCK_HOST_REALTIME, the clock value as seen by the host, typically + * Linux. Resolution and precision depend on the host, but it is guaranteed + * that both, host and Xenomai, see the same information. * - * @param clock_id clock identifier, either CLOCK_REALTIME or CLOCK_MONOTONIC; + * @param clock_id clock identifier, either CLOCK_REALTIME, CLOCK_MONOTONIC, + *or CLOCK_HOST_REALTIME; * * @param tp the address where the value of the specified clock will be stored. * @@ -126,6 +207,13 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp) xnarch_uldivrem(cpu_time, ONE_BILLION, tp-tv_nsec); break; + case CLOCK_HOST_REALTIME: + if (do_clock_host_realtime(tp) != 0) { + thread_set_errno(EINVAL
[Xenomai-core] [PATCH 6/7] posix: Userspace hostrt reading without switching to kernel mode
From: Wolfgang Mauerer (none) wolfg...@dirichlet We can do this since the data are on the shared semaphore heap. The basis data structure is placed so that it is accessible from both the Linux kernel and Xenomai kernel/userland. This also requires to make the structure work with both kernel and userland definitions for elementary data types. We use a seqlock retry mechanism to ensure that we obtain a consistent data set. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/nucleus/Makefile.am|1 + include/nucleus/seqlock_user.h | 57 +++ src/skins/posix/clock.c| 65 3 files changed, 123 insertions(+), 0 deletions(-) create mode 100644 include/nucleus/seqlock_user.h diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am index b04d7f6..9a72b24 100644 --- a/include/nucleus/Makefile.am +++ b/include/nucleus/Makefile.am @@ -35,5 +35,6 @@ includesub_HEADERS = \ trace.h \ types.h \ vdso.h \ + seqlock_user.h \ version.h \ xenomai.h diff --git a/include/nucleus/seqlock_user.h b/include/nucleus/seqlock_user.h new file mode 100644 index 000..598d4da --- /dev/null +++ b/include/nucleus/seqlock_user.h @@ -0,0 +1,57 @@ +#ifndef __SEQLOCK_USER_H +#define __SEQLOCK_USER_H + +/* Originally from the linux kernel, adapted for userland and Xenomai */ + +#include asm/xenomai/atomic.h + +typedef struct seqcount { + unsigned sequence; +} seqcount_t; + +#define SEQCNT_ZERO { 0 } +#define seqcount_init(x) do { *(x) = (seqcount_t) SEQCNT_ZERO; } while (0) + +/* Start of read using pointer to a sequence counter only. */ +static inline unsigned read_seqcount_begin(const seqcount_t *s) +{ + unsigned ret; + +repeat: + ret = s-sequence; + xnarch_read_memory_barrier(); + if (unlikely(ret 1)) { + cpu_relax(); + goto repeat; + } + return ret; +} + +/* + * Test if reader processed invalid data because sequence number has changed. + */ +static inline int read_seqcount_retry(const seqcount_t *s, unsigned start) +{ + xnarch_read_memory_barrier(); + + return s-sequence != start; +} + + +/* + * The sequence counter only protects readers from concurrent writers. + * Writers must use their own locking. + */ +static inline void write_seqcount_begin(seqcount_t *s) +{ + s-sequence++; + xnarch_write_memory_barrier(); +} + +static inline void write_seqcount_end(seqcount_t *s) +{ + xnarch_write_memory_barrier(); + s-sequence++; +} + +#endif diff --git a/src/skins/posix/clock.c b/src/skins/posix/clock.c index e27a2ec..19d6c07 100644 --- a/src/skins/posix/clock.c +++ b/src/skins/posix/clock.c @@ -25,6 +25,9 @@ #include time.h #include asm/xenomai/arith.h #include asm-generic/xenomai/timeconv.h +#include nucleus/seqlock_user.h +#include sys/types.h +#include nucleus/vdso.h extern int __pse51_muxid; @@ -56,6 +59,63 @@ int __wrap_clock_getres(clockid_t clock_id, struct timespec *tp) return -1; } +int __do_clock_host_realtime(struct timespec *ts, void *tzp) +{ + int err; +#ifdef XNARCH_HAVE_NONPRIV_TSC + unsigned int seq; + cycle_t now, base, mask, cycle_delta; + unsigned long mult, shift, nsec, rem; + struct xnarch_hostrt_data *hostrt_data; + + if (!xnvdso_test_feature(XNVDSO_FEAT_HOST_REALTIME)) + return -1; + + hostrt_data = nkvdso-hostrt_data; + + if (unlikely(!hostrt_data-live)) + return -1; + + /* +* The following is essentially a verbatim copy of the +* mechanism in the kernel. +*/ +retry: + seq = read_seqcount_begin(hostrt_data-seqcount); + + now = __xn_rdtsc(); + base = hostrt_data-cycle_last; + mask = hostrt_data-mask; + mult = hostrt_data-mult; + shift = hostrt_data-shift; + ts-tv_sec = hostrt_data-wall_time_sec; + nsec = hostrt_data-wall_time_nsec; + + /* If the data changed during the read, try the + alternative data element */ + if (read_seqcount_retry(hostrt_data-seqcount, seq)) + goto retry; + + cycle_delta = (now - base) mask; + nsec += (cycle_delta * mult) shift; + + ts-tv_sec += xnarch_divrem_billion(nsec, rem); + ts-tv_nsec = rem; + + return 0; +#else /* XNARCH_HAVE_NONPRIV_TSC */ + err = -XENOMAI_SKINCALL2(__pse51_muxid, +__pse51_clock_gettime, +CLOCK_HOST_REALTIME, ts); + + if (!err) + return 0; + + errno = err; + return -1; +#endif /* XNARCH_HAVE_NONPRIV_TSC */ +} + int __wrap_clock_gettime(clockid_t clock_id, struct timespec *tp) { int err; @@ -68,7 +128,11 @@ int __wrap_clock_gettime(clockid_t clock_id, struct timespec *tp) tp-tv_sec
[Xenomai-core] [PATCH 7/7] posix: Add some example code for CLOCK_HOST_REALTIME
... and for reading the contents of the hostrt data. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com --- examples/posix/Makefile |3 +- examples/posix/show_hrtime.c | 82 ++ 2 files changed, 84 insertions(+), 1 deletions(-) create mode 100644 examples/posix/show_hrtime.c diff --git a/examples/posix/Makefile b/examples/posix/Makefile index 1bf46a4..c5ec8cb 100644 --- a/examples/posix/Makefile +++ b/examples/posix/Makefile @@ -1,7 +1,7 @@ ## CONFIGURATION ## ### List of applications to be build -APPLICATIONS = satch +APPLICATIONS = satch show_hrtime ### Note: to override the search path for the xeno-config script, use make XENO=... @@ -14,6 +14,7 @@ MODULES = satch all:: satch: satch.c +show_hrtime: show_hrtime.c ## USER SPACE BUILD (no change required normally) ## diff --git a/examples/posix/show_hrtime.c b/examples/posix/show_hrtime.c new file mode 100644 index 000..932c9f8 --- /dev/null +++ b/examples/posix/show_hrtime.c @@ -0,0 +1,82 @@ +/* + * Dump CLOCK_HOST_REALTIME data in the vdso page + * Written by Wolfgang Mauerer wolfgang.maue...@siemens.com + */ + +#include stdio.h +#include sys/mman.h +#include nucleus/vdso.h +#include nucleus/types.h +#include nucleus/seqlock_user.h +#include native/task.h + +extern unsigned long xeno_sem_heap[2]; +static unsigned modeswitches = 0; + +void count_modeswitches(int sig __attribute__((unused))) +{ + modeswitches++; +} + +int main(int argc, char **argv) +{ + int res; + struct timespec ts1, ts2; + + mlockall(MCL_CURRENT|MCL_FUTURE); + + if (!xeno_sem_heap[1]) { + fprintf(stderr, Could not determine position of the + global semaphore heap\n); + return 1; + } + + if (!xnvdso_test_feature(XNVDSO_FEAT_HOST_REALTIME)) { + printf(XNVDSO_FEAT_HOST_REALTIME not available\n); + return 1; + } + + if (nkvdso-hostrt_data.live) + printf(hostrt data area is live\n); + else { + printf(hostrt data area is not live\n); + return 2; + } + + printf(Sequence counter : %u\n, + nkvdso-hostrt_data.seqcount.sequence); + printf(wall_time_sec: %ld\n, nkvdso-hostrt_data.wall_time_sec); + printf(wall_time_nsec : %u\n, nkvdso-hostrt_data.wall_time_nsec); + printf(wall_to_monotonic\n); + printf( tv_sec : %jd\n, + (intmax_t)nkvdso-hostrt_data.wall_to_monotonic.tv_sec); + printf( tv_nsec : %ld\n, + nkvdso-hostrt_data.wall_to_monotonic.tv_nsec); + printf(cycle_last : %lu\n, nkvdso-hostrt_data.cycle_last); + printf(mask : 0x%lx\n, nkvdso-hostrt_data.mask); + printf(mult : %u\n, nkvdso-hostrt_data.mult); + printf(shift: %u\n\n, nkvdso-hostrt_data.shift); + + res = clock_gettime(CLOCK_REALTIME, ts1); + if (res) + printf(clock_gettime(CLOCK_REALTIME) failed!\n); + + signal(SIGXCPU, count_modeswitches); + rt_task_set_mode(0, T_WARNSW, NULL); + modeswitches = 0; + res = clock_gettime(CLOCK_HOST_REALTIME, ts2); + if (res) + printf(clock_gettime(CLOCK_HOST_REALTIME) failed!\n); + + if (modeswitches == 1) { + printf(CLOCK_HOST_REALTIME caused a mode switch.\n); + return 3; + } + + rt_task_set_mode(T_PRIMARY, 0, NULL); + printf(CLOCK_REALTIME : tv_sec = %jd, tv_nsec = %ld\n, + ts1.tv_sec, ts1.tv_nsec); + printf(CLOCK_HOST_REALTIME: tv_sec = %jd, tv_nsec = %ld\n, + ts2.tv_sec, ts2.tv_nsec); + return 0; +} -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH (7+1)/7] posix: Add some example code for CLOCK_HOST_REALTIME
Kiszka, Jan wrote: Wolfgang Mauerer wrote: ... and for reading the contents of the hostrt data. Just realized: Please also update clocktest. Should already work with the new clock ID, but requires a cosmetic output patch. easy enough -- here you go ;-) (I'll integrate this into the series) Best, Wolfgang diff --git a/src/testsuite/clocktest/clocktest.c b/src/testsuite/clocktest/clocktest.c index f0a29ba..42f3f0b 100644 --- a/src/testsuite/clocktest/clocktest.c +++ b/src/testsuite/clocktest/clocktest.c @@ -234,6 +234,10 @@ int main(int argc, char *argv[]) printf(CLOCK_MONOTONIC); break; +case CLOCK_HOST_REALTIME: +printf(CLOCK_HOST_REALTIME); +break; + default: printf(unknown); break; ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 7/7] posix: Add some example code for CLOCK_HOST_REALTIME
Gilles Chanteperdrix wrote: Jan Kiszka wrote: Wolfgang Mauerer wrote: ... and for reading the contents of the hostrt data. Just realized: Please also update clocktest. Should already work with the new clock ID, but requires a cosmetic output patch. Yes, I thought about that. Maybe even if clocktest uses the new clock_id, we could drop the example? I do not think it is a good idea to provide as an example code which uses the internals of the implementation (though it makes sense to use the implementation internals in the clocktest code). And if we remove that, only clock_gettime(CLOCK_HOST_REALTIME) remains, which does not seem to need an example. I would not keep it as an example, but as a test case for checking that nothing is wrong with the data on the shared page. Which does not fit quite well into clocktest. Although I have no problem with just omitting the stuff if reading the (internal) content of the data page is deemed inappropriate. Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/7] Host realtime clock support
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: (This is the Xenomai part of the mechanism, please see the ipipe mailing list for the patches that provide the required basis infrastructure) This patch series extends Xenomai with a new clock, CLOCK_HOST_REALTIME. It allows for sharing NTP-corrected real time timestamps between Linux/ipipe and Xenomai. The data are also available in userspace and can be read without switching to kernel mode. Notice, however, that the new clock only enables to read to time, but cannot serve as a full time basis. Some changes to the ipipe layer are required as basis. In contrast to the initial approach, we don't use a transactional mechanism to copy the information over from Linux, but use classical synchronisation. The code can be compiled in conditionally for both, ipipe and Xenomai. When disabled by architectures that don't support apt clock sources, there is no runtime-overhead associated with the feature. Some points that may require further discussion: - POSIX only specifies a few clock_ids, and these have already been extended by the Linux kernel. We use the maximum id (16) for the new clock, but it might also make sense to use 7 (CLOCK_MONOTONIC_COARSE+1) or 4 (CLOCK_THREAD_CPUTIME_ID+1). - The current implementation deals with x86_64's TSC. Support for other architectures can be added. Additionally, the user has to make sure that the TSC clock source remains active once selected. To implement deactivation (e.g., when the Linux clock source is changed), more ipipe hooks would be required, though. There are two alternatives including other architectures: * We can create a new clocksource that abstracts the per-architecture differences, and use this clocksource as basis for Xenomai. Essentially, this means mapping all desired non-x86-Clocksources to the interface offered in this patch. This requires more changes in the ipipe layer than variant B, namely, * We can create a union in struct xnvdso of all arch-specific clock datasets and introduce feature flags like XNVDSO_FEAT_HOSTRT_X86, XNVDSO_FEAT_HOSTRT_WHATEVER. The reader-side code then needs to match the data provided, which requires more changes on the Xenomai side. The dataset is the same on all architectures, since we provide the same clocksource abstraction on the 5 architectures we support: a TSC emulation. So, a simple approach is simply cut-and-paste the x86 update_vsyscall code for other architectures, another approach is to put this code in a wrapper which we call on all architectures. this disables the possibility of using non-TSC time sources that can also be accessed from userland. That's certainly not a requirement for us, I'd just like to mention it. We also need to ensure that the host has not switched away from TSC because in this case, the NTP correction values delivered from Linux are for a different clocksource than the one used by Xenomai. So we need to detect a switch from TSC to non-TSC, which can only be done in arch-specific code -- I don't see a generic way to know which clocksources are based on the TSC and which one are not. This update_vsyscall code would call ipipe_dispatch_event to pass the data to Xenomai. okay. That seems to have dropped from my radar, as I don't recall having any issues with this requirement. Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/7] Host realtime clock support
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: This update_vsyscall code would call ipipe_dispatch_event to pass the data to Xenomai. okay. That seems to have dropped from my radar, as I don't recall having any issues with this requirement. I am sure that I always thought that it should be done this way. But maybe I did not say it... Sorry if that is the case. you did say it, in fact. I looked at the interface, but seemingly forgot to make this change. Will follow in the next submission. Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 5/7] posix: Support reading the host realtime clock in realtime context
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Wall time management is typically assisted by the NTP protocol in the Linux context, but this information is not propagated to Xenomai. This patch adds support for a CLOCK_HOST_REALTIME clock id that is coupled to the host operating system's realtime clock. The required information from the Kernel into Xenomai. The data exchange is designed to allow for lockless reading from userland. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/posix/time.h |2 + ksrc/skins/posix/clock.c | 90 +- 2 files changed, 91 insertions(+), 1 deletions(-) diff --git a/include/posix/time.h b/include/posix/time.h index 4f2d760..938feb6 100644 --- a/include/posix/time.h +++ b/include/posix/time.h @@ -50,6 +50,8 @@ #define CLOCK_MONOTONIC 1 #endif /* CLOCK_MONOTONIC */ +#define CLOCK_HOST_REALTIME 16 + #if defined(__KERNEL__) || defined(__XENO_SIM__) struct sigevent; diff --git a/ksrc/skins/posix/clock.c b/ksrc/skins/posix/clock.c index 553e123..f5a789b 100644 --- a/ksrc/skins/posix/clock.c +++ b/ksrc/skins/posix/clock.c @@ -50,6 +50,13 @@ *...@{*/ #include posix/thread.h +#include linux/ipipe_tickdev.h +#include linux/math64.h Do we really need this? Because we probably do not have it in 2.4 kernels. Besides, we do not include linux/ headers in Xenomai code. right, math64.h should be xenomai/asm-generic/arith.h for xnarch_divrem_billion(). ipipe_tickdev.h is superfluous since all definitions go through xnarch and rthal. +static int do_clock_host_realtime(struct timespec *tp) +{ +#ifdef CONFIG_XENO_OPT_HOSTRT (...) +return 0; +#else /* CONFIG_XENO_OPT_HOSTRT */ +return -1; return -EINVAL; +case CLOCK_HOST_REALTIME: +if (do_clock_host_realtime(tp) != 0) { rc = do_clock_host_realtime(tp); if (rc 0) { thread_set_errno(-rc); return -1; } ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/7] Host realtime clock support
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Some points that may require further discussion: - POSIX only specifies a few clock_ids, and these have already been extended by the Linux kernel. We use the maximum id (16) for the new clock, but it might also make sense to use 7 (CLOCK_MONOTONIC_COARSE+1) or 4 (CLOCK_THREAD_CPUTIME_ID+1). Why are we limited to 16 clocks? I mean we do not go trough any kernel/glibc path, so it looks like we can use any number. Or did I miss something? To goal was to find a number that's least surprising and blends best with the POSIX conventions and the existing numbers in Linux. Maximum was referring to the definition in the Linux kernel, but I'm happy to pick any larger number, including 23 and 42 ;-) Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 4/7] nucleus: Add CLOCK_HOST_REALTIME bits to nkvdso
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Augment the shared vdso page with all data required to implement a clock CLOCK_HOST_REALTIME that provides time information synchronised with the NTP-corrected time in the Linux kernel. The definition of the hostrt data is placed in a separate head file so that we can use it irregardless of pervasive rt support is compiled in or not. Ok. Would not it be more simple to define the nkvdso even without pervasive support? Thought about that, but having a shared kernel/userland object without pervasive RT support seemed a bit unnatural to me. +#if defined(CONFIG_XENO_OPT_HOSTRT) !defined(CONFIG_XENO_OPT_PERVASIVE) +nkhostrt_data = xnarch_alloc_host_mem(sizeof(struct xnarch_hostrt_data)); should be sizeof(*nkhostrt_data) But why a dynamic allocation at all? not really required, can also make it static if you prefer. Best regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 4/7] nucleus: Add CLOCK_HOST_REALTIME bits to nkvdso
Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Gilles Chanteperdrix wrote: Wolfgang Mauerer wrote: Augment the shared vdso page with all data required to implement a clock CLOCK_HOST_REALTIME that provides time information synchronised with the NTP-corrected time in the Linux kernel. The definition of the hostrt data is placed in a separate head file so that we can use it irregardless of pervasive rt support is compiled in or not. Ok. Would not it be more simple to define the nkvdso even without pervasive support? Thought about that, but having a shared kernel/userland object without pervasive RT support seemed a bit unnatural to me. The idea would be to have a static nkvdso in the non-pervasive case, we do not need the mapping stuff. But at least, if we do this once and for all for nkvdso, we will not have to repeat the code which handles the non-pervasive case for all the data in nkvdso which need to exist in that case. okay, let's do it like this. Might also save a couple of ifdefs. Regards, Wolfgang ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 5/7] posix: Userspace hostrt reading without switching to kernel mode
From: Wolfgang Mauerer (none) wolfg...@dirichlet We can do this since the data are on the shared semaphore heap. The basis data structure is placed so that it is accessible from both the Linux kernel and Xenomai kernel/userland. This also requires to make the structure work with both kernel and userland definitions for elementary data types. We use a seqlock retry mechanism to ensure that we obtain a consistent data set. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/nucleus/Makefile.am|1 + include/nucleus/seqlock_user.h | 57 + src/skins/posix/clock.c| 92 ++- 3 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 include/nucleus/seqlock_user.h diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am index b04d7f6..9a72b24 100644 --- a/include/nucleus/Makefile.am +++ b/include/nucleus/Makefile.am @@ -35,5 +35,6 @@ includesub_HEADERS = \ trace.h \ types.h \ vdso.h \ + seqlock_user.h \ version.h \ xenomai.h diff --git a/include/nucleus/seqlock_user.h b/include/nucleus/seqlock_user.h new file mode 100644 index 000..598d4da --- /dev/null +++ b/include/nucleus/seqlock_user.h @@ -0,0 +1,57 @@ +#ifndef __SEQLOCK_USER_H +#define __SEQLOCK_USER_H + +/* Originally from the linux kernel, adapted for userland and Xenomai */ + +#include asm/xenomai/atomic.h + +typedef struct seqcount { + unsigned sequence; +} seqcount_t; + +#define SEQCNT_ZERO { 0 } +#define seqcount_init(x) do { *(x) = (seqcount_t) SEQCNT_ZERO; } while (0) + +/* Start of read using pointer to a sequence counter only. */ +static inline unsigned read_seqcount_begin(const seqcount_t *s) +{ + unsigned ret; + +repeat: + ret = s-sequence; + xnarch_read_memory_barrier(); + if (unlikely(ret 1)) { + cpu_relax(); + goto repeat; + } + return ret; +} + +/* + * Test if reader processed invalid data because sequence number has changed. + */ +static inline int read_seqcount_retry(const seqcount_t *s, unsigned start) +{ + xnarch_read_memory_barrier(); + + return s-sequence != start; +} + + +/* + * The sequence counter only protects readers from concurrent writers. + * Writers must use their own locking. + */ +static inline void write_seqcount_begin(seqcount_t *s) +{ + s-sequence++; + xnarch_write_memory_barrier(); +} + +static inline void write_seqcount_end(seqcount_t *s) +{ + xnarch_write_memory_barrier(); + s-sequence++; +} + +#endif diff --git a/src/skins/posix/clock.c b/src/skins/posix/clock.c index e27a2ec..14ddf1d 100644 --- a/src/skins/posix/clock.c +++ b/src/skins/posix/clock.c @@ -25,6 +25,9 @@ #include time.h #include asm/xenomai/arith.h #include asm-generic/xenomai/timeconv.h +#include nucleus/seqlock_user.h +#include sys/types.h +#include nucleus/vdso.h extern int __pse51_muxid; @@ -56,25 +59,90 @@ int __wrap_clock_getres(clockid_t clock_id, struct timespec *tp) return -1; } -int __wrap_clock_gettime(clockid_t clock_id, struct timespec *tp) +int __do_clock_host_realtime(struct timespec *ts, void *tzp) { int err; #ifdef XNARCH_HAVE_NONPRIV_TSC - if (clock_id == CLOCK_MONOTONIC __pse51_sysinfo.tickval == 1) { - unsigned long long ns; - unsigned long rem; + unsigned int seq; + cycle_t now, base, mask, cycle_delta; + unsigned long mult, shift, nsec, rem; + struct xnarch_hostrt_data *hostrt_data; + + if (!xnvdso_test_feature(XNVDSO_FEAT_HOST_REALTIME)) + return -1; + + hostrt_data = nkvdso-hostrt_data; + + if (unlikely(!hostrt_data-live)) + return -1; + + /* +* The following is essentially a verbatim copy of the +* mechanism in the kernel. +*/ +retry: + seq = read_seqcount_begin(hostrt_data-seqcount); + + now = __xn_rdtsc(); + base = hostrt_data-cycle_last; + mask = hostrt_data-mask; + mult = hostrt_data-mult; + shift = hostrt_data-shift; + ts-tv_sec = hostrt_data-wall_time_sec; + nsec = hostrt_data-wall_time_nsec; + + /* If the data changed during the read, try the + alternative data element */ + if (read_seqcount_retry(hostrt_data-seqcount, seq)) + goto retry; + + cycle_delta = (now - base) mask; + nsec += (cycle_delta * mult) shift; + + ts-tv_sec += xnarch_divrem_billion(nsec, rem); + ts-tv_nsec = rem; + + return 0; +#else /* XNARCH_HAVE_NONPRIV_TSC */ + err = -XENOMAI_SKINCALL2(__pse51_muxid, +__pse51_clock_gettime, +CLOCK_HOST_REALTIME, ts); - ns = xnarch_tsc_to_ns(__xn_rdtsc()); - tp-tv_sec = xnarch_divrem_billion(ns, rem
[Xenomai-core] [PATCH 1/7] nucleus: Spelling fix for check-vdso.c
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/testsuite/unit/check-vdso.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/testsuite/unit/check-vdso.c b/src/testsuite/unit/check-vdso.c index 8f90420..17684de 100644 --- a/src/testsuite/unit/check-vdso.c +++ b/src/testsuite/unit/check-vdso.c @@ -15,7 +15,7 @@ int main(int argc, char **argv) unsigned long long test_features; if (argc != 2) { - printf(No specific feature(s) given, using XNVDSO_FEATURE\n); + printf(No specific feature(s) given, using XNVDSO_FEATURES\n); test_features = XNVDSO_FEATURES; } else { test_features = strtoull(argv[1], NULL, 0); -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 4/7] posix: Support reading the host realtime clock in realtime context
Wall time management is typically assisted by the NTP protocol in the Linux context, but this information is not propagated to Xenomai. This patch adds support for a CLOCK_HOST_REALTIME clock id that is coupled to the host operating system's realtime clock. The required information from the Kernel into Xenomai. The data exchange is designed to allow for lockless reading from userland. Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/posix/time.h |7 ksrc/skins/posix/clock.c | 86 +- 2 files changed, 92 insertions(+), 1 deletions(-) diff --git a/include/posix/time.h b/include/posix/time.h index 4f2d760..92ec1d5 100644 --- a/include/posix/time.h +++ b/include/posix/time.h @@ -50,6 +50,13 @@ #define CLOCK_MONOTONIC 1 #endif /* CLOCK_MONOTONIC */ +/* + * This number is supposed to not collide with any of the POSIX and + * Linux kernel definitions so that no ambiguities arise when porting + * applications in both directions. + */ +#define CLOCK_HOST_REALTIME 42 + #if defined(__KERNEL__) || defined(__XENO_SIM__) struct sigevent; diff --git a/ksrc/skins/posix/clock.c b/ksrc/skins/posix/clock.c index 553e123..62493f1 100644 --- a/ksrc/skins/posix/clock.c +++ b/ksrc/skins/posix/clock.c @@ -50,6 +50,9 @@ *...@{*/ #include posix/thread.h +#include nucleus/vdso.h +#include asm-generic/xenomai/arith.h +#include asm-generic/xenomai/system.h /** * Get the resolution of the specified clock. @@ -88,6 +91,76 @@ int clock_getres(clockid_t clock_id, struct timespec *res) } /** + * Read the host-synchronised realtime clock. + * + * Obtain the current time with NTP corrections from the Linux domain + * + * @param tp pointer to a struct timespec + * + * @retval 0 on success; + * @retval -1 if no suitable NTP-corrected clocksource is availabel + * + * @see + * a href=http://www.opengroup.org/onlinepubs/95399/functions/gettimeofday.html; + * Specification./a + * + */ +static int do_clock_host_realtime(struct timespec *tp) +{ +#ifdef CONFIG_XENO_OPT_HOSTRT + cycle_t now, base, mask, cycle_delta; + unsigned long mult, shift, nsec, rem; + struct xnarch_hostrt_data *hostrt_data; + unsigned int seq; + + hostrt_data = get_hostrt_data(); + BUG_ON(!hostrt_data); + + if (unlikely(!hostrt_data-live)) + return -1; + + /* +* Note: Disabling HW interrupts around writes to hostrt_data ensures +* that a reader (on the Xenomai side) cannot interrupt a writer (on +* the Linux kernel side) on the same CPU. The sequence counter is +* required when a reader is interleaved by a writer on a different +* CPU. This follows the approach from userland, where tasking the +* spinlock is not possible. +*/ +retry: + seq = read_seqcount_begin(hostrt_data-seqcount); + + now = xnarch_get_cpu_tsc(); + base = hostrt_data-cycle_last; + mask = hostrt_data-mask; + mult = hostrt_data-mult; + shift = hostrt_data-shift; + tp-tv_sec = hostrt_data-wall_time_sec; + nsec = hostrt_data-wall_time_nsec; + + if (read_seqcount_retry(hostrt_data-seqcount, seq)) + goto retry; + + /* +* At this point, we have a consistent copy of the fundamental +* data structure - calculate the interval between the current +* and base time stamp cycles, and convert the difference +* to nanoseconds. +*/ + cycle_delta = (now - base) mask; + nsec += (cycle_delta * mult) shift; + + /* Convert to the desired sec, usec representation */ + tp-tv_sec += xnarch_divrem_billion(nsec, rem); + tp-tv_nsec = rem; + + return 0; +#else /* CONFIG_XENO_OPT_HOSTRT */ + return -EINVAL; +#endif +} + +/** * Read the specified clock. * * This service returns, at the address @a tp the current value of the clock @a @@ -97,8 +170,12 @@ int clock_getres(clockid_t clock_id, struct timespec *res) * - CLOCK_MONOTONIC, the clock value is given by an architecture-dependent high * resolution counter, with a precision independent from the system clock tick * duration. + * - CLOCK_HOST_REALTIME, the clock value as seen by the host, typically + * Linux. Resolution and precision depend on the host, but it is guaranteed + * that both, host and Xenomai, see the same information. * - * @param clock_id clock identifier, either CLOCK_REALTIME or CLOCK_MONOTONIC; + * @param clock_id clock identifier, either CLOCK_REALTIME, CLOCK_MONOTONIC, + *or CLOCK_HOST_REALTIME; * * @param tp the address where the value of the specified clock will be stored. * @@ -126,6 +203,13 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp) xnarch_uldivrem(cpu_time, ONE_BILLION, tp-tv_nsec); break; + case CLOCK_HOST_REALTIME
[Xenomai-core] [PATCH 2/7] nucleus: Add userland cpu_relax() definition for x86
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/asm-x86/atomic.h |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/include/asm-x86/atomic.h b/include/asm-x86/atomic.h index bb3ce46..16bc990 100644 --- a/include/asm-x86/atomic.h +++ b/include/asm-x86/atomic.h @@ -71,6 +71,11 @@ typedef struct { unsigned long counter; } xnarch_atomic_t; #define xnarch_write_memory_barrier() xnarch_memory_barrier() +static inline void cpu_relax(void) +{ + asm volatile(rep; nop ::: memory); +} + #ifdef __i386__ struct __xeno_xchg_dummy { unsigned long a[100]; }; -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH v2 0/7] Host realtime clock support
(This is the Xenomai part of the mechanism, please see the ipipe mailing list for the patches that provide the required basis infrastructure) This patch series extends Xenomai with a new clock, CLOCK_HOST_REALTIME. It allows for sharing NTP-corrected real time timestamps between Linux/ipipe and Xenomai. The data are also available in userspace and can be read without switching to kernel mode. Notice, however, that the new clock only enables to read to time, but cannot serve as a full time basis. Some changes to the ipipe layer are required as basis. Changes since the last submission: - Unconditionally create an nkvdso instance to get rid of splitting the implementation into a pervasive/non-pervasive part - Remove explicit usage example, update clocktest code for the new clock instead - Include some smaller review comments The selection of Linux clocks suitable for our purposes remains as in the last series. Tested on x86_64, compile tested for all combinations of IPIPE_HOSTRT, XENO_OPT_HOSTRT and XENO_OPT_PERVASIVE include/asm-generic/hal.h | 11 + include/asm-generic/system.h |3 include/asm-sim/system.h |3 include/asm-x86/atomic.h |5 include/nucleus/Makefile.am |1 include/nucleus/hostrt.h | 54 ++ include/nucleus/seqlock_user.h| 57 ++ include/nucleus/vdso.h| 20 +- include/posix/time.h |7 ksrc/nucleus/Kconfig |4 ksrc/nucleus/module.c | 64 ++- ksrc/skins/posix/clock.c | 86 +- src/skins/posix/clock.c | 92 +- src/testsuite/clocktest/clocktest.c | 64 ++- src/testsuite/unit/check-vdso.c |2 include/nucleus/Makefile.am |1 src/testsuite/clocktest/clocktest.c | 292 +- 17 files changed, 594 insertions(+), 172 deletions(-) ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 7/7] Update clocktest for CLOCK_HOST_REALTIME
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- src/testsuite/clocktest/clocktest.c | 64 --- 1 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/testsuite/clocktest/clocktest.c b/src/testsuite/clocktest/clocktest.c index 7f29c23..bf4feb6 100644 --- a/src/testsuite/clocktest/clocktest.c +++ b/src/testsuite/clocktest/clocktest.c @@ -28,6 +28,7 @@ #include sys/syscall.h #include sys/mman.h #include sys/time.h +#include nucleus/vdso.h #include xeno_config.h @@ -75,11 +76,49 @@ struct per_cpu_data { pthread_t thread; } *per_cpu_data; +static void show_hostrt_diagnostics(void) +{ + if (!xnvdso_test_feature(XNVDSO_FEAT_HOST_REALTIME)) { + printf(XNVDSO_FEAT_HOST_REALTIME not available\n); + return; + } + + if (nkvdso-hostrt_data.live) + printf(hostrt data area is live\n); + else { + printf(hostrt data area is not live\n); + return; + } + + printf(Sequence counter : %u\n, + nkvdso-hostrt_data.seqcount.sequence); + printf(wall_time_sec: %ld\n, nkvdso-hostrt_data.wall_time_sec); + printf(wall_time_nsec : %u\n, nkvdso-hostrt_data.wall_time_nsec); + printf(wall_to_monotonic\n); + printf( tv_sec : %jd\n, + (intmax_t)nkvdso-hostrt_data.wall_to_monotonic.tv_sec); + printf( tv_nsec : %ld\n, + nkvdso-hostrt_data.wall_to_monotonic.tv_nsec); + printf(cycle_last : %lu\n, nkvdso-hostrt_data.cycle_last); + printf(mask : 0x%lx\n, nkvdso-hostrt_data.mask); + printf(mult : %u\n, nkvdso-hostrt_data.mult); + printf(shift: %u\n\n, nkvdso-hostrt_data.shift); +} + static inline unsigned long long read_clock(clockid_t clock_id) { struct timespec ts; + int res; - clock_gettime(clock_id, ts); + res = clock_gettime(clock_id, ts); + if (res != 0) { + fprintf(stderr, clock_gettime failed for clock id %d\n, + clock_id); + if (clock_id == CLOCK_HOST_REALTIME) + show_hostrt_diagnostics(); + + exit(-1); + } return ts.tv_nsec + ts.tv_sec * 10ULL; } @@ -87,8 +126,10 @@ static inline unsigned long long read_reference_clock(void) { struct timeval tv; - /* Make sure we do not pick the vsyscall variant. It won't - switch us into secondary mode and can easily deadlock. */ + /* +* Make sure we do not pick the vsyscall variant. It won't +* switch us into secondary mode and can easily deadlock. +*/ syscall(SYS_gettimeofday, tv, NULL); return tv.tv_usec * 1000ULL + tv.tv_sec * 10ULL; } @@ -186,8 +227,9 @@ int main(int argc, char *argv[]) int cpus = sysconf(_SC_NPROCESSORS_ONLN); int i; int c; + int d = 0; - while ((c = getopt(argc, argv, C:T:)) != EOF) + while ((c = getopt(argc, argv, C:T:D)) != EOF) switch (c) { case 'C': clock_id = atoi(optarg); @@ -197,10 +239,15 @@ int main(int argc, char *argv[]) alarm(atoi(optarg)); break; + case 'D': + d = 1; + break; + default: fprintf(stderr, usage: clocktest [options]\n [-C clock_id] # tested clock, default=%d (CLOCK_REALTIME)\n - [-T test_duration_seconds] # default=0, so ^C to end\n, + [-T test_duration_seconds] # default=0, so ^C to end\n + [-D] # print extra diagnostics for CLOCK_HOST_REALTIME\n, CLOCK_REALTIME); exit(2); } @@ -211,6 +258,9 @@ int main(int argc, char *argv[]) init_lock(lock); + if (d clock_id == CLOCK_HOST_REALTIME) + show_hostrt_diagnostics(); + per_cpu_data = malloc(sizeof(*per_cpu_data) * cpus); if (!per_cpu_data) { fprintf(stderr, %s\n, strerror(ENOMEM)); @@ -234,6 +284,10 @@ int main(int argc, char *argv[]) printf(CLOCK_MONOTONIC); break; + case CLOCK_HOST_REALTIME: + printf(CLOCK_HOST_REALTIME); + break; + default: printf(unknown); break; -- 1.6.4 ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] [PATCH 6/7] Convert clocktest to Xenomai indentation style
Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com --- src/testsuite/clocktest/clocktest.c | 292 +- 1 files changed, 146 insertions(+), 146 deletions(-) diff --git a/src/testsuite/clocktest/clocktest.c b/src/testsuite/clocktest/clocktest.c index f0a29ba..7f29c23 100644 --- a/src/testsuite/clocktest/clocktest.c +++ b/src/testsuite/clocktest/clocktest.c @@ -66,190 +66,190 @@ unsigned long long last_common = 0; clockid_t clock_id = CLOCK_REALTIME; struct per_cpu_data { -unsigned long long first_tod, first_clock; -int first_round; -long long offset; -double drift; -unsigned long warps; -unsigned long long max_warp; -pthread_t thread; + unsigned long long first_tod, first_clock; + int first_round; + long long offset; + double drift; + unsigned long warps; + unsigned long long max_warp; + pthread_t thread; } *per_cpu_data; static inline unsigned long long read_clock(clockid_t clock_id) { -struct timespec ts; + struct timespec ts; -clock_gettime(clock_id, ts); -return ts.tv_nsec + ts.tv_sec * 10ULL; + clock_gettime(clock_id, ts); + return ts.tv_nsec + ts.tv_sec * 10ULL; } static inline unsigned long long read_reference_clock(void) { -struct timeval tv; + struct timeval tv; -/* Make sure we do not pick the vsyscall variant. It won't - switch us into secondary mode and can easily deadlock. */ -syscall(SYS_gettimeofday, tv, NULL); -return tv.tv_usec * 1000ULL + tv.tv_sec * 10ULL; + /* Make sure we do not pick the vsyscall variant. It won't + switch us into secondary mode and can easily deadlock. */ + syscall(SYS_gettimeofday, tv, NULL); + return tv.tv_usec * 1000ULL + tv.tv_sec * 10ULL; } void check_reference(struct per_cpu_data *per_cpu_data) { -unsigned long long clock_val[10], tod_val[10]; -long long delta, min_delta; -int i, idx; - -for (i = 0; i 10; i++) { -tod_val[i] = read_reference_clock(); -clock_val[i] = read_clock(clock_id); -} - -min_delta = tod_val[1] - tod_val[0]; -idx = 1; - -for (i = 2; i 10; i++) { -delta = tod_val[i] - tod_val[i-1]; -if (delta min_delta) { -min_delta = delta; -idx = i; -} -} - -if (per_cpu_data-first_round) { -per_cpu_data-first_round = 0; - -per_cpu_data-first_tod = tod_val[idx]; -per_cpu_data-first_clock = clock_val[idx]; -} else -per_cpu_data-drift = -(clock_val[idx] - per_cpu_data-first_clock) / -(double)(tod_val[idx] - per_cpu_data-first_tod) - 1; - -per_cpu_data-offset = clock_val[idx] - tod_val[idx]; + unsigned long long clock_val[10], tod_val[10]; + long long delta, min_delta; + int i, idx; + + for (i = 0; i 10; i++) { + tod_val[i] = read_reference_clock(); + clock_val[i] = read_clock(clock_id); + } + + min_delta = tod_val[1] - tod_val[0]; + idx = 1; + + for (i = 2; i 10; i++) { + delta = tod_val[i] - tod_val[i-1]; + if (delta min_delta) { + min_delta = delta; + idx = i; + } + } + + if (per_cpu_data-first_round) { + per_cpu_data-first_round = 0; + + per_cpu_data-first_tod = tod_val[idx]; + per_cpu_data-first_clock = clock_val[idx]; + } else + per_cpu_data-drift = + (clock_val[idx] - per_cpu_data-first_clock) / + (double)(tod_val[idx] - per_cpu_data-first_tod) - 1; + + per_cpu_data-offset = clock_val[idx] - tod_val[idx]; } void check_time_warps(struct per_cpu_data *per_cpu_data) { -int i; -unsigned long long last, now; -long long incr; - -for (i = 0; i 100; i++) { -acquire_lock(lock); -now = read_clock(clock_id); -last = last_common; -last_common = now; -release_lock(lock); - -incr = now - last; -if (incr 0) { -acquire_lock(lock); -per_cpu_data-warps++; -if (-incr per_cpu_data-max_warp) -per_cpu_data-max_warp = -incr; -release_lock(lock); -} -} + int i; + unsigned long long last, now; + long long incr; + + for (i = 0; i 100; i++) { + acquire_lock(lock); + now = read_clock(clock_id); + last = last_common; + last_common = now; + release_lock(lock); + + incr = now - last; + if (incr 0) { + acquire_lock(lock); + per_cpu_data-warps++; + if (-incr per_cpu_data-max_warp) + per_cpu_data-max_warp = -incr
Re: [Xenomai-core] [announce] Xenomai v2.5.4
Gilles Chanteperdrix wrote: Hi, you will find Xenomai 2.5.4, aka Sleep Walk, at the usual place: http://download.gna.org/xenomai/stable/xenomai-2.5.4.tar.bz2 It contains the usual amount of bug fixes and I-pipe patches update, as well as a new piece of nice code: the so-called mayday support, which should allow Xenomai watchdog to work less brutally (namely, avoid killing the tasks occupying too much CPU, and send them a signal), and be the base for a new version of user-space signals in the 2.6 branch. Alex also pushed a lot of improvements for the analogy support. So, it seems not everybody has been lazy like me for this last version. to satisfy my curiosity: What was the exact reason for not including the hostrt stuff? After having spent quite some (obviously wasted) effort to make the code compliant with the maintainer's requests, I wonder what could have been missing. Wolfgang The complete shortlog follows: Alexis Berlemont (48): analogy: change the context's role (broken) analogy: the buffer structure is now the central field of a4l_context (bro ken) analogy: the subdevice structure got a new status field (broken) analogy: the transfer structure is left with a minimal role (broken) analogy: first draft of buffer initialization functions (broken) analogy: adapt open, r/w, select and ioctl functions (broken) analogy: adapt a4l_set_dev() after a4l_context's overhaul (broken) analogy: update a4l_set_dev() declaration (broken) analogy: update comments on a4l_context (broken) analogy: changes related with subdevice's status field (broken) analogy: replace transfer setup functions with buffer setup ones (broken) analogy: update cancel functions (broken) analogy: rewrite the cancel ioctl handler (broken) analogy: fix bulk flag declaration in buffer.h (broken) analogy: update a4l_read and a4l_write (broken) analogy: update all a4l_buf_* functions (broken) analogy: last updates in the buffer part (broken) analogy: cosmetic changes (broken) analogy: declare the reserve / release functions at the subd level (broken ) analogy: update a4l_get_minor function (broken) analogy: update a4l_set_dev and remove useless info traces (broken) analogy: use rtdm_context_to_private (broken) analogy: minor fix in the subdevice structure declaration analogy: add some helper macros to test the subdevice's characteristics analogy: remove useless functions in the subdevice part analogy: fix the buffer syscalls (ioctl + r/w) after buffer review (broken) analogy: fix the declaration of the structure a4l_context (broken) analogy: fix compilation issues and review the mmap ioctl handler (broken) analogy: cosmetic change (broken) analogy: fix buffer's compilation issues (broken) analogy: prettify some subdevice tests (broken) analogy: [pcimio] fix a huge hack in the mite initialization (broken) analogy: fix the last compilation problems analogy: fix a missing setting of the buf field in subdevice (broken) analogy: fix the subdevice status management analogy: fix buffer initialization/cleanup calls at open/close times analogy: [loop] add a debug trace when trigger is called analogy: fix test of subdevice status in a4l_write analogy: [fake - loop] remove volatile keywords analogy: add a detail in a4l_close doxygen doc analogy: add an arbitrary sleep in cmd_write before closing the device analogy: [ni_pcimio] really minor changes analogy: [ni_pcimio] add the missing allocation of the digital ring analogy: [ni_pcimio] fix timeout value in digital trigger analogy: remove a4l_subd_is_busy calls in analogy core analogy: remove calls of a4l_release/reserve_subd in the core analogy: remove some tests which become with the buffer overhaul analogy: fix a bug in a4l_fill_desc() when called on an idle device Gilles Chanteperdrix (18): arm: fix VFP handling in the SMP case arm: get the nodiv_llimd code to compile in thumb mode Merge commit 'rpm' into pending Merge branch 'pending' rtcan: add missing PCI IDs for old kernels Merge commit 'rpm/for-upstream' into build-test Update autotools files arm: clarify the patches README with regard to vendor-specifi branches Merge commit 'analogy' into pending testsuite: adapt run scripts to the --with-testdir option native: add cancellation points arm: upgrade adeos patches to 2.6.30-1.15-02, 2.6.31-1.16-02, 2.6.33-1.17- 02 Add IMX51 patch build: bootstrap compat: add missing PCI ID for 2.4 kernels sched: avoid infinite reschedule loops build: bump version number doc: regenerate Pavel Cheblakov (1):
Re: [Xenomai-core] hostrt broken on ARM.
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. - 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. Here is the patch. diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am index 9a72b24..5fc1c21 100644 diff --git a/include/nucleus/Makefile.in b/include/nucleus/Makefile.in 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. Thanks, Wolfgang -struct xnarch_hostrt_data { +struct xnvdso_hostrt_data { short live; - seqcount_t seqcount; + xnseqcount_t seqcount; time_t wall_time_sec; - u32 wall_time_nsec; + unsigned wall_time_nsec; struct timespec wall_to_monotonic; - cycle_t cycle_last; - cycle_t mask; - u32 mult; - u32 shift; + unsigned long long cycle_last; + unsigned long long mask; + unsigned mult; + unsigned shift; }; -#endif /* !__KERNEL__ */ #endif diff --git a/include/nucleus/seqlock.h b/include/nucleus/seqlock.h new file mode 100644 index 000..897d411 --- /dev/null +++ b/include/nucleus/seqlock.h @@ -0,0 +1,57 @@ +#ifndef __SEQLOCK_H +#define __SEQLOCK_H + +/* Originally from the linux kernel, adapted for userland and Xenomai */ + +#include asm/xenomai/atomic.h + +typedef struct xnseqcount { + unsigned sequence; +} xnseqcount_t; + +#define XNSEQCNT_ZERO { 0 } +#define xnseqcount_init(x) do { *(x) = (xnseqcount_t) SEQCNT_ZERO; } while (0) + +/* Start of read using pointer to a sequence counter only. */ +static inline unsigned xnread_seqcount_begin(const xnseqcount_t *s) +{ + unsigned ret; + +repeat: + ret = s-sequence; + xnarch_read_memory_barrier(); + if (unlikely(ret 1)) { + cpu_relax(); + goto repeat; + } + return ret; +} + +/* + * Test if reader processed invalid data because sequence number has changed. + */ +static inline int xnread_seqcount_retry(const xnseqcount_t *s, unsigned start) +{ + xnarch_read_memory_barrier(); + + return s-sequence != start; +} + + +/* + * The sequence counter only protects readers from concurrent writers. + * Writers must use their own locking. + */ +static inline void xnwrite_seqcount_begin(xnseqcount_t *s) +{ + s-sequence++; +
Re: [Xenomai-core] hostrt broken on ARM.
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, Wolfgang Here is the patch. diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am index 9a72b24..5fc1c21 100644 diff --git a/include/nucleus/Makefile.in b/include/nucleus/Makefile.in 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 Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core