public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Marc Poulhiès" <poulhies@adacore.com>
To: gcc-patches@gcc.gnu.org
Cc: Ronan Desplanques <desplanques@adacore.com>
Subject: [COMMITTED] ada: Tweak CPU affinity handling on Linux
Date: Tue, 18 Jul 2023 15:13:33 +0200	[thread overview]
Message-ID: <20230718131333.81169-1-poulhies@adacore.com> (raw)

From: Ronan Desplanques <desplanques@adacore.com>

Before this patch, the run-time assumed that not specifying a CPU
affinity mask when creating a thread was equivalent to specifying a
CPU affinity mask that included all CPUs.

As documented in the man pages for pthread_create and
pthread_setaffinity_np, this assumption is incorrect: a thread created
using pthread_create inherits the CPU affinity mask of the creating
thread by default. There was a comment in Set_Task_Affinity that
acknowledged this behavior, but the actual code made the erroneous
assumption mentioned above.

That assumption caused the run-time to behave incorrectly when tasks were
explicity assigned to Not_A_Specific_CPU: those tasks were assigned to
the same CPUs as their parents instead of being allowed to run on any
CPU. This patch fixes that behavior.

This patch has the negative effect of making the runtime issue
sched_setaffinity syscalls that are not necessary.

gcc/ada/

	* libgnarl/s-taprop__linux.adb (Set_Task_Affinity, Create_Task): Tweak
	handling of CPU affinities.

Tested on x86_64-pc-linux-gnu, committed on master.

---
 gcc/ada/libgnarl/s-taprop__linux.adb | 38 +++++-----------------------
 1 file changed, 7 insertions(+), 31 deletions(-)

diff --git a/gcc/ada/libgnarl/s-taprop__linux.adb b/gcc/ada/libgnarl/s-taprop__linux.adb
index 821ceef30e4..efb99b3f388 100644
--- a/gcc/ada/libgnarl/s-taprop__linux.adb
+++ b/gcc/ada/libgnarl/s-taprop__linux.adb
@@ -966,16 +966,7 @@ package body System.Task_Primitives.Operations is
 
       --  Handle dispatching domains
 
-      --  To avoid changing CPU affinities when not needed, we set the
-      --  affinity only when assigning to a domain other than the default
-      --  one, or when the default one has been modified.
-
-      elsif T.Common.Domain /= null and then
-        (T.Common.Domain /= ST.System_Domain
-          or else T.Common.Domain.all /=
-                    [Multiprocessors.CPU'First ..
-                     Multiprocessors.Number_Of_CPUs => True])
-      then
+      else
          declare
             CPUs    : constant size_t :=
                         C.size_t (Multiprocessors.Number_Of_CPUs);
@@ -1497,17 +1488,9 @@ package body System.Task_Primitives.Operations is
 
             --  Handle dispatching domains
 
-            elsif T.Common.Domain /= null and then
-              (T.Common.Domain /= ST.System_Domain
-                or else T.Common.Domain.all /=
-                          [Multiprocessors.CPU'First ..
-                           Multiprocessors.Number_Of_CPUs => True])
-            then
+            else
                --  Set the affinity to all the processors belonging to the
-               --  dispatching domain. To avoid changing CPU affinities when
-               --  not needed, we set the affinity only when assigning to a
-               --  domain other than the default one, or when the default one
-               --  has been modified.
+               --  dispatching domain.
 
                CPU_Set := CPU_ALLOC (CPUs);
                System.OS_Interface.CPU_ZERO (Size, CPU_Set);
@@ -1519,18 +1502,11 @@ package body System.Task_Primitives.Operations is
                end loop;
             end if;
 
-            --  We set the new affinity if needed. Otherwise, the new task
-            --  will inherit its creator's CPU affinity mask (according to
-            --  the documentation of pthread_setaffinity_np), which is
-            --  consistent with Ada's required semantics.
-
-            if CPU_Set /= null then
-               Result :=
-                 pthread_setaffinity_np (T.Common.LL.Thread, Size, CPU_Set);
-               pragma Assert (Result = 0);
+            Result :=
+              pthread_setaffinity_np (T.Common.LL.Thread, Size, CPU_Set);
+            pragma Assert (Result = 0);
 
-               CPU_FREE (CPU_Set);
-            end if;
+            CPU_FREE (CPU_Set);
          end;
       end if;
    end Set_Task_Affinity;
-- 
2.40.0


                 reply	other threads:[~2023-07-18 13:13 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230718131333.81169-1-poulhies@adacore.com \
    --to=poulhies@adacore.com \
    --cc=desplanques@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).