Re: [Xenomai-core] [PATCH 0/3] NMI watchdog fixes / enhancements
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > This is basically a repost of the NNI watchdog series I sent out a few > weeks ago. I just rebased things over latest trunk and fixed some > warnings. > > All patches are also available at > git://git.kiszka.org/xenomai.git nmi-wd-queue That is a lot of stuff to review. I am afraid it is impossible to review everything, so the only thing we can rely on is testing, hence the next question: have these patches been tested in every configuration (enabled, disabled, built-in, module, voluntary overrun)? >>> In most configurations, but definitely not in all (they are too many). >>> >>> This is a debugging tool, so first of all the disabled case must not >>> cause harm, and I'm quite sure I haven't changed anything regarding >>> this. Moreover, the enabled case was not working for many recent >>> platforms anymore as we were lacking P6 support. So there shouldn't be >>> much to loose. >> I disagree: the current version compiles in all configurations tested >> until now, and happened to work when enabled at some point in the past. >> I find it annoying, to say the least, when I want to test something on >> trunk, that some previous unrelated commit breaks a configuration >> because it was not tested. So, please test your patch. The >> configurations to test are not so numerous: disabled, enabled built-in >> with an overrun check, enabled in module with an overrun check, repeat >> for x86_64. That makes 6 configurations, not that much. > > The trivial ones have been tested, of course. But keep in mind that > there are more variables (CPU types , kernel versions, interfering > config settings, etc.) that create much more that 6 variants to be built > and run. To give an example of untested scenarios: Current trunk does not build against 2.6.27 for x86-32 with NMI watchdog enabled (die_nmi gained another argument doe to unification with x86-64). Well, my patch does not change this picture yet (I once tested against 2.6.26 for 32-bit support). Will fix this as well. Jan signature.asc Description: OpenPGP digital signature ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/3] NMI watchdog fixes / enhancements
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: This is basically a repost of the NNI watchdog series I sent out a few weeks ago. I just rebased things over latest trunk and fixed some warnings. All patches are also available at git://git.kiszka.org/xenomai.git nmi-wd-queue >>> That is a lot of stuff to review. I am afraid it is impossible to review >>> everything, so the only thing we can rely on is testing, hence the next >>> question: have these patches been tested in every configuration >>> (enabled, disabled, built-in, module, voluntary overrun)? >> In most configurations, but definitely not in all (they are too many). >> >> This is a debugging tool, so first of all the disabled case must not >> cause harm, and I'm quite sure I haven't changed anything regarding >> this. Moreover, the enabled case was not working for many recent >> platforms anymore as we were lacking P6 support. So there shouldn't be >> much to loose. > > I disagree: the current version compiles in all configurations tested > until now, and happened to work when enabled at some point in the past. > I find it annoying, to say the least, when I want to test something on > trunk, that some previous unrelated commit breaks a configuration > because it was not tested. So, please test your patch. The > configurations to test are not so numerous: disabled, enabled built-in > with an overrun check, enabled in module with an overrun check, repeat > for x86_64. That makes 6 configurations, not that much. The trivial ones have been tested, of course. But keep in mind that there are more variables (CPU types , kernel versions, interfering config settings, etc.) that create much more that 6 variants to be built and run. Jan signature.asc Description: OpenPGP digital signature ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/3] NMI watchdog fixes / enhancements
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> This is basically a repost of the NNI watchdog series I sent out a few >>> weeks ago. I just rebased things over latest trunk and fixed some >>> warnings. >>> >>> All patches are also available at >>> git://git.kiszka.org/xenomai.git nmi-wd-queue >> That is a lot of stuff to review. I am afraid it is impossible to review >> everything, so the only thing we can rely on is testing, hence the next >> question: have these patches been tested in every configuration >> (enabled, disabled, built-in, module, voluntary overrun)? > > In most configurations, but definitely not in all (they are too many). > > This is a debugging tool, so first of all the disabled case must not > cause harm, and I'm quite sure I haven't changed anything regarding > this. Moreover, the enabled case was not working for many recent > platforms anymore as we were lacking P6 support. So there shouldn't be > much to loose. I disagree: the current version compiles in all configurations tested until now, and happened to work when enabled at some point in the past. I find it annoying, to say the least, when I want to test something on trunk, that some previous unrelated commit breaks a configuration because it was not tested. So, please test your patch. The configurations to test are not so numerous: disabled, enabled built-in with an overrun check, enabled in module with an overrun check, repeat for x86_64. That makes 6 configurations, not that much. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/3] NMI watchdog fixes / enhancements
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> This is basically a repost of the NNI watchdog series I sent out a few >> weeks ago. I just rebased things over latest trunk and fixed some >> warnings. >> >> All patches are also available at >> git://git.kiszka.org/xenomai.git nmi-wd-queue > > That is a lot of stuff to review. I am afraid it is impossible to review > everything, so the only thing we can rely on is testing, hence the next > question: have these patches been tested in every configuration > (enabled, disabled, built-in, module, voluntary overrun)? In most configurations, but definitely not in all (they are too many). This is a debugging tool, so first of all the disabled case must not cause harm, and I'm quite sure I haven't changed anything regarding this. Moreover, the enabled case was not working for many recent platforms anymore as we were lacking P6 support. So there shouldn't be much to loose. > > As for the 32nd bit issue, I am afraid it can not explain the spurious > shots observed on some platfomrs (note that I implemented the early shot > thing a bit in the dark: I never observed the spurious shots myself), > that is because the nmi timer is programmed for durations between 100us > and 10ms, which should be far from using the 32nd bit. Yes, the signedness issue that is fixed in patch 1 has likely nothing to do with the spurious invocations. But wherever they may still come from, patch 3 ensures that they are now properly ignored (ie. forwarded to the next handling instance). Jan signature.asc Description: OpenPGP digital signature ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH 0/3] NMI watchdog fixes / enhancements
Jan Kiszka wrote: > This is basically a repost of the NNI watchdog series I sent out a few > weeks ago. I just rebased things over latest trunk and fixed some > warnings. > > All patches are also available at > git://git.kiszka.org/xenomai.git nmi-wd-queue That is a lot of stuff to review. I am afraid it is impossible to review everything, so the only thing we can rely on is testing, hence the next question: have these patches been tested in every configuration (enabled, disabled, built-in, module, voluntary overrun)? As for the 32nd bit issue, I am afraid it can not explain the spurious shots observed on some platfomrs (note that I implemented the early shot thing a bit in the dark: I never observed the spurious shots myself), that is because the nmi timer is programmed for durations between 100us and 10ms, which should be far from using the 32nd bit. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
