On 02/04/2013 08:43 AM, Michael Haberler wrote:
> Am 04.02.2013 um 14:11 schrieb EBo:
>> On Feb 4 2013 4:18 AM, Michael Haberler wrote:
>>> Am 04.02.2013 um 11:29 schrieb EBo:
>>>> In the past, and I can read it between the lines now, that the
>>>> invasiveness of the modifications will take so much time and
>>>> effort that lots of other things gets pushed back.
>>> 
>>> I dont agree on 'invasiveness'. In fact the RTAPI api is used 
>>> basically unchanged, barring some new functions for PCI handling;
>>> the codebase is much cleaner and easier to maintain I would
>>> think. I am not aware of any outstanding bugs, in particular not
>>> the RTAI RTAPI.
>> 
>> ok.  How difficult would it be to break all the new stuff out into
>>  modules, and build a modular framework for it?
> 
> well in fact that's what John is doing for RTAPI, with the intent to
> arrive at a universal build that supports RTAI, Xenomai, RT_PREEMPT
> and sim 'automagically'; that's not part of the current merge
> candidate-in-the-waiting though
> 
> the design and flow stands, and I think it's another incremental
> change - basically linking RTAPI though support libraries for RT OS
> x,y or z, that is replaced by loading a .so module - the same stunt
> Jeff does with the interpreter; since Jeff's change wasnt noticed
> widely, we hope this one doesnt either. And every HAL module uses a
> similar method, so that's all safe ground.

Hi fellas,

It's my own fault that the first opinion expressed on the list of the
RTAPI restructuring is one of suspicion.  I got a bit carried away for
just 'cleanups', and didn't broadcast purpose, progress or pointers to
information.  Doubt should be the natural reaction to an end result of a
patch with 3000 additions and 7000 subtractions to a critical piece of code.

Following is a bit of a deep dive into what the RTAPI 'cleanups' are
about in this email.  Apologies for the great length, but I implore
anyone both suspicious of the RTAPI work and also influential in setting
the direction of LinuxCNC's development to at least scan through so it
gets a fair shake.

I hope to show that the 'cleanups' really are just cleanups,
well-contained, introduce almost no new code or make semantic changes to
any existing code, do not touch the API, as Michael said preserve the
design and flow, and are conceptually simple.

At the same time, there are a number of benefits that I hope will be
evident.  Adding new RT technologies is greatly simplified.  Bugs in
what was highly duplicated boilerplate now need only be fixed in a
single place.  And now there is the unexpected opportunity to create a
'unified binary' that will allow a single LinuxCNC binary to run on
multiple thread systems without recompilation.

>>> Altogether I think that is a actually an incremental change.
>> 
>> I would love to be proven wrong on this one, but I can see
>> modularizing monolithic code causing all sorts of unforeseen
>> consequences.  You might be lucky though.
> 
> or, of course, insanely bright ;)

I'd love to say my work was bright, but after reading this, anyone would
see it really is only an incremental change that could have been done by
a junior programmer.  Insane, though, is still a possibility.

--------------
Idea behind the cleanups:

The current mainline code supports three thread systems:  RTAI, sim
('posix' after the cleanups) and RTLinux.  Any thread system
implementation must define several functions declared, documented and
grouped by function in src/rtapi/rtapi.h:

http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=blob;f=src/rtapi/rtapi.h;h=d9773f34603f2ddb9efee694ddf9d9fb5d149ac7;hb=refs/heads/rtos-integration-preview3-merged-into-master

Looking inside the old RT implementations, it is clear that one was
written first, and following implementations simply copied
<threads>_rtapi.c and <threads>_ulapi.c into new files and replaced the
RT system-specific code, a small portion of the overall code.  A 'diff
-u rtl_rtapi.c rtai_rtapi.c | diffstat' shows that 590 of ~1750 lines
differ between the two rtapi implementations, and ~200 of ~840 lines in
ulapi (the important differences are actually much smaller).

This same pattern holds for sim and the new PREEMPT_RT, Xenomai user and
Xenomai kernel thread systems.  Furthermore, a close look back at
rtapi.h shows that many functions like shmem, timing and messaging
functions are duplicated yet again between RTAPI (kernel side) and ULAPI
(userland) within a single thread system.  That's a lot of duplication
in two dimensions.

When Drill Sergeant Haberler barked the order to start cleaning up our
thread implementations (he held me responsible for the PREEMPT_RT code
after I brought it up to date last May), I couldn't see how to clean up
'just a little' while leaving this mess in place.  The initial example
seemed easy (wrong), so I committed to cleaning the whole RTAPI system.

-----------------------
Basic organization of cleanups

