[libc-commits] [libc] 943d194 - [libc] Fix sched_getscheduler return value

Mikhail R. Gadelha via libc-commits libc-commits at lists.llvm.org
Tue Aug 1 10:27:33 PDT 2023 

Author: Mikhail R. Gadelha
Date: 2023-08-01T14:26:41-03:00
New Revision: 943d194fd90fc943fd0b99cf987c1b118f8edaad

URL: https://github.com/llvm/llvm-project/commit/943d194fd90fc943fd0b99cf987c1b118f8edaad
DIFF: https://github.com/llvm/llvm-project/commit/943d194fd90fc943fd0b99cf987c1b118f8edaad.diff

LOG: [libc] Fix sched_getscheduler return value

This patch fixes the return time for sched_getscheduler which was set to
always zero. The syscall documentation, however, defines:

On success, sched_getscheduler() returns the policy for the thread (a
nonnegative integer).

I also changed the return type for sched_setscheduler, but this change
didn't impact and test case.

This patch also removes the duplicated code from param_and_scheduler_test.cpp
and adds SCHED_BATCH and SCHED_IDLE to the tests.

Reviewed By: michaelrj

Differential Revision: https://reviews.llvm.org/D156700

Added: 
    

Modified: 
    libc/src/sched/linux/sched_getscheduler.cpp
    libc/src/sched/linux/sched_setscheduler.cpp
    libc/test/src/sched/param_and_scheduler_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/sched/linux/sched_getscheduler.cpp b/libc/src/sched/linux/sched_getscheduler.cpp
index 997aaac151a776..a8f9b49974131d 100644
--- a/libc/src/sched/linux/sched_getscheduler.cpp
+++ b/libc/src/sched/linux/sched_getscheduler.cpp
@@ -22,7 +22,7 @@ LLVM_LIBC_FUNCTION(int, sched_getscheduler, (pid_t tid)) {
     libc_errno = -ret;
     return -1;
   }
-  return 0;
+  return ret;
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/sched/linux/sched_setscheduler.cpp b/libc/src/sched/linux/sched_setscheduler.cpp
index d91c69e3339522..36fe9d95d39d78 100644
--- a/libc/src/sched/linux/sched_setscheduler.cpp
+++ b/libc/src/sched/linux/sched_setscheduler.cpp
@@ -24,7 +24,7 @@ LLVM_LIBC_FUNCTION(int, sched_setscheduler,
     libc_errno = -ret;
     return -1;
   }
-  return 0;
+  return ret;
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/test/src/sched/param_and_scheduler_test.cpp b/libc/test/src/sched/param_and_scheduler_test.cpp
index 35e902cedface3..2c5745432f579a 100644
--- a/libc/test/src/sched/param_and_scheduler_test.cpp
+++ b/libc/test/src/sched/param_and_scheduler_test.cpp
@@ -32,313 +32,124 @@
 //          sched policy on a running task.
 //
 //       Linux specific test could also include:
-//          SCHED_BATCH, SCHED_ISO, SCHED_IDLE, SCHED_DEADLINE
+//          SCHED_ISO, SCHED_DEADLINE
 
