On Thu, 2016-09-15 at 09:22 +0200, Jakub Jelinek wrote:
> Hi!
> These days on many targets that use dl_iterate_phdr to find .eh_frame_hdr
> that way in most of the programs the old style EH registry is never used,
> yet we still lock a global mutex and unlock it soon afterwards to find out
> it is the case.
> This patch adds a fast path to that, by replacing that with an atomic load
> of a flag "has anything ever been registered in the old style registry"
> and if the flag says no, it avoids the locking.

I think this needs a more detailed reasoning about the synchronization,
and comments in the code that explain this reasoning.  For example,
there are no comments why you need the acquire/release pair, nor what
the requirements or external synchronization is that would make it
sufficient to use relaxed memory order.

The current code with the critical sections seems to only ensure that
_Unwind_Find_FDE observes calls to  __register_frame_info_bases or
__register_frame_info_table_bases if those already happen-before
(referring to the so-called relation in the memory model) the
_Unwind_Find_FDE call.  This happen-before can be due to other
synchronization or sequenced-before.
If that is all you need, relaxed MO would be sufficient, but then this
should get documented.  If that's not all that you need, then what the
patch adds seems insufficient (as well as the existing code).

The acquire/release pair you currently have in the patch would ensure
that the changes to unseen_objects are observed if
any_objects_registered is either 0 or 1; if 1, you use the critical
section, so the critical section that stores the 1 that you read from
would happen-before the _Unwind_Find_FDE critical section anyway.  If
one observes 0, I guess you don't have any invariant wrt other stores
(that the caller would have to observe too).

Also, it might be possible that the critical section is required to
serve as a fence wrt something else, under some conditions.  But that
would be a weird way to synchronize, so I guess that wouldn't be the
case.  OTOH, I don't know enough about the callers, so maybe it's worth
considering ;)

Reply via email to