You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/09/23 08:04:43 UTC

[1/2] mesos git commit: Added os::clone function in stout.

Repository: mesos
Updated Branches:
  refs/heads/master 11a2311f7 -> 3b1eeec85


Added os::clone function in stout.

Review: https://reviews.apache.org/r/38639


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f279cf2f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f279cf2f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f279cf2f

Branch: refs/heads/master
Commit: f279cf2f82541852b878bdd2e881fe24cd8d0a7b
Parents: 11a2311
Author: haosdent huang <ha...@gmail.com>
Authored: Tue Sep 22 23:02:54 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Sep 22 23:02:55 2015 -0700

----------------------------------------------------------------------
 .../3rdparty/stout/include/stout/os/linux.hpp   | 46 +++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/f279cf2f/3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp
index b994b13..15ca091 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp
@@ -28,6 +28,7 @@
 
 #include <stout/error.hpp>
 #include <stout/foreach.hpp>
+#include <stout/lambda.hpp>
 #include <stout/option.hpp>
 #include <stout/proc.hpp>
 #include <stout/result.hpp>
@@ -37,6 +38,49 @@
 
 namespace os {
 
+
+// Helper for clone() which expects an int(void*).
+static int childMain(void* _func)
+{
+  const lambda::function<int()>* func =
+    static_cast<const lambda::function<int()>*> (_func);
+
+  return (*func)();
+}
+
+
+inline pid_t clone(const lambda::function<int()>& func, int flags)
+{
+  // Stack for the child.
+  // - unsigned long long used for best alignment.
+  // - 8 MiB appears to be the default for "ulimit -s" on OSX and Linux.
+  //
+  // NOTE: We need to allocate the stack dynamically. This is because
+  // glibc's 'clone' will modify the stack passed to it, therefore the
+  // stack must NOT be shared as multiple 'clone's can be invoked
+  // simultaneously.
+  int stackSize = 8 * 1024 * 1024;
+  unsigned long long *stack =
+    new unsigned long long[stackSize/sizeof(unsigned long long)];
+
+  pid_t pid = ::clone(
+      childMain,
+      &stack[stackSize/sizeof(stack[0]) - 1],  // stack grows down.
+      flags,
+      (void*) &func);
+
+  // If CLONE_VM is not set, ::clone would create a process which runs in a
+  // separate copy of the memory space of the calling process. So we destroy the
+  // stack here to avoid memory leak. If CLONE_VM is set, ::clone would create a
+  // thread which runs in the same memory space with the calling process.
+  if (!(flags & CLONE_VM)) {
+    delete[] stack;
+  }
+
+  return pid;
+}
+
+
 inline Result<Process> process(pid_t pid)
 {
   // Page size, used for memory accounting.
@@ -91,7 +135,7 @@ inline Result<Process> process(pid_t pid)
 }
 
 
-inline Try<std::set<pid_t> > pids()
+inline Try<std::set<pid_t>> pids()
 {
   return proc::pids();
 }


[2/2] mesos git commit: Replaced clone system call with os::clone.

Posted by ji...@apache.org.
Replaced clone system call with os::clone.

Review: https://reviews.apache.org/r/38625


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3b1eeec8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3b1eeec8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3b1eeec8

Branch: refs/heads/master
Commit: 3b1eeec85e51d548d9152da838a9cddebd14cf17
Parents: f279cf2
Author: haosdent huang <ha...@gmail.com>
Authored: Tue Sep 22 23:03:41 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Sep 22 23:03:41 2015 -0700

----------------------------------------------------------------------
 src/slave/containerizer/linux_launcher.cpp | 55 ++++---------------------
 src/tests/containerizer/launch_tests.cpp   | 22 +---------
 src/tests/containerizer/ns_tests.cpp       | 52 +++--------------------
 3 files changed, 13 insertions(+), 116 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/slave/containerizer/linux_launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/linux_launcher.cpp b/src/slave/containerizer/linux_launcher.cpp
index fd0ffcf..459af1b 100644
--- a/src/slave/containerizer/linux_launcher.cpp
+++ b/src/slave/containerizer/linux_launcher.cpp
@@ -162,53 +162,6 @@ Future<hashset<ContainerID>> LinuxLauncher::recover(
 }
 
 
-// Helper for clone() which expects an int(void*).
-static int childMain(void* _func)
-{
-  const lambda::function<int()>* func =
-    static_cast<const lambda::function<int()>*> (_func);
-
-  return (*func)();
-}
-
-
-// The customized clone function which will be used by 'subprocess()'.
-static pid_t clone(
-    const lambda::function<int()>& func,
-    const Option<int>& namespaces)
-{
-  // Stack for the child.
-  // - unsigned long long used for best alignment.
-  // - static is ok because each child gets their own copy after the clone.
-  // - 8 MiB appears to be the default for "ulimit -s" on OSX and Linux.
-  //
-  // NOTE: We need to allocate the stack dynamically. This is because
-  // glibc's 'clone' will modify the stack passed to it, therefore the
-  // stack must NOT be shared as multiple 'clone's can be invoked
-  // simultaneously.
-  int stackSize = 8 * 1024 * 1024;
-
-  unsigned long long *stack =
-    new unsigned long long[stackSize/sizeof(unsigned long long)];
-
-  int flags = namespaces.isSome() ? namespaces.get() : 0;
-  flags |= SIGCHLD; // Specify SIGCHLD as child termination signal.
-
-  LOG(INFO) << "Cloning child process with flags = "
-            << ns::stringify(flags);
-
-  pid_t pid = ::clone(
-      childMain,
-      &stack[stackSize/sizeof(stack[0]) - 1],  // stack grows down.
-      flags,
-      (void*) &func);
-
-  delete[] stack;
-
-  return pid;
-}
-
-
 static int childSetup(
     int pipes[2],
     const Option<lambda::function<int()>>& setup)
@@ -284,6 +237,12 @@ Try<pid_t> LinuxLauncher::fork(
   // use CHECK.
   CHECK_EQ(0, ::pipe(pipes));
 
+  int cloneFlags = namespaces.isSome() ? namespaces.get() : 0;
+  cloneFlags |= SIGCHLD; // Specify SIGCHLD as child termination signal.
+
+  LOG(INFO) << "Cloning child process with flags = "
+            << ns::stringify(cloneFlags);
+
   Try<Subprocess> child = subprocess(
       path,
       argv,
@@ -293,7 +252,7 @@ Try<pid_t> LinuxLauncher::fork(
       flags,
       environment,
       lambda::bind(&childSetup, pipes, setup),
-      lambda::bind(&clone, lambda::_1, namespaces));
+      lambda::bind(&os::clone, lambda::_1, cloneFlags));
 
   if (child.isError()) {
     return Error("Failed to clone child process: " + child.error());

http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/tests/containerizer/launch_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launch_tests.cpp b/src/tests/containerizer/launch_tests.cpp
index dbe891c..de655ec 100644
--- a/src/tests/containerizer/launch_tests.cpp
+++ b/src/tests/containerizer/launch_tests.cpp
@@ -81,33 +81,13 @@ public:
         launchFlags,
         None(),
         None(),
-        lambda::bind(&clone, lambda::_1));
+        lambda::bind(&os::clone, lambda::_1, CLONE_NEWNS | SIGCHLD));
 
     close(launchFlags.pipe_read.get());
     close(launchFlags.pipe_write.get());
 
     return s;
   }