-TEST(LlvmLibcSchedParamAndSchedulerTest, SchedOtherTest) {
-  libc_errno = 0;
-
-  int policy = SCHED_OTHER;
-  bool can_set = true;
-
-  int init_policy = __llvm_libc::sched_getscheduler(0);
-  ASSERT_GE(init_policy, 0);
-  ASSERT_EQ(libc_errno, 0);
-
-  int max_priority = __llvm_libc::sched_get_priority_max(policy);
-  ASSERT_GE(max_priority, 0);
-  ASSERT_EQ(libc_errno, 0);
-  int min_priority = __llvm_libc::sched_get_priority_min(policy);
-  ASSERT_GE(min_priority, 0);
-  ASSERT_EQ(libc_errno, 0);
-
-  struct sched_param param = {min_priority};
-
-  // Negative pid
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(-1, policy, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  ASSERT_EQ(__llvm_libc::sched_getscheduler(-1), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  // Invalid Policy
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy | 128, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  // Out of bounds priority
-  param.sched_priority = min_priority - 1;
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  param.sched_priority = max_priority + 1;
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
-  // A bit hard to test as depending if we are root or not we can run into
-  // 
diff erent issues.
-  ASSERT_TRUE(libc_errno == EINVAL || libc_errno == EPERM);
-  libc_errno = 0;
-
-  // Some sched policies require permissions, so skip
-  param.sched_priority = min_priority;
-  // Success / missing permissions.
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param),
-            can_set ? 0 : -1);
-  ASSERT_TRUE(can_set ? (libc_errno == 0)
-                      : (libc_errno == EINVAL || libc_errno == EPERM));
-  libc_errno = 0;
-
-  ASSERT_EQ(__llvm_libc::sched_getscheduler(0), can_set ? policy : init_policy);
-  ASSERT_EQ(libc_errno, 0);
-
-  // Out of bounds priority
-  param.sched_priority = -1;
-  ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+class SchedTest : public __llvm_libc::testing::Test {
+public:
+  void testSched(int policy, bool can_set) {
+    libc_errno = 0;
 
-  param.sched_priority = max_priority + 1;
-  ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+    int init_policy = __llvm_libc::sched_getscheduler(0);
+    ASSERT_GE(init_policy, 0);
+    ASSERT_EQ(libc_errno, 0);
 
-  for (int priority = min_priority; priority <= max_priority; ++priority) {
-    ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
+    int max_priority = __llvm_libc::sched_get_priority_max(policy);
+    ASSERT_GE(max_priority, 0);
+    ASSERT_EQ(libc_errno, 0);
+    int min_priority = __llvm_libc::sched_get_priority_min(policy);
+    ASSERT_GE(min_priority, 0);
     ASSERT_EQ(libc_errno, 0);
-    int init_priority = param.sched_priority;
 
-    param.sched_priority = priority;
+    struct sched_param param = {min_priority};
 
     // Negative pid
-    ASSERT_EQ(__llvm_libc::sched_setparam(-1, &param), -1);
+    ASSERT_EQ(__llvm_libc::sched_setscheduler(-1, policy, &param), -1);
     ASSERT_EQ(libc_errno, EINVAL);
     libc_errno = 0;
 
-    ASSERT_EQ(__llvm_libc::sched_getparam(-1, &param), -1);
+    ASSERT_EQ(__llvm_libc::sched_getscheduler(-1), -1);
     ASSERT_EQ(libc_errno, EINVAL);
     libc_errno = 0;
 
-    // Success / missing permissions
-    ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), can_set ? 0 : -1);
-    ASSERT_TRUE(can_set ? (libc_errno == 0)
-                        : (libc_errno == EINVAL || libc_errno == EPERM));
+    // Invalid Policy
+    ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy | 128, &param), -1);
+    ASSERT_EQ(libc_errno, EINVAL);
     libc_errno = 0;
 
