Le 23/09/2026 à 23:34, Jerry D a écrit :
On 9/23/26 1:41 PM, Mikael Morin wrote:
Le 22/09/2026 à 14:30, Jerry D a écrit :
The attached patch is version 2 of 2 of 3.
(Patch 1 of 3 was approved previously and is unchanged.)
This responds to Mikael's comment about using callbacks. Callbacks
are eliminated in this version.
Regression tested on x86_64.
OK for mainline?
Regards,
Jerry
---
libgfortran: [PR127347] caf_shmem: do not wait for
terminated images
When an image terminated normally or with FAIL IMAGE, the other images
kept counting it in their team barriers and blocked forever in the next
SYNC ALL. An image executing STOP or FAIL IMAGE now drops out of the
barriers of every team it is a member of before it exits. Lowering the
count wakes the images already waiting once it reaches zero.
An image that terminates without STOP or FAIL IMAGE, e.g. by a signal,
cannot do so. It now initiates error termination of all images, as for
ERROR STOP, instead of being marked failed. Only the fork based
supervisor does so.
I'm not sure it is correct.
Failed state is defined as (F2023 5.3.6: Image execution states):
> An image that has ceased participating in program execution but has
> not initiated termination is a failed image.
Killed images match this definition. And the definition also kind of
implicitly means that the other images continue their execution.
I'm starting to understand what was the reason for the callback in the
original patch. The supervisor has no access to the images' teams
data, so that the call to leave_teams that is done in the STOP case
can't be done by the supervisor in the signal case. Is that correct?
Yes, that is exactly it. The supervisor is a separate process and
only has supervisor->images[]. caf_current_team and caf_teams_formed
are per-image lists in the image's own memory, so it cannot walk the
teams an image belonged to, and it cannot lower their barrier counts.
I also agree with your reading of F2023 5.3.6: a killed image is a
failed image and the remaining images are meant to keep running, so
terminating all of them is a deviation. I have split that part out of
2/3 into a separate patch (attached separately, not for commit yet) so
that the rest can go in. Its commit message now states the deviation
explicitly. Without it, images waiting in a team barrier for a killed
image still block, as they do today.
Maybe we can store a copy of caf_current_team->u.image_info and
caf_teams_formed->u.image_info in supervisor's image_tracker, so that
a call to leave_teams or similar can be done from the supervisor.
A pointer to the parent would also be necessary in shmem_image_info.
Unfortunately it's not very nice to have every image updating rapidly
the current team of their image info stored in shared memory, to
support something as rare as a signal. I have nothing better to
propose so far.
Agreed, that would be a lot of traffic for a rare event. How about
turning it around and registering teams instead of images? A team's
shmem_image_info already lives in shared memory and is created exactly
once, by the first image reaching alloc_get_memory_by_id_created in
FORM TEAM. If that image also appended the team to a list in shared
memory, the supervisor could walk the list when it reaps an image,
and, under each team's barrier lock, recompute the count from the
image statuses and broadcast -- the same thing update_teams_images
does now, just done by the supervisor. A team is only ever added,
so no per-statement updates are needed, and images that are not in the
team are unaffected because their count does not change. END TEAM
would leave the entry in place.
I have not implemented this yet; if you think it is the right
direction I will do that instead of the signal patch.
Yes, sounds good. The list could be just one extra pointer in
shmem_image_info and one extra pointer in the supervisor if I understand
correctly.
One thing to note in the new 2/3: the supervisor.c hunk is not just
dropped, it is replaced. mark_stopped now records IMAGE_SUCCESS
before exit, but a STOP with a non-zero stop code exits non-zero, and
the supervisor marked every such image as failed, overwriting that and
counting it twice. So the status the image recorded is now kept, the
same way it already is for an image exiting with zero. That hunk is
new since your review.
Thanks, I overlooked this.
One nit:
Please mention that the number of finished or failed images has to be
updated beforehand for the function to have any effect.
void check_health (int *, char *, size_t);
#define HEALTH_CHECK(stat, errmsg, errlen) check_health (stat,
errmsg, errlen)
OK for the non-signal part (all but supervisor.c and
signal_sync_1.f90), with the nit above addressed.
The doc nit is addressed: leave_teams now says the number of finished
or failed images has to be updated before the call for it to have any
effect.
The attached patch is now v3 of the 2/3. The signal part I will submit
seprately later
OK for 2/3 (v3).
3/3 is attached as v3 as well. The only change from the posted v2 is
the leave_teams comment, which now keeps the sentence added in 2/3
instead of replacing it; without that the two patches conflict.
Retested on x86_64 with 2, 3 and 8 images, no hangs and no new
failures.
for 3/3 (v3) I have just one concern below; the rest is OK.
diff --git a/libgfortran/caf/shmem/collective_subroutine.c
b/libgfortran/caf/shmem/collective_subroutine.c
index 01389b18b0a..f97789cab1c 100644
--- a/libgfortran/caf/shmem/collective_subroutine.c
+++ b/libgfortran/caf/shmem/collective_subroutine.c
@@ -219,12 +220,17 @@ get_collsub_buf (size_t size)
}
/* This function syncs all images with one another. It will only return once
- all images have called it. */
+ all images have called it. The reduction is laid out over a fixed set of
+ images, so it cannot be completed once an image of the team terminated. */
static void
collsub_sync (void)
{
- counter_barrier_wait (&caf_current_team->u.image_info->collsub.barrier);
+ counter_barrier *barrier = &caf_current_team->u.image_info->collsub.barrier;
+
+ if (!counter_barrier_wait_abortable (barrier))
+ caf_runtime_error ("Image terminated while executing a collective "
+ "subroutine");
}
typedef void *(*red_op) (void *, void *);
This causes all images to abort when an immage stops, even if a STAT=
argument was provided. The first SYNC ALL in your testcase prevents
this from ever triggering. Here is a testcase:
program prog
implicit none
integer :: s[*]
integer :: i, st, img
if (num_images() == 1) stop
img = num_images() / 2
print "('img=',i3)", img
s = 0
if (this_image() == img) then
s = heavy_work()
stop
else
s = this_image()
end if
call co_sum(s, stat=st)
print "('img=', i3, ' s=', i5, ' stat=', i3)", this_image(), s, st
contains
function heavy_work() result(r)
integer :: r, c
integer(kind=16) :: v
integer, parameter :: i = 2
integer, parameter :: p = 1999619
c = 0
v = i
do while (c == 0 .or. v /= i)
v = v * i
v = mod(v, p)
c = c + 1
end do
r = c
end function
end program
As the patch contains already many things, you can push as is or with
the caf_runtime_error call removed, and work on a fix as a followup.
Thanks for the patches.
