Project

General

Profile

Bug #1904

Review CMK_PCQUEUE_LOCK

Added by Evan Ramos 2 months ago. Updated 2 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
SMP
Target version:
-
Start date:
05/10/2018
Due date:
% Done:

0%


Description

From comments on https://charm.cs.illinois.edu/gerrit/418:

Eric Bohm:

Change looks good, has this been tried on the layers that don't #define CMK_PCQUEUE_LOCK in their conv-mach-smp.h ?
Also worth reviewing if the layers that do #define CMK_PCQUEUE_LOCK could now safely switch over.

Evan Ramos:

The only situations in which `#define CMK_PCQUEUE_LOCK 1` occurs are:

1. if x86 assembly does not work
2. On ppc64le
3. On BG/Q

I believe 1 can be unconditionally removed because std::atomic has no dependence on inline assembly. I have no access to 2 and 3 to test.

Eric Bohm:

No need to hold this for review regarding whether layers that lock could stop locking. That is really a separate optimization issue that should be addressed elsewhere.

The availability of std::atomic should lead us to re-evaluate the existence and uses of CMK_PCQUEUE_LOCK. As mentioned above, one nuance it currently holds is "can we use inline assembly to implement PCQueue correctness without a lock?", and the introduction of std::atomic makes this suboptimal. I am unsure what the reasoning is for using it on ppc64le and BG/Q (whether it is due to performance benchmarking, or the unavailability or lack of implementation of the necessary asm), but it is possible that std::atomic obviates the need for locking on these platforms as well. It is possible that outright defining CMK_PCQUEUE_LOCK on these platforms should be replaced with the existing configure test for std::atomic, or its presence should be tested one and CMK_PCQUEUE_LOCK removed altogether.

History

#1 Updated by Sam White 2 months ago

Also available in: Atom PDF