From patchwork Wed May 25 09:23:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 54368 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4FE743836422 for ; Wed, 25 May 2022 09:24:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4FE743836422 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1653470654; bh=B1WV10JVkimiYfdyNgN483mnVDSDUtGA5zWIl7P+JSI=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=S3zH7osJFUXAcguH9uAFq7fvn48ZvCbdAT6Uxgs7BWqeTCGEfxsS7US4Qd4O8oQr/ 3CsegGHp8/Ob55GqlXdwHVl3W2OE76iZpp9+/H4jDwywxb8chWH00326L6W5aRr3UQ DffEuhr5EwYy8hqIGj6lHe0loCsMQDUq+IcQfssQ= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id EFAC03838023 for ; Wed, 25 May 2022 09:23:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EFAC03838023 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-214-M-yf4CxqMu-Qs57jyfxSjw-1; Wed, 25 May 2022 05:23:18 -0400 X-MC-Unique: M-yf4CxqMu-Qs57jyfxSjw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0ADD3801228; Wed, 25 May 2022 09:23:18 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8DB98112131E; Wed, 25 May 2022 09:23:17 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 24P9NESx030444 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 25 May 2022 11:23:15 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24P9NDZ3030443; Wed, 25 May 2022 11:23:13 +0200 Date: Wed, 25 May 2022 11:23:12 +0200 To: gcc-patches@gcc.gnu.org Subject: [committed] libgomp: Fix occassional hangs with taskwait nowait depend Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Cc: Tobias Burnus Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Hi! Richi reported occassional hangs with taskwait-depend-nowait-1.* tests and I've finally manged to reproduce. The problem is if taskwait depend without nowait is encountered soon after taskwait depend nowait and the former depends on the latter and there is no other work to do, the taskwait depend without nowait is put to sleep, but the empty_task optimization in gomp_task_run_post_handle_dependers wouldn't wake it up in that case. gomp_task_run_post_handle_dependers normally does some wakeups because it schedules more work (another task), which is not the case of empty_task, but we need to do the wakeups that would be done upon task completion so that we awake sleeping threads when the last child is done. So, the taskwait-depend-nowait-1.* testcase is fixed with the else if (__builtin_expect (task->parent_depends_on, 0) part of the patch. The new testcase can hang on another problem, if the empty task is the last task of a taskgroup, we need to use atomic store like elsewhere to decrease the counter to 0, and wake up taskgroup end if needed. Yet another spot which can sleep is normal taskwait (without depend), but I believe nothing needs to be done for that - in that case we await solely until the children's queue has no tasks, tasks still waiting for dependencies aren't accounted in that, but the reason is that if taskwait should wait for something, there needs to be at least one active child doing something (in the children queue), which then possibly awakes some of its siblings when the dependencies are met, or in the empty task case awakes further dependencies, but in any case the child that finished is still handled as active child and will awake taskwait at the end if there is nothing further to do. Last sleeping case are barriers, but that is handled by ++ret and awaking the barrier. Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk. 2022-05-25 Jakub Jelinek * task.c (gomp_task_run_post_handle_dependers): If empty_task is the last task taskwait depend depends on, wake it up. Similarly if it is the last child of a taskgroup, use atomic store instead of decrement and awak taskgroup wait if any. * testsuite/libgomp.c-c++-common/taskwait-depend-nowait-2.c: New test. Jakub --- libgomp/task.c.jj 2022-05-23 22:38:26.381094885 +0200 +++ libgomp/task.c 2022-05-24 18:23:03.054074341 +0200 @@ -1382,10 +1382,30 @@ gomp_task_run_post_handle_dependers (str { if (!parent) task->parent = NULL; + else if (__builtin_expect (task->parent_depends_on, 0) + && --parent->taskwait->n_depend == 0 + && parent->taskwait->in_depend_wait) + { + parent->taskwait->in_depend_wait = false; + gomp_sem_post (&parent->taskwait->taskwait_sem); + } if (gomp_task_run_post_handle_depend (task, team)) ++ret; if (taskgroup) - taskgroup->num_children--; + { + if (taskgroup->num_children > 1) + --taskgroup->num_children; + else + { + __atomic_store_n (&taskgroup->num_children, 0, + MEMMODEL_RELEASE); + if (taskgroup->in_taskgroup_wait) + { + taskgroup->in_taskgroup_wait = false; + gomp_sem_post (&taskgroup->taskgroup_sem); + } + } + } gomp_finish_task (task); free (task); continue; --- libgomp/testsuite/libgomp.c-c++-common/taskwait-depend-nowait-2.c.jj 2022-05-25 09:56:20.131618294 +0200 +++ libgomp/testsuite/libgomp.c-c++-common/taskwait-depend-nowait-2.c 2022-05-25 10:50:36.781833445 +0200 @@ -0,0 +1,48 @@ +#include +#include + +int +main () +{ + int a[48], b = 1; + #pragma omp parallel num_threads (4) + { + #pragma omp barrier + #pragma omp single + { + int i; + for (i = 0; i < 48; ++i) + #pragma omp task depend(in: a) shared(a) + a[i] = i; + for (i = 0; i < 32; ++i) + { + #pragma omp taskwait depend(inout: a) nowait + } + #pragma omp taskwait + for (i = 0; i < 48; ++i) + if (a[i] != i) + abort (); + for (i = 0; i < 48; ++i) + #pragma omp task depend(in: a) shared(a) + a[i] = 2 * i + 1; + #pragma omp taskgroup + { + #pragma omp taskwait depend(inoutset: a) nowait + #pragma omp taskgroup + { + #pragma omp taskwait depend(inoutset: a) nowait + } + } + for (i = 0; i < 48; ++i) + if (a[i] != 2 * i + 1) + abort (); + #pragma omp task depend(in: a) shared(a) + usleep (5000); + #pragma omp taskgroup + { + #pragma omp taskwait depend(inout: a) nowait + } + } + } + return 0; +}