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