On Sun, Mar 2, 2014, at 02:29 PM, Michael Haberler wrote:
>
> I have read up on the data flow in stepgen.c and I found the following for
> normal (i.e. not disabled, and setup disregarded):
>
> make_pulses reads:
> stepgen->accum
> stepgen->target_addval
> stepgen->deltalim
>
> make_pulses writes:
> stepgen->accum
>
> update_freq reads:
> stepgen->accum
>
> update_freq writes:
> stepgen->target_addval
> stepgen->deltalim
>
> Now stepgen->accum can be taken out of the picture for considering
> transactions since that is the single scalar value read back by update_freq
> (it could be replaced by an atomic 64bit fetch which is supported on all
> architectures, but that is a side issue).
>
> The interesting case are the two values which update_freq writes. I assume
> you are referring to those?
>
> I can see how scheduling comes in here the way this is coded, but I think
> this is basically a complicated way to achieve an atomic message transmit
> from update_freq to make_pulses.
>
Actually, I don't think there is any need to worry about atomic updates of
target_addval and deltalim.
The relevant code is this (from make_pulses, in the faster thread):
/* update addval (ramping) */
old_addval = stepgen->addval;
target_addval = stepgen->target_addval;
if (stepgen->deltalim != 0) {
/* implement accel/decel limit */
if (target_addval > (old_addval + stepgen->deltalim)) {
/* new value is too high, increase addval as far as
possible */
new_addval = old_addval + stepgen->deltalim;
} else if (target_addval < (old_addval - stepgen->deltalim)) {
/* new value is too low, decrease addval as far as possible
*/
new_addval = old_addval - stepgen->deltalim;
} else {
/* new value can be reached in one step - do it */
new_addval = target_addval;
}
} else {
/* go to new freq without any ramping */
new_addval = target_addval;
}
/* save result */
stepgen->addval = new_addval;
"addval" is the value that is added to the accumulator every base period. In
other
words, it is the instantaneous velocity.
"deltalim" is the maximum allowed change in addval per base period. It is based
on the maximum allowable acceleration parameter.
In normal LCNC configurations, the acceleration is set at startup and never
changes. But the stepgen was designed to be more versatile than that, and
it needs to work correctly even if the acceleration limit is changed on the fly.
But there is no need for an acceleration change to be atomic with the velocity
command.
There is a bit of sloppiness in this code. Deltalim isn't treated with
the same care as addval. You'll notice that stepgen->addval and
stepgen->target_addval are copied from the structure to local
variables at the beginning of this code snipped, and then the new
value of addval is copied back to the structure at the end.
stepgen->target_addval is modified by the slow thread, and if
we allow multi-core, it is subject to being changed at any time.
If it changes before it is copied to the local target_adval, the
change will take effect this time, if not, it will take effect the
next time. The value is read only once.
Now look at stepgen->deltalim. It is also written by the slow
thread, and if we allow multi-core, it can change at any time.
The code snippet above has five references to stepgen->deltalim.
It would be possible for the value to change after the first
conditional "if (stepgen->deltalim != 0) {", but before the second
one "if (target_addval > (old_addval + stepgen->deltalim)) ", or
after the second conditional but before the line that uses the
value: "new_addval = old_addval + stepgen->deltalim;"
When that code was written, the assumption was rate monotonic
and single core. SMP was around at that time, but RTAPI limited
all realtime threads to a single core. Under those assumptions,
the code is fine. The low speed thread can't sneak in and
change the value at an unexpected time.
But under the new assumption that threads can be running on
different cores, the slow thread might be running (and changing
stepgen->deltalim) at the same time as the fast thread is running.
The answer is to treat deltalim like addval. Read stegen->deltalim
a single time, at the start of the code, and copy it to a local
variable. Then the five references to deltalim can use the local.
Any change made to stepgen->deltalim by the slow thread will
either be before the single read, and take effect this time, or
after the read, and take effect next time. There is no risk of
using one value during a comparison and another value during
the subsequent assignment, because the slow thread cannot
change the local variable.
I wrote this code, and to be honest I don't know why I treated
addval with more care than deltalim. Under the assumptions
of that time, both were safe from modification by the slow
thread, and neither one needed to be copied to a local. Under
the new assumptions, both are vulnerable, and both need to
be copied to locals.
Because velocity and acceleration limits are independent
attributes, there is no need to worry about synchronization
of the two values to each other.
--
John Kasunich
jmkasun...@fastmail.fm
------------------------------------------------------------------------------
Subversion Kills Productivity. Get off Subversion & Make the Move to Perforce.
With Perforce, you get hassle-free workflows. Merge that actually works.
Faster operations. Version large binaries. Built-in WAN optimization and the
freedom to use Git, Perforce or both. Make the move to Perforce.
http://pubads.g.doubleclick.net/gampad/clk?id=122218951&iu=/4140/ostg.clktrk
_______________________________________________
Emc-developers mailing list
Emc-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/emc-developers