-
-private:
-  static pid_t clone(const lambda::function<int()>& f)
-  {
-    static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
-    return ::clone(
-        _clone,
-        &stack[sizeof(stack)/sizeof(stack[0]) - 1],  // Stack grows down.
-        CLONE_NEWNS | SIGCHLD,   // Specify SIGCHLD as child termination signal.
-        (void*) &f);
-  }
-
-  static int _clone(void* f)
-  {
-    const lambda::function<int()>* _f =
-      static_cast<const lambda::function<int()>*> (f);
-
-    return (*_f)();
-  }
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/tests/containerizer/ns_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/ns_tests.cpp b/src/tests/containerizer/ns_tests.cpp
index 3a22938..ac3c9ff 100644
--- a/src/tests/containerizer/ns_tests.cpp
+++ b/src/tests/containerizer/ns_tests.cpp
@@ -56,32 +56,6 @@ namespace internal {
 namespace tests {
 
 
-// Helper for cloneChild() which expects an int(void*).
-static int cloneChildHelper(void* _func)
-{
-  const lambda::function<int()>* func =
-    static_cast<const lambda::function<int()>*> (_func);
-
-  return (*func)();
-}
-
-
-static pid_t cloneChild(
-    int flags,
-    const lambda::function<int()>& func)
-
-{
-  // 8 MiB stack for child.
-  static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
-  return ::clone(
-      cloneChildHelper,
-      &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
-      flags | SIGCHLD,
-      (void*) &func);
-}
-
-
 // Test that a child in different namespace(s) can setns back to the
 // root namespace. We must fork a child to test this because setns
 // doesn't support multi-threaded processes (which gtest is).
@@ -121,7 +95,7 @@ TEST(NsTest, ROOT_setns)
       None(),
       None(),
       None(),
-      lambda::bind(&cloneChild, flags, lambda::_1));
+      lambda::bind(&os::clone, lambda::_1, flags | SIGCHLD));
 
   // Continue in parent.
   ASSERT_SOME(s);
@@ -164,9 +138,7 @@ TEST(NsTest, ROOT_setnsMultipleThreads)
 }
 
 
-// Use a different child function for clone because it requires
-// int(*)(void*).
-static int childGetns(void* arg)
+static int childGetns()
 {
   // Sleep until killed.
   while (true) { sleep(1); }
@@ -192,14 +164,7 @@ TEST(NsTest, ROOT_getns)
   Try<int> nstype = ns::nstype(ns);
   ASSERT_SOME(nstype);
 
-  // 8 MiB stack for child.
-  static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
-  pid_t pid = clone(
-      childGetns,
-      &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
-      SIGCHLD | nstype.get(),
-      NULL);
+  pid_t pid = os::clone(childGetns, SIGCHLD | nstype.get());
 
   ASSERT_NE(-1, pid);
 
@@ -224,7 +189,7 @@ TEST(NsTest, ROOT_getns)
 }
 
 
-static int childDestroy(void* arg)
+static int childDestroy()
 {
   // Fork a bunch of children.
   ::fork();
@@ -251,14 +216,7 @@ TEST(NsTest, ROOT_destroy)
   Try<int> nstype = ns::nstype("pid");
   ASSERT_SOME(nstype);
 
-  // 8 MiB stack for child.
-  static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
-  pid_t pid = clone(
-      childDestroy,
-      &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
-      SIGCHLD | nstype.get(),
-      NULL);
+  pid_t pid = os::clone(childDestroy, SIGCHLD | nstype.get());
 
   ASSERT_NE(-1, pid);