-    ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
-    ASSERT_EQ(libc_errno, 0);
-
-    ASSERT_EQ(param.sched_priority, can_set ? priority : init_priority);
-  }
-  // Null test
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, nullptr), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-}
-
-TEST(LlvmLibcSchedParamAndSchedulerTest, SchedFIFOTest) {
-  libc_errno = 0;
-
-  int policy = SCHED_FIFO;
-  bool can_set = __llvm_libc::getuid() == 0;
-
-  int init_policy = __llvm_libc::sched_getscheduler(0);
-  ASSERT_GE(init_policy, 0);
-  ASSERT_EQ(libc_errno, 0);
-
-  int max_priority = __llvm_libc::sched_get_priority_max(policy);
-  ASSERT_GE(max_priority, 0);
-  ASSERT_EQ(libc_errno, 0);
-  int min_priority = __llvm_libc::sched_get_priority_min(policy);
-  ASSERT_GE(min_priority, 0);
-  ASSERT_EQ(libc_errno, 0);
-
-  struct sched_param param = {min_priority};
-
-  // Negative pid
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(-1, policy, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  ASSERT_EQ(__llvm_libc::sched_getscheduler(-1), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  // Invalid Policy
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy | 128, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  // Out of bounds priority
-  param.sched_priority = min_priority - 1;
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  param.sched_priority = max_priority + 1;
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
-  // A bit hard to test as depending if we are root or not we can run into
-  // 
diff erent issues.
-  ASSERT_TRUE(libc_errno == EINVAL || libc_errno == EPERM);
-  libc_errno = 0;
-
-  // Some sched policies require permissions, so skip
-  param.sched_priority = min_priority;
-  // Success / missing permissions.
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param),
-            can_set ? 0 : -1);
-  ASSERT_TRUE(can_set ? (libc_errno == 0)
-                      : (libc_errno == EINVAL || libc_errno == EPERM));
-  libc_errno = 0;
-
-  ASSERT_EQ(__llvm_libc::sched_getscheduler(0), can_set ? policy : init_policy);
-  ASSERT_EQ(libc_errno, 0);
-
-  // Out of bounds priority
-  param.sched_priority = -1;
-  ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  param.sched_priority = max_priority + 1;
-  ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  for (int priority = min_priority; priority <= max_priority; ++priority) {
-    ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
-    ASSERT_EQ(libc_errno, 0);
-    int init_priority = param.sched_priority;
-
-    param.sched_priority = priority;
-
-    // Negative pid
-    ASSERT_EQ(__llvm_libc::sched_setparam(-1, &param), -1);
+    // Out of bounds priority
+    param.sched_priority = min_priority - 1;
+    ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
     ASSERT_EQ(libc_errno, EINVAL);
     libc_errno = 0;
 
-    ASSERT_EQ(__llvm_libc::sched_getparam(-1, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
+    param.sched_priority = max_priority + 1;
+    ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
+    // A bit hard to test as depending if we are root or not we can run into
+    // 
diff erent issues.
+    ASSERT_TRUE(libc_errno == EINVAL || libc_errno == EPERM);
     libc_errno = 0;
 
-    // Success / missing permissions
-    ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), can_set ? 0 : -1);
+    // Some sched policies require permissions, so skip
+    param.sched_priority = min_priority;
+    // Success / missing permissions.
+    ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param),
+              can_set ? 0 : -1);
     ASSERT_TRUE(can_set ? (libc_errno == 0)
                         : (libc_errno == EINVAL || libc_errno == EPERM));
     libc_errno = 0;
 
-    ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
+    ASSERT_EQ(__llvm_libc::sched_getscheduler(0),
+              can_set ? policy : init_policy);
     ASSERT_EQ(libc_errno, 0);
 
-    ASSERT_EQ(param.sched_priority, can_set ? priority : init_priority);
-  }
-  // Null test
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, nullptr), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-}
-
-TEST(LlvmLibcSchedParamAndSchedulerTest, SchedRRTest) {
-  libc_errno = 0;
-
-  int policy = SCHED_RR;
-  bool can_set = __llvm_libc::getuid() == 0;
-
-  int init_policy = __llvm_libc::sched_getscheduler(0);
-  ASSERT_GE(init_policy, 0);
-  ASSERT_EQ(libc_errno, 0);
-
-  int max_priority = __llvm_libc::sched_get_priority_max(policy);
-  ASSERT_GE(max_priority, 0);
-  ASSERT_EQ(libc_errno, 0);
-  int min_priority = __llvm_libc::sched_get_priority_min(policy);
-  ASSERT_GE(min_priority, 0);
-  ASSERT_EQ(libc_errno, 0);
-
-  struct sched_param param = {min_priority};
-
-  // Negative pid
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(-1, policy, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-
-  ASSERT_EQ(__llvm_libc::sched_getscheduler(-1), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+    // Out of bounds priority
+    param.sched_priority = -1;
+    ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
+    ASSERT_EQ(libc_errno, EINVAL);
+    libc_errno = 0;
 
-  // Invalid Policy
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy | 128, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+    param.sched_priority = max_priority + 1;
+    ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
+    ASSERT_EQ(libc_errno, EINVAL);
+    libc_errno = 0;
 
-  // Out of bounds priority
-  param.sched_priority = min_priority - 1;
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+    for (int priority = min_priority; priority <= max_priority; ++priority) {
+      ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
+      ASSERT_EQ(libc_errno, 0);
+      int init_priority = param.sched_priority;
 
-  param.sched_priority = max_priority + 1;
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param), -1);
-  // A bit hard to test as depending if we are root or not we can run into
-  // 
diff erent issues.
-  ASSERT_TRUE(libc_errno == EINVAL || libc_errno == EPERM);
-  libc_errno = 0;
+      param.sched_priority = priority;
 
-  // Some sched policies require permissions, so skip
-  param.sched_priority = min_priority;
-  // Success / missing permissions.
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, &param),
-            can_set ? 0 : -1);
-  ASSERT_TRUE(can_set ? (libc_errno == 0)
-                      : (libc_errno == EINVAL || libc_errno == EPERM));
-  libc_errno = 0;
+      // Negative pid
+      ASSERT_EQ(__llvm_libc::sched_setparam(-1, &param), -1);
+      ASSERT_EQ(libc_errno, EINVAL);
+      libc_errno = 0;
 
