[etherlab-dev] ec_lock_* vs. ec_ioctl_lock in master/ioctl.c

[Esben Haabendal]

Graeme Foot <Graeme.Foot at touchcut.com> writes:

> The ec_ioctl_lock_up() and ec_ioctl_lock_down_interruptible() calls were added
> to protect the following functions when multiple send/receive loops are
> running:
> - ec_master_send()
> - ec_master_receive()
> - ec_master_domain_process()
> - ec_master_domain_queue()
>
> In my opinion any locking on these functions should be at the application
> level instead.

Well, I have a different opinion on that.

Implementation of locking is inherently difficult to get right.  You
both have to ensure against race conditions while avoiding deadlocks.
But you also do not want to block for too long.

While I do acknowledge that for trivial cases where there are only a
single application, it is not a big program for that single application
to maintain synchronization, I don't think that it is a good solution to
let each application developer in the more complicated situations (like
multiple independent processes) have to do this without any help from
etherlabmaster.  Forcing all (or at least some) application developers
to solve this same problem again and again should not be the best we can
do.

> However, if they are being called from multiple processes (rather than
> threads) then you need to use something like a named semaphore so that
> all processes share the same lock.  Of course if you are using
> callbacks (for EoE) you are probably doing that anyway.

You easily have We have multiple processes.  Even with just a single
application, you have EtherCAT-OP, EtherCAT-EoE and the application.
All using some of the same shared data structures.  Throwing more
multiple application just adds to that, but I think it is critical
enough with just a single application.

> The reason that they have been named differently is that they are the
> functions that are called from the realtime send/receive loop and the define
> allows them to be ignored.  RTAI (and Xenomi?) are NOT allowed to call
> standard Linux blocking functions from a hard realtime thread (to maintain
> hard realtime).

Yes, I get that.

> The EC_IOCTL_RTDM define turns off these locks for (RTAI / Xenomi)
> RTDM applications to avoid this problem.

Yes, EC_IOCTL_RTDM turns off these locks.  But it does not really do
anything, as it is never defined.

> They should probably also be turned off for RTAI / Xenomi in general
> and as I said above use application level locking.
>
> You can pass --enable-rtdm when compiling to enable RTDM (and compile
> rtdm-ioctl.o).

Passing --enable-rtdm to ./configure will define EC_RTDM macro and
enable the automake conditional ENABLE_RTDM.

This will trigger Kbuild to build rtdm-ioctl.o (from rtdm-ioctl.c), and
this will be done with EC_IOCTL_RTDM macro defined.

But ioctl.c will be compiled as always, and that is without
EC_IOCTL_RTDM defined.  Was it supposed to be defined?  If so, it should
be easy to fix, but someone should definitely do some real testing of
it.

> So in summary:
> ec_ioctl_lock_up() and ec_ioctl_lock_down_interruptible() are for the hard
> realtime loop function calls, otherwise use the standard lock calls.

And these are the closed set of

- ec_ioctl_send()
- ec_ioctl_receive()
- ec_ioctl_domain_process()
- ec_ioctl_domain_queue()

?

/Esben