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 ;)
