[ET Trac] #2832: possible race condition in LoopControl

[Roland Haas]

#2832: possible race condition in LoopControl

 Reporter: Roland Haas
   Status: new
Milestone: 
  Version: 
     Type: bug
 Priority: major
Component: 

Comment (by Roland Haas):

The issue may be alignment related. gcc-14 produces a warning:

```
COMPILING Carpet/LoopControl/src/loopcontrol.cc
/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/yosef/build/LoopControl/loopcontrol.cc: In function ?voi  LC_control_init(lc_control_t*, lc_descr_t*, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t, ptrdiff_t)?:
/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/yosef/build/LoopControl/loopcontrol.cc:797:29: warning:  new? of type ?lc_thread_info_t? with extended alignment 128 [-Waligned-new=]
  797 |     { thread_info_ptr = new lc_thread_info_t; }
      |                             ^~~~~~~~~~~~~~~~
/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/yosef/build/LoopControl/loopcontrol.cc:797:29: note: uses ?void* operator new(std::size_t)?, which does not have an alignment parameter
/data/rhaas/postdoc/gr/cactus/ET_trunk/configs/yosef/build/LoopControl/loopcontrol.cc:797:29: note: use ?-faligned-new? to enable C++17 over-aligned new support
```

and changing LoopControl like this:  

```diff
diff --git a/LoopControl/src/loopcontrol.cc b/LoopControl/src/loopcontrol.cc
index a2e3460f2..062db9c1f 100644
--- a/LoopControl/src/loopcontrol.cc
+++ b/LoopControl/src/loopcontrol.cc
@@ -71,12 +71,16 @@ static minstd_rand::result_type const constexpr lc_random_range =

 struct lc_thread_info_t {
   volatile int idx;            // linear index of next coarse thread block
-} CCTK_ATTRIBUTE_ALIGNED(128); // align to prevent sharing cache lines
+  char pad[128 - sizeof(int)]; // align to prevent sharing cache lines
+};
+static_assert(sizeof(lc_thread_info_t) == 128, "Incorrect size for alignment");

 struct lc_fine_thread_comm_t {
   volatile int state;          // waiting threads
   volatile int value;          // broadcast value
-} CCTK_ATTRIBUTE_ALIGNED(128); // align to prevent sharing cache lines
+  char pad[128-2*sizeof(int)]; // align to prevent sharing cache lines
+};
+static_assert(sizeof(lc_fine_thread_comm_t) == 128, "Incorrect size for alignment");

 // One object per coarse thread: shared between fine threads
 // Note: Since we use a vector, the individual elements may not
```

makes the issue go away.

Note that this may not actually give alignment to 128 bytes as originally requested. However the comments make it clear that alignment was never the goal, instead the goal was to spread out consecutive `lc_fine_thread_comm_t` across multiple cache lines, which making them big enough should also achieve.

--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2832/possible-race-condition-in-loopcontrol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.einsteintoolkit.org/pipermail/trac/attachments/20241105/e5e25856/attachment.htm>