-  ASSERT_EQ(__llvm_libc::sched_getscheduler(0), can_set ? policy : init_policy);
-  ASSERT_EQ(libc_errno, 0);
+      ASSERT_EQ(__llvm_libc::sched_getparam(-1, &param), -1);
+      ASSERT_EQ(libc_errno, EINVAL);
+      libc_errno = 0;
 
-  // Out of bounds priority
-  param.sched_priority = -1;
-  ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+      // Success / missing permissions
+      ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), can_set ? 0 : -1);
+      ASSERT_TRUE(can_set ? (libc_errno == 0)
+                          : (libc_errno == EINVAL || libc_errno == EPERM));
+      libc_errno = 0;
 
-  param.sched_priority = max_priority + 1;
-  ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
+      ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
+      ASSERT_EQ(libc_errno, 0);
 
-  for (int priority = min_priority; priority <= max_priority; ++priority) {
-    ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
-    ASSERT_EQ(libc_errno, 0);
-    int init_priority = param.sched_priority;
+      ASSERT_EQ(param.sched_priority, can_set ? priority : init_priority);
+    }
 
-    param.sched_priority = priority;
-
-    // Negative pid
-    ASSERT_EQ(__llvm_libc::sched_setparam(-1, &param), -1);
-    ASSERT_EQ(libc_errno, EINVAL);
-    libc_errno = 0;
-
-    ASSERT_EQ(__llvm_libc::sched_getparam(-1, &param), -1);
+    // Null test
+    ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, nullptr), -1);
     ASSERT_EQ(libc_errno, EINVAL);
     libc_errno = 0;
+  }
+};
 
-    // Success / missing permissions
-    ASSERT_EQ(__llvm_libc::sched_setparam(0, &param), can_set ? 0 : -1);
-    ASSERT_TRUE(can_set ? (libc_errno == 0)
-                        : (libc_errno == EINVAL || libc_errno == EPERM));
-    libc_errno = 0;
+#define LIST_SCHED_TESTS(policy, can_set)                                      \
+  using LlvmLibcSchedTest = SchedTest;                                         \
+  TEST_F(LlvmLibcSchedTest, Sched_##policy) { testSched(policy, can_set); }
 
-    ASSERT_EQ(__llvm_libc::sched_getparam(0, &param), 0);
-    ASSERT_EQ(libc_errno, 0);
+// Root is required to set these policies.
+LIST_SCHED_TESTS(SCHED_FIFO, __llvm_libc::getuid() == 0)
+LIST_SCHED_TESTS(SCHED_RR, __llvm_libc::getuid() == 0)
 
-    ASSERT_EQ(param.sched_priority, can_set ? priority : init_priority);
-  }
-  // Null test
-  ASSERT_EQ(__llvm_libc::sched_setscheduler(0, policy, nullptr), -1);
-  ASSERT_EQ(libc_errno, EINVAL);
-  libc_errno = 0;
-}
+// No root is required to set these policies.
+LIST_SCHED_TESTS(SCHED_OTHER, true)
+LIST_SCHED_TESTS(SCHED_BATCH, true)
+LIST_SCHED_TESTS(SCHED_IDLE, true)
 
 TEST(LlvmLibcSchedParamAndSchedulerTest, NullParamTest) {
   libc_errno = 0;

More information about the libc-commits mailing list