John,
Am 06.03.2014 um 04:43 schrieb John Kasunich <jmkasun...@fastmail.fm>:
>
>
> 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.
thanks for going through this. I think we have reached clarification of a
not-so-well understood corner of HAL which was unclear to some (well at least
me).
Can I summarize what I read from the above as follows:
- there are two pieces of state (addval and deltalim) passed from servo to base
in stepgen.c
- for correctness of stepgen operation it is not essential these values origin
from the same invocation of update-freq, meaning make-pulses is not a critical
region with respect to access of these values
- as a consequence, the threads executing update-freq and make-pulses are not
subject to any priority and non-preemption preconditions
does this reflect our common understanding?
- Michael
------------------------------------------------------------------------------
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