Examine these two trees in Michael's git repo representing before and
after snapshots of the work, almost all done within the src/rtapi directory.

Before cleanups (with Xenomai and PREEMPT_RT threads):
http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=tree;f=src/rtapi;h=ffffb586fa28c8e3289cd6780c1604589cbe1039;hb=c46feaa689638de8a0cfd55e55b88e2a5a21c7fa

After cleanups (today):
http://git.mah.priv.at/gitweb?p=emc2-dev.git;a=tree;f=src/rtapi;h=9daa26a1d9aca416626d107cb3a50dffe0613bc7;hb=refs/heads/rtos-integration-preview3-merged-into-master

You'll first notice new files named after the groupings in rtapi.h.  The
main rtapi function definitions, stripped of thread-specific code, have
been removed from the respective thread system files and merged into
generic files of more manageable size; e.g. messaging functions are now
in rtapi_msg.c and shared memory functions in rtapi_shmem.c.

Each of the thread systems has its own .c and its own .h file, e.g.
rtai-kernel.c and rtai-kernel.h (posix threads, formerly 'sim', are now
merged with rt-preempt-user).  These source files retain only the thread
system-specific code, most in what I call 'hook functions' that are
called from the generic code.  Formerly duplicated boilerplate is left
in the generic rtapi_<function>.c files.

Places where thread-specific code is stripped out of the common code
instead contain a call to the appropriate thread-specific 'hook
function'.  For example, the generic rtapi_clock_set_period() function
(declared in rtapi.h and defined in rtapi_time.c) contains a call to
rtapi_clock_set_period_hook() that each kernel threads system must
define in its respective .c file.  Because the boilerplate was virtually
identical, in every thread system the RT system-specific code was always
in the same place in each function, so this method of using hooks is
very clean, with zero or one hook per function, and one exception
requiring two.

And that's the basic idea.  In many functional groups nearly all code
was the same and hooks were not needed at all.  The messaging functions,
for instance, were identical for all systems except RTAI, which has its
own printk() function.  In this case where a hook overcomplicates, the
build is controlled by generic macros that the exceptional thread system
header file may set ('#define RTAPI_PRINTK rt_printk' in this case);
otherwise a suitable default is used.

--------------------------
Walk-through example

Here's a quick walk-through of how it works using rtapi_task_start() as
an example.  This function is declared in rtapi.h (see line 477, 'After'
version) that all thread systems must implement.  In all three kernel
thread styles (RTAI, RTLinux and Xenomai-kernel), the function starts
with nearly identical boiler plate that validates task ID and state,
checks for a running timer, and sets up pointers.  RT system-specific
code then starts the task, and is finally followed by more almost
identical boiler plate.  It is instantly obvious after an eyeball that
most of the code is generic:  see xenomai_kernel_rtapi.c:691,
rtai_rtapi.c:764, and rtl_rtapi.c:837.

In the 'after' tree, the rtapi_task_start() definition has been moved to
rtapi_task.c:325.  The boilerplate is intact, but on line 346 the thread
system code is replaced with a call to rtapi_task_start_hook().  This is
defined in rtai-kernel.c:156 and xenomai-kernel.c:256.  RTLinux was left
out since I don't have that kernel in my menagerie, but it would be a
few hours' work to sift new rtlinux-kernel.[ch] files.

-------------------------------
Benefits

It doesn't simply please my aesthetic senses to chop out 4k lines of
duplicated code!  The resulting merged boilerplate and separated thread
system-specific code naturally form a clean break that makes it easy to
see what minimal code is needed to add a new RT system.  The RTAI code
is only 275 lines, compared to about 1700 lines of more dense and
daunting pre-cleanup code.

This clean split is also what made it apparent that a 'universal binary'
is just a small step away.  All that's needed, as Michael wrote, is RT
kernel detection code (done), the means to load the right shared lib or
kernel module (done), and to define a struct for the interface to
replace #ifdef with runtime conditionals and provide pointers to the
hook functions instead of calling directly.  I am particularly fond of
the unified binary feature because it simplifies packaging and doesn't
require the user to choose the correct package that matches the
installed kernel.

One extra benefit I found while debugging my initial mistakes, it became
clear that any bugs found in the common code can now be fixed in one
place.  No more need to manually apply fixes across several copies of
boilerplate.  Related, a great many useful debugging statements and
comments that existed in some copies but not others were integrated into
the common code.

------------------------------

If you've made it this far, I'm admirous of your high pain threshold and
also grateful for taking the work seriously.  Hopefully the main
concerns about what the changes are and their overall impact on the code
base have been addressed.  Please ask questions about anything still
unclear, and I promise a short answer.

        John

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
Emc-developers mailing list
Emc-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/emc-developers

Reply via email to