On Fri, Jul 20, 2012, at 01:06 PM, Michael Haberler wrote:
> looking at hal_lib.c some more:
> 
> the hal_set_lock() and hal_get_lock() functions are kind of misnomers
> IMO, all they do is set a global *flag* which is NOT a mutex or atomic
> for that matter, so it is advisory in nature and some of the routines in
> hal_lib.c report an error if some lock bit is set
> 
> for the same reason the halcmd lock and unlock commands are misnomers IMO
> too - the dont aquire or release the mutex, they set said flag

The halcmd lock and unlock commands (and the underlying code) have 
nothing to do with mutexes.  They were added years ago in response
to someone grumbling that the interactive nature of HAL means that
a system could be (re)configured while it was running, and that was
a bad thing.  Issuing a halcmd "lock tune" disables things like net
and unlink and addf which could be used to modify the execution of
realtime code and the flow of data between modules, but allows things
like setp that are used for tuning.  "lock all" disables setp as
well.  This isn't really intended to provide security, it is very
easy to issue "halcmd unlock".  It is more to prevent accidents.
It isn't well documented (missing from the man page, for example:
http://sourceforge.net/tracker/?func=detail&aid=3389036&group_id=6744&atid=106744
), and I doubt anyone actually uses it.

> however, the real locking and unlocking is always done with
> rtapi_mutex_get(&(hal_data->mutex)) and
> rtapi_mutex_give(&(hal_data->mutex)), so I dont quite see the value of
> those lock bits in the first place - if the idea was to allow for more
> parallelism due to finer-granular locks, that is defeated by having a
> single hal_data->mutex lateron in the first place

As mentioned above, the mutex and the lock bits are completely
unrelated, and the lock bits are probably used by 0.001% of the
user population.

The mutex exists to protect the HAL metadata from corruption due
to concurrent access.  It is unlikely that someone will be running
two instances of halcmd at the same time, but not impossible.  It
is also possible to have more than one component starting up at
the same time (during a startup script for example).  Components
export pins during startup, and that modifies the metadata.

Metadata includes the lists of components, functions, threads,
pins, signals, etc.  Inserting or deleting items from those lists
is not an atomic operation, hence the mutex.  Likewise, scrolling
through the lists (for example "halcmd show pin") could crash if
a list item was added or deleted half-way through.

The actual realtime code (functions that are exported from HAL
components and run in realtime threads) do not and should not
use the mutexes.  The flow of data from one HAL component to
another is atomic.  Even if someone is using halcmd to connect
or disconnect a pin, the realtime code will continue to work.
The actual change of the pin (connect, disconnect, reconnect)
is atomic - simply writing a pointer.  And since realtime code
runs at a higher priority than userspace, a pin connection will
never change during the execution of a RT thread, only between
passes thru the thread.


> 
> IMO doing away with the set/get lock calls and data, and
> testing for locked condition with a call to rtapi_mutex_try()
> should give the same effect cleaner and for less bloat, but I
> might be overlooking something

I haven't looked at the code in ages, but keep in mind that
the lock and unlock HAL commands and the metadata locking are
completely different things.

That said, I think I agree with you that we could do away
with the lock/unlock HAL commands.  They are undocumented and
probably unused.  Keeping that code around is certainly a 
form of bloat.

 
> Maybe jmk can shed some light on the intent and purpose

I hope this helped.  I'm afraid I probably didn't do much
to answer the original question, which had to do with getting
memory from hal_malloc, and what locking needed to be done
in that case.  The original question didn't contain enough
information for me to figure out what they are trying to do.
(And I have been away from the LinuxCNC code for so long that
I hoped someone else would answer it.)

-- 
  John Kasunich
  [email protected]


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Emc-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/emc-developers

Reply via email to