[PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

classic Classic list List threaded Threaded
36 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

torbenh
---
 configure.ac        |   10 ++++++++++
 jack/internal.h     |    2 ++
 jackd/Makefile.am   |    2 +-
 jackd/engine.c      |   27 +++++++++++++++++++++++++++
 libjack/Makefile.am |    2 +-
 libjack/client.c    |   12 ++++++++++++
 libjack/thread.c    |   17 +++++++++++++++++
 7 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 86faa7b..a394557 100644
--- a/configure.ac
+++ b/configure.ac
@@ -753,6 +753,16 @@ fi
 AC_SUBST(NETJACK_LIBS)
 AC_SUBST(NETJACK_CFLAGS)
 
+PKG_CHECK_MODULES(CGROUP, libcgroup,[HAVE_CGROUP=true], [HAVE_CGROUP=false])
+if test x$HAVE_CGROUP = xfalse; then
+ AC_DEFINE(HAVE_CGROUP,0,"Whether libcgroup is available")
+else
+ AC_DEFINE(HAVE_CGROUP,1,"Whether libcgroup is available")
+fi
+
+AC_SUBST(CGROUP_LIBS)
+AC_SUBST(CGROUP_CFLAGS)
+
 # Note: A bug in pkg-config causes problems if the first occurence of
 # PKG_CHECK_MODULES can be disabled. So, if you're going to use
 # PKG_CHECK_MODULES inside a --disable-whatever check, you need to
diff --git a/jack/internal.h b/jack/internal.h
index c6107c8..ad164fe 100644
--- a/jack/internal.h
+++ b/jack/internal.h
@@ -199,6 +199,8 @@ typedef struct {
     float  max_delayed_usecs;
     uint32_t  port_max;
     int32_t  engine_ok;
+    int32_t               cgroups_enabled;
+    char  current_cgroup[JACK_PORT_NAME_SIZE];
     jack_port_type_id_t  n_port_types;
     jack_port_type_info_t port_types[JACK_MAX_PORT_TYPES];
     jack_port_shared_t    ports[0];
diff --git a/jackd/Makefile.am b/jackd/Makefile.am
index adf5be5..8238289 100644
--- a/jackd/Makefile.am
+++ b/jackd/Makefile.am
@@ -51,7 +51,7 @@ libjackserver_la_SOURCES = engine.c clientengine.c transengine.c \
         ../libjack/midiport.c ../libjack/ringbuffer.c ../libjack/shm.c \
         ../libjack/thread.c ../libjack/time.c  ../libjack/transclient.c \
         ../libjack/unlock.c
-libjackserver_la_LIBADD  = simd.lo @OS_LDFLAGS@
+libjackserver_la_LIBADD  = simd.lo @OS_LDFLAGS@ $(CGROUP_LIBS)
 libjackserver_la_LDFLAGS  = -export-dynamic -version-info @JACK_SO_VERSION@
 
 simd.lo: $(srcdir)/../libjack/simd.c
diff --git a/jackd/engine.c b/jackd/engine.c
index 4f05bec..5cc2134 100644
--- a/jackd/engine.c
+++ b/jackd/engine.c
@@ -63,6 +63,10 @@
 
 #include "libjack/local.h"
 
+#if HAVE_CGROUP
+#include <libcgroup.h>
+#endif
+
 typedef struct {
 
     jack_port_internal_t *source;
@@ -1708,6 +1712,26 @@ jack_server_thread (void *arg)
  return 0;
 }
 
+#if HAVE_CGROUP
+void jack_engine_setup_cgroup_info (jack_engine_t *engine)
+{
+ int res = cgroup_init();
+
+ if (res==0) {
+ char *current_controller;
+ cgroup_get_current_controller_path (getpid(), "cpu", &current_controller);
+
+ VERBOSE (engine, "current_cgroup = %s\n", current_controller);
+
+ strncpy (engine->control->current_cgroup, current_controller, sizeof(engine->control->current_cgroup));
+ engine->control->cgroups_enabled = 1;
+ } else {
+ jack_error ("libcgroup failed to initialise, error %d trying without", res);
+ engine->control->cgroups_enabled = 0;
+ }
+}
+#endif
+
 jack_engine_t *
 jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock,
  const char *server_name, int temporary, int verbose,
@@ -1962,6 +1986,9 @@ jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock,
 #endif /* JACK_USE_MACH_THREADS */
         
         
+#if HAVE_CGROUP
+ jack_engine_setup_cgroup_info (engine);
+#endif
 #ifdef USE_CAPABILITIES
  if (uid == 0 || euid == 0) {
  VERBOSE (engine, "running with uid=%d and euid=%d, "
diff --git a/libjack/Makefile.am b/libjack/Makefile.am
index 178616f..d4569cb 100644
--- a/libjack/Makefile.am
+++ b/libjack/Makefile.am
@@ -37,6 +37,6 @@ AM_CXXFLAGS = $(JACK_CFLAGS)
 libjack_la_CFLAGS = $(AM_CFLAGS)
 
 libjack_la_SOURCES = $(SOURCE_FILES)
-libjack_la_LIBADD  = simd.lo @OS_LDFLAGS@
+libjack_la_LIBADD  = simd.lo @OS_LDFLAGS@ $(CGROUP_LIBS)
 libjack_la_LDFLAGS  = -export-dynamic -version-info @JACK_SO_VERSION@
 
diff --git a/libjack/client.c b/libjack/client.c
index de0048a..a06edaa 100644
--- a/libjack/client.c
+++ b/libjack/client.c
@@ -62,6 +62,10 @@
 #include <sysdeps/pThreadUtilities.h>
 #endif
 
+#if HAVE_CGROUP
+#include <libcgroup.h>
+#endif
+
 static pthread_mutex_t client_lock;
 static pthread_cond_t  client_ready;
 
@@ -1123,6 +1127,14 @@ jack_client_open_aux (const char *client_name,
  client->control = (jack_client_control_t *)
  jack_shm_addr (&client->control_shm);
 
+#if HAVE_CGROUP
+ if (client->engine->cgroups_enabled) {
+ int res = cgroup_init ();
+ if (res != 0) {
+ jack_error ("error initialising libcgroups: %d", res);
+ }
+ }
+#endif
  /* Nobody else needs to access this shared memory any more, so
  * destroy it.  Because we have it attached, it won't vanish
  * till we exit (and release it).
diff --git a/libjack/thread.c b/libjack/thread.c
index 9bae2d0..3501962 100644
--- a/libjack/thread.c
+++ b/libjack/thread.c
@@ -39,6 +39,12 @@
 #include <sysdeps/pThreadUtilities.h>
 #endif
 
+#if HAVE_CGROUP
+#include <sys/types.h>
+#include <sys/syscall.h>
+#include <libcgroup.h>
+#endif
+
 jack_thread_creator_t jack_thread_creator = pthread_create;
 
 void
@@ -97,6 +103,17 @@ maybe_get_capabilities (jack_client_t* client)
  }
  }
 #endif /* USE_CAPABILITIES */
+#if HAVE_CGROUP
+ if (client != 0) {
+ int res;
+ const char * const controllers [] = { "cpu", NULL };
+
+ res = cgroup_change_cgroup_path (client->engine->current_cgroup, syscall( SYS_gettid ), controllers);
+ if (res != 0) {
+ jack_error ("failed to join cgroup %s : error %d\n", client->engine->current_cgroup, res);
+ }
+ }
+#endif
 }
 
 static void*
--
1.7.2.3

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

torbenh

i am assuming you understand what cgroups are. there is an excellent
explanation in the above thread.

this patch moves client RT threads into the same cgroup jackd was in.
we just use the facilities libcgroup provides.
(this is an optional dependency, but the cause of the problem was libcgroup,
 then we can also use it to fix the problem)

there are still a few pitfalls hiding. jackd might be inside a cgroup
where the current user doesnt have permission to move tasks into.

and the problem that the cgroups are not configured properly still needs
to be solved by the user.




On Thu, Dec 16, 2010 at 03:16:11AM +0100, Torben Hohn wrote:

> ---
>  configure.ac        |   10 ++++++++++
>  jack/internal.h     |    2 ++
>  jackd/Makefile.am   |    2 +-
>  jackd/engine.c      |   27 +++++++++++++++++++++++++++
>  libjack/Makefile.am |    2 +-
>  libjack/client.c    |   12 ++++++++++++
>  libjack/thread.c    |   17 +++++++++++++++++
>  7 files changed, 70 insertions(+), 2 deletions(-)
>

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

Paul Davis
On Wed, Dec 15, 2010 at 9:52 PM, torbenh <[hidden email]> wrote:
>
> i am assuming you understand what cgroups are. there is an excellent
> explanation in the above thread.
>
> this patch moves client RT threads into the same cgroup jackd was in.
> we just use the facilities libcgroup provides.

seems to me that

    jackd ... -g <cgroup-name> ....

to tell jackd what group to join as its start up would be somewhat
helpful, to avoid having to use cgexec or a script to start it up.
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

torbenh
On Thu, Dec 16, 2010 at 07:21:11AM -0500, Paul Davis wrote:

> On Wed, Dec 15, 2010 at 9:52 PM, torbenh <[hidden email]> wrote:
> >
> > i am assuming you understand what cgroups are. there is an excellent
> > explanation in the above thread.
> >
> > this patch moves client RT threads into the same cgroup jackd was in.
> > we just use the facilities libcgroup provides.
>
> seems to me that
>
>     jackd ... -g <cgroup-name> ....
>
> to tell jackd what group to join as its start up would be somewhat
> helpful, to avoid having to use cgexec or a script to start it up.

yes, we can add this.
but we still dont have a sane default for this option.

more on this shortly.

> _______________________________________________
> Jack-Devel mailing list
> [hidden email]
> http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

Dhaval Giani
In reply to this post by torbenh
On Thu, Dec 16, 2010 at 3:16 AM, Torben Hohn <[hidden email]> wrote:

> ---
>  configure.ac        |   10 ++++++++++
>  jack/internal.h     |    2 ++
>  jackd/Makefile.am   |    2 +-
>  jackd/engine.c      |   27 +++++++++++++++++++++++++++
>  libjack/Makefile.am |    2 +-
>  libjack/client.c    |   12 ++++++++++++
>  libjack/thread.c    |   17 +++++++++++++++++
>  7 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 86faa7b..a394557 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -753,6 +753,16 @@ fi
>  AC_SUBST(NETJACK_LIBS)
>  AC_SUBST(NETJACK_CFLAGS)
>
> +PKG_CHECK_MODULES(CGROUP, libcgroup,[HAVE_CGROUP=true], [HAVE_CGROUP=false])
> +if test x$HAVE_CGROUP = xfalse; then
> +       AC_DEFINE(HAVE_CGROUP,0,"Whether libcgroup is available")
> +else
> +       AC_DEFINE(HAVE_CGROUP,1,"Whether libcgroup is available")
> +fi
> +
> +AC_SUBST(CGROUP_LIBS)
> +AC_SUBST(CGROUP_CFLAGS)
> +
>  # Note: A bug in pkg-config causes problems if the first occurence of
>  # PKG_CHECK_MODULES can be disabled. So, if you're going to use
>  # PKG_CHECK_MODULES inside a --disable-whatever check, you need to
> diff --git a/jack/internal.h b/jack/internal.h
> index c6107c8..ad164fe 100644
> --- a/jack/internal.h
> +++ b/jack/internal.h
> @@ -199,6 +199,8 @@ typedef struct {
>     float                max_delayed_usecs;
>     uint32_t             port_max;
>     int32_t              engine_ok;
> +    int32_t               cgroups_enabled;
> +    char                 current_cgroup[JACK_PORT_NAME_SIZE];
>     jack_port_type_id_t          n_port_types;
>     jack_port_type_info_t port_types[JACK_MAX_PORT_TYPES];
>     jack_port_shared_t    ports[0];
> diff --git a/jackd/Makefile.am b/jackd/Makefile.am
> index adf5be5..8238289 100644
> --- a/jackd/Makefile.am
> +++ b/jackd/Makefile.am
> @@ -51,7 +51,7 @@ libjackserver_la_SOURCES = engine.c clientengine.c transengine.c \
>         ../libjack/midiport.c ../libjack/ringbuffer.c ../libjack/shm.c \
>         ../libjack/thread.c ../libjack/time.c  ../libjack/transclient.c \
>         ../libjack/unlock.c
> -libjackserver_la_LIBADD  = simd.lo @OS_LDFLAGS@
> +libjackserver_la_LIBADD  = simd.lo @OS_LDFLAGS@ $(CGROUP_LIBS)
>  libjackserver_la_LDFLAGS  = -export-dynamic -version-info @JACK_SO_VERSION@
>
>  simd.lo: $(srcdir)/../libjack/simd.c
> diff --git a/jackd/engine.c b/jackd/engine.c
> index 4f05bec..5cc2134 100644
> --- a/jackd/engine.c
> +++ b/jackd/engine.c
> @@ -63,6 +63,10 @@
>
>  #include "libjack/local.h"
>
> +#if HAVE_CGROUP
> +#include <libcgroup.h>
> +#endif
> +
>  typedef struct {
>
>     jack_port_internal_t *source;
> @@ -1708,6 +1712,26 @@ jack_server_thread (void *arg)
>        return 0;
>  }
>
> +#if HAVE_CGROUP
> +void jack_engine_setup_cgroup_info (jack_engine_t *engine)
> +{
> +       int res = cgroup_init();
> +
> +       if (res==0) {
> +               char *current_controller;
> +               cgroup_get_current_controller_path (getpid(), "cpu", &current_controller);
> +
> +               VERBOSE (engine, "current_cgroup = %s\n", current_controller);
> +
> +               strncpy (engine->control->current_cgroup, current_controller, sizeof(engine->control->current_cgroup));
> +               engine->control->cgroups_enabled = 1;
> +       } else {
> +               jack_error ("libcgroup failed to initialise, error %d trying without", res);
> +               engine->control->cgroups_enabled = 0;
> +       }
> +}
> +#endif
> +
>  jack_engine_t *
>  jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock,
>                 const char *server_name, int temporary, int verbose,
> @@ -1962,6 +1986,9 @@ jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock,
>  #endif /* JACK_USE_MACH_THREADS */
>
>
> +#if HAVE_CGROUP
> +       jack_engine_setup_cgroup_info (engine);
> +#endif
>  #ifdef USE_CAPABILITIES
>        if (uid == 0 || euid == 0) {
>                VERBOSE (engine, "running with uid=%d and euid=%d, "
> diff --git a/libjack/Makefile.am b/libjack/Makefile.am
> index 178616f..d4569cb 100644
> --- a/libjack/Makefile.am
> +++ b/libjack/Makefile.am
> @@ -37,6 +37,6 @@ AM_CXXFLAGS = $(JACK_CFLAGS)
>  libjack_la_CFLAGS = $(AM_CFLAGS)
>
>  libjack_la_SOURCES = $(SOURCE_FILES)
> -libjack_la_LIBADD  = simd.lo @OS_LDFLAGS@
> +libjack_la_LIBADD  = simd.lo @OS_LDFLAGS@ $(CGROUP_LIBS)
>  libjack_la_LDFLAGS  = -export-dynamic -version-info @JACK_SO_VERSION@
>
> diff --git a/libjack/client.c b/libjack/client.c
> index de0048a..a06edaa 100644
> --- a/libjack/client.c
> +++ b/libjack/client.c
> @@ -62,6 +62,10 @@
>  #include <sysdeps/pThreadUtilities.h>
>  #endif
>
> +#if HAVE_CGROUP
> +#include <libcgroup.h>
> +#endif
> +
>  static pthread_mutex_t client_lock;
>  static pthread_cond_t  client_ready;
>
> @@ -1123,6 +1127,14 @@ jack_client_open_aux (const char *client_name,
>        client->control = (jack_client_control_t *)
>                jack_shm_addr (&client->control_shm);
>
> +#if HAVE_CGROUP
> +       if (client->engine->cgroups_enabled) {
> +               int res = cgroup_init ();

hmm. So are the daemon and client not going to share anything at all?
If they are sharing address space, I think this will fail (because it
is alrady initialized).

> +               if (res != 0) {
> +                       jack_error ("error initialising libcgroups: %d", res);
> +               }
> +       }
> +#endif
>        /* Nobody else needs to access this shared memory any more, so
>         * destroy it.  Because we have it attached, it won't vanish
>         * till we exit (and release it).
> diff --git a/libjack/thread.c b/libjack/thread.c
> index 9bae2d0..3501962 100644
> --- a/libjack/thread.c
> +++ b/libjack/thread.c
> @@ -39,6 +39,12 @@
>  #include <sysdeps/pThreadUtilities.h>
>  #endif
>
> +#if HAVE_CGROUP
> +#include <sys/types.h>
> +#include <sys/syscall.h>
> +#include <libcgroup.h>
> +#endif
> +
>  jack_thread_creator_t jack_thread_creator = pthread_create;
>
>  void
> @@ -97,6 +103,17 @@ maybe_get_capabilities (jack_client_t* client)
>                }
>        }
>  #endif /* USE_CAPABILITIES */
> +#if HAVE_CGROUP
> +       if (client != 0) {
> +               int res;
> +               const char * const controllers [] = { "cpu", NULL };
> +
> +               res = cgroup_change_cgroup_path (client->engine->current_cgroup, syscall( SYS_gettid ), controllers);
> +               if (res != 0) {
> +                       jack_error ("failed to join cgroup %s : error %d\n", client->engine->current_cgroup, res);
> +               }
> +       }
> +#endif
>  }
>


Seems quite sane apart from the comment I had. Would like to give it a
run though :-)

Thanks!
Dhaval
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

torbenh
On Fri, Dec 17, 2010 at 03:42:50PM +0100, Dhaval Giani wrote:

> On Thu, Dec 16, 2010 at 3:16 AM, Torben Hohn <[hidden email]> wrote:
> > ---
> > +++ b/libjack/client.c
> > @@ -62,6 +62,10 @@
> >  #include <sysdeps/pThreadUtilities.h>
> >  #endif
> >
> > +#if HAVE_CGROUP
> > +#include <libcgroup.h>
> > +#endif
> > +
> >  static pthread_mutex_t client_lock;
> >  static pthread_cond_t  client_ready;
> >
> > @@ -1123,6 +1127,14 @@ jack_client_open_aux (const char *client_name,
> >        client->control = (jack_client_control_t *)
> >                jack_shm_addr (&client->control_shm);
> >
> > +#if HAVE_CGROUP
> > +       if (client->engine->cgroups_enabled) {
> > +               int res = cgroup_init ();
>
> hmm. So are the daemon and client not going to share anything at all?
> If they are sharing address space, I think this will fail (because it
> is alrady initialized).

client->engine  is a structure in shm.
that is shared.
but the rest of the address space is not shared.

this part works fine. dont worry about it :)
we need a few tweaks for apps that create multiple jackclients.
in order to not call cgroup_init more than once.
i am going to do the finishing touches once i have my mainsystem using
the cgroups.

>
> > +               if (res != 0) {
> > +                       jack_error ("error initialising libcgroups: %d", res);
> > +               }
> > +       }
> > +#endif
> >        /* Nobody else needs to access this shared memory any more, so
> >         * destroy it.  Because we have it attached, it won't vanish
> >         * till we exit (and release it).
> > diff --git a/libjack/thread.c b/libjack/thread.c
> > index 9bae2d0..3501962 100644
> > --- a/libjack/thread.c
> > +++ b/libjack/thread.c
> > @@ -39,6 +39,12 @@
> >  #include <sysdeps/pThreadUtilities.h>
> >  #endif
> >
> > +#if HAVE_CGROUP
> > +#include <sys/types.h>
> > +#include <sys/syscall.h>
> > +#include <libcgroup.h>
> > +#endif
> > +
> >  jack_thread_creator_t jack_thread_creator = pthread_create;
> >
> >  void
> > @@ -97,6 +103,17 @@ maybe_get_capabilities (jack_client_t* client)
> >                }
> >        }
> >  #endif /* USE_CAPABILITIES */
> > +#if HAVE_CGROUP
> > +       if (client != 0) {
> > +               int res;
> > +               const char * const controllers [] = { "cpu", NULL };
> > +
> > +               res = cgroup_change_cgroup_path (client->engine->current_cgroup, syscall( SYS_gettid ), controllers);
> > +               if (res != 0) {
> > +                       jack_error ("failed to join cgroup %s : error %d\n", client->engine->current_cgroup, res);
> > +               }
> > +       }
> > +#endif
> >  }
> >
>
>
> Seems quite sane apart from the comment I had. Would like to give it a
> run though :-)

its in git://hochstrom.endofinternet.org/jack.git cgroups branch

>
> Thanks!
> Dhaval

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

torbenh
In reply to this post by torbenh
On Fri, Dec 17, 2010 at 03:24:45PM +0100, torbenh wrote:

> On Thu, Dec 16, 2010 at 07:21:11AM -0500, Paul Davis wrote:
> > On Wed, Dec 15, 2010 at 9:52 PM, torbenh <[hidden email]> wrote:
> > >
> > > i am assuming you understand what cgroups are. there is an excellent
> > > explanation in the above thread.
> > >
> > > this patch moves client RT threads into the same cgroup jackd was in.
> > > we just use the facilities libcgroup provides.
> >
> > seems to me that
> >
> >     jackd ... -g <cgroup-name> ....
> >
> > to tell jackd what group to join as its start up would be somewhat
> > helpful, to avoid having to use cgexec or a script to start it up.
>
> yes, we can add this.
> but we still dont have a sane default for this option.
>
> more on this shortly.

i am not sure, if we do actually want to move the whole jack process
into the rt cgroup.
if this is not the case, we need to change the architecture for creating
RT threads inside the engine a bit. (it needs to get split from the
libjack side, because we need to provide an engine pointer, so the
thread knows where to move)

i am going to move the whole process there first.


on a side note, how are things on *BSD ?
would we get a libcgroup implementation there too ?
or do we need to do some platform abstraction ?

>
> > _______________________________________________
> > Jack-Devel mailing list
> > [hidden email]
> > http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
>
> --
> torben Hohn
> _______________________________________________
> Jack-Devel mailing list
> [hidden email]
> http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

Dhaval Giani
> on a side note, how are things on *BSD ?
> would we get a libcgroup implementation there too ?
> or do we need to do some platform abstraction ?
>

cgruops is a linux feature. So no, there is no libcgroup for BSD
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

[PATCH] [cgroup] add -g option to join a specific cgroup

torbenh
In reply to this post by torbenh
---
 jack/engine.h    |    2 +-
 jackd/engine.c   |   29 +++++++++++++++++++++--------
 jackd/jackd.1.in |    4 ++++
 jackd/jackd.c    |    9 +++++++--
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/jack/engine.h b/jack/engine.h
index 1884690..6b70549 100644
--- a/jack/engine.h
+++ b/jack/engine.h
@@ -180,7 +180,7 @@ jack_engine_t  *jack_engine_new (int real_time, int real_time_priority,
  const char *server_name, int temporary,
  int verbose, int client_timeout,
  unsigned int port_max,
-                                 pid_t waitpid, jack_nframes_t frame_time_offset, int nozombies,
+                                 pid_t waitpid, jack_nframes_t frame_time_offset, int nozombies, char *cgroup_name,
  JSList *drivers);
 void jack_engine_delete (jack_engine_t *);
 int jack_run (jack_engine_t *engine);
diff --git a/jackd/engine.c b/jackd/engine.c
index 5cc2134..3f12cfe 100644
--- a/jackd/engine.c
+++ b/jackd/engine.c
@@ -1713,18 +1713,31 @@ jack_server_thread (void *arg)
 }
 
 #if HAVE_CGROUP
-void jack_engine_setup_cgroup_info (jack_engine_t *engine)
+static void
+jack_engine_setup_cgroup_info (jack_engine_t *engine, char *cgroup_name)
 {
+ const char * const controllers [] = { "cpu", NULL };
  int res = cgroup_init();
 
  if (res==0) {
  char *current_controller;
- cgroup_get_current_controller_path (getpid(), "cpu", &current_controller);
-
- VERBOSE (engine, "current_cgroup = %s\n", current_controller);
+ if (cgroup_name) {
+ res = cgroup_change_cgroup_path (cgroup_name, getpid(), controllers);
+ if (res != 0) {
+ jack_error ("failed to join cgroup %s : error %d\n", cgroup_name, res);
+ engine->control->cgroups_enabled = 0;
+ } else {
+ VERBOSE (engine, "joined cgroup %s\n", cgroup_name);
+ strncpy (engine->control->current_cgroup, cgroup_name, sizeof(engine->control->current_cgroup));
+ engine->control->cgroups_enabled = 1;
+ }
+ } else {
+ cgroup_get_current_controller_path (getpid(), "cpu", &current_controller);
+ VERBOSE (engine, "current_cgroup = %s\n", current_controller);
 
- strncpy (engine->control->current_cgroup, current_controller, sizeof(engine->control->current_cgroup));
- engine->control->cgroups_enabled = 1;
+ strncpy (engine->control->current_cgroup, current_controller, sizeof(engine->control->current_cgroup));
+ engine->control->cgroups_enabled = 1;
+ }
  } else {
  jack_error ("libcgroup failed to initialise, error %d trying without", res);
  engine->control->cgroups_enabled = 0;
@@ -1736,7 +1749,7 @@ jack_engine_t *
 jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock,
  const char *server_name, int temporary, int verbose,
  int client_timeout, unsigned int port_max, pid_t wait_pid,
- jack_nframes_t frame_time_offset, int nozombies, JSList *drivers)
+ jack_nframes_t frame_time_offset, int nozombies, char *cgroup_name, JSList *drivers)
 {
  jack_engine_t *engine;
  unsigned int i;
@@ -1987,7 +2000,7 @@ jack_engine_new (int realtime, int rtpriority, int do_mlock, int do_unlock,
         
         
 #if HAVE_CGROUP
- jack_engine_setup_cgroup_info (engine);
+ jack_engine_setup_cgroup_info (engine, cgroup_name);
 #endif
 #ifdef USE_CAPABILITIES
  if (uid == 0 || euid == 0) {
diff --git a/jackd/jackd.1.in b/jackd/jackd.1.in
index b634201..f39fcb4 100644
--- a/jackd/jackd.1.in
+++ b/jackd/jackd.1.in
@@ -99,6 +99,10 @@ Exit once all clients have closed their connections.
 Set client timeout limit in milliseconds.  The default is 500 msec.
 In realtime mode the client timeout must be smaller than the watchdog timeout (5000 msec).
 .TP
+\fB\-g, \-\-cgroup\fR \fIcgroup\-name\fR
+.br
+This option makes the jackd client join a specific cgroup. All client RT threads will also join this cgroup.
+.TP
 \fB\-Z, \-\-nozombies\fR
 .br
 Prevent JACK from ever kicking out clients because they were too slow.
diff --git a/jackd/jackd.c b/jackd/jackd.c
index d0a0c1f..1456815 100644
--- a/jackd/jackd.c
+++ b/jackd/jackd.c
@@ -67,6 +67,7 @@ static unsigned int port_max = 256;
 static int do_unlock = 0;
 static jack_nframes_t frame_time_offset = 0;
 static int nozombies = 0;
+static char * cgroup_name = NULL;
 
 extern int sanitycheck (int, int);
 
@@ -150,7 +151,7 @@ jack_main (jack_driver_desc_t * driver_desc, JSList * driver_params)
        do_mlock, do_unlock, server_name,
        temporary, verbose, client_timeout,
        port_max, getpid(), frame_time_offset,
-       nozombies, drivers)) == 0) {
+       nozombies, cgroup_name, drivers)) == 0) {
  jack_error ("cannot create engine");
  return -1;
  }
@@ -522,7 +523,7 @@ main (int argc, char *argv[])
  int do_sanity_checks = 1;
  int show_version = 0;
 
- const char *options = "-d:P:uvshVrRZTFlt:mM:n:Np:c:";
+ const char *options = "-d:P:uvshVrRZTFlt:mM:n:Np:c:g:";
  struct option long_options[] =
  {
  /* keep ordered by single-letter option code */
@@ -548,6 +549,7 @@ main (int argc, char *argv[])
  { "version", 0, 0, 'V' },
  { "verbose", 0, 0, 'v' },
  { "nozombies", 0, 0, 'Z' },
+ { "cgroup", 1, 0, 'g' },
  { 0, 0, 0, 0 }
  };
  int opt = 0;
@@ -662,6 +664,9 @@ main (int argc, char *argv[])
  case 'Z':
  nozombies = 1;
  break;
+ case 'g':
+ cgroup_name = optarg;
+ break;
 
  default:
  jack_error ("Unknown option character %c",
--
1.7.2.3

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

Hermann Meyer
Am Freitag, den 17.12.2010, 23:31 +0100 schrieb Torben Hohn:
> +This option makes the jackd client join a specific cgroup. All client
> RT threads will also join this cgroup.

Hi

Because I don't have look at or use cgroup for now, let me ask please,
your patch, did it mean that rt-threads created by clients will also
join the group, or is it related only for threads witch created by
jack?  
So do I need to add support for libcgroup in my app when it create his
own rt-threads outside the process_callback, and I wont to make it work
under a cgroup enabled environment or is that already covered by this
patch and there is nothig left to do for me ?

regards   hermann

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

Dhaval Giani
On Sat, Dec 18, 2010 at 7:53 AM, hermann <[hidden email]> wrote:

> Am Freitag, den 17.12.2010, 23:31 +0100 schrieb Torben Hohn:
>> +This option makes the jackd client join a specific cgroup. All client
>> RT threads will also join this cgroup.
>
> Hi
>
> Because I don't have look at or use cgroup for now, let me ask please,
> your patch, did it mean that rt-threads created by clients will also
> join the group, or is it related only for threads witch created by
> jack?
> So do I need to add support for libcgroup in my app when it create his
> own rt-threads outside the process_callback, and I wont to make it work
> under a cgroup enabled environment or is that already covered by this
> patch and there is nothig left to do for me ?
>

IIUC, you will need to move your task manually. Having said that, we
are still trying to figure out a mechanism which will not need changes
in clients. Give us a little bit of time, and we will have everything
working just fine!

Thanks!
Dhaval
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

Fons Adriaensen-2
On Sat, Dec 18, 2010 at 12:47:54PM +0100, Dhaval Giani wrote:

> IIUC, you will need to move your task manually. Having said that, we
> are still trying to figure out a mechanism which will not need changes
> in clients. Give us a little bit of time, and we will have everything
> working just fine!

That would be excellent. I hope 'not need changes in clients' means
'not need changes in audio apps requiring RT'. They all not all or
always Jack clients.


Ciao,

--
FA

There are three of them, and Alleline.

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

torbenh
In reply to this post by Hermann Meyer
On Sat, Dec 18, 2010 at 07:53:09AM +0100, hermann wrote:

> Am Freitag, den 17.12.2010, 23:31 +0100 schrieb Torben Hohn:
> > +This option makes the jackd client join a specific cgroup. All client
> > RT threads will also join this cgroup.
>
> Hi
>
> Because I don't have look at or use cgroup for now, let me ask please,
> your patch, did it mean that rt-threads created by clients will also
> join the group, or is it related only for threads witch created by
> jack?  
> So do I need to add support for libcgroup in my app when it create his
> own rt-threads outside the process_callback, and I wont to make it work
> under a cgroup enabled environment or is that already covered by this
> patch and there is nothig left to do for me ?

your app is a jack app right ?
just create your RT thread with jacks thread api, and it will be fine.

http://jackaudio.org/files/docs/html/group__ClientThreads.html#ga8bf1ab53777e5554dba60b72c8e1e42a

for non-jack apps, things will be different.
i hope we can provide a proper solution soon.

>
> regards   hermann
>

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

Hermann Meyer
In reply to this post by Dhaval Giani
Am Samstag, den 18.12.2010, 12:47 +0100 schrieb Dhaval Giani:
> IIUC, you will need to move your task manually. Having said that, we
> are still trying to figure out a mechanism which will not need changes
> in clients. Give us a little bit of time, and we will have everything
> working just fine!
>
> Thanks!
> Dhaval

Great, that will be very welcome. Please let us know when you have
found / or not/ a solution that will cover that.

regards  hermann

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

torbenh
In reply to this post by Dhaval Giani
On Sat, Dec 18, 2010 at 12:47:54PM +0100, Dhaval Giani wrote:

> On Sat, Dec 18, 2010 at 7:53 AM, hermann <[hidden email]> wrote:
> > Am Freitag, den 17.12.2010, 23:31 +0100 schrieb Torben Hohn:
> >> +This option makes the jackd client join a specific cgroup. All client
> >> RT threads will also join this cgroup.
> >
> > Hi
> >
> > Because I don't have look at or use cgroup for now, let me ask please,
> > your patch, did it mean that rt-threads created by clients will also
> > join the group, or is it related only for threads witch created by
> > jack?
> > So do I need to add support for libcgroup in my app when it create his
> > own rt-threads outside the process_callback, and I wont to make it work
> > under a cgroup enabled environment or is that already covered by this
> > patch and there is nothig left to do for me ?
> >
>
> IIUC, you will need to move your task manually. Having said that, we

if using jack, you can use jacks thread api, to create RT threads.

> are still trying to figure out a mechanism which will not need changes
> in clients. Give us a little bit of time, and we will have everything
> working just fine!

this mechanism would move the whole process into the RT cgroup.
it would be nice if we could provide something that allowed moving
individual threads.

doing this with current libcgroup is just a matter of 2 function calls.
we just need to make sure, that some rt cgroup is setup.
and we need a way to have the app know which cgroup to move to.



>
> Thanks!
> Dhaval

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

Hermann Meyer
In reply to this post by torbenh
Am Samstag, den 18.12.2010, 14:14 +0100 schrieb torbenh:

> your app is a jack app right ?
> just create your RT thread with jacks thread api, and it will be fine.
>
> http://jackaudio.org/files/docs/html/group__ClientThreads.html#ga8bf1ab53777e5554dba60b72c8e1e42a
>
> for non-jack apps, things will be different.
> i hope we can provide a proper solution soon.
>
Right, I have think already about that, when no other solution is coming
up, witch didn't require a interaction at all, I will do that.

Thanks  hermann


_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

Tommaso Cucinotta-2
In reply to this post by torbenh
Il 17/12/2010 17:15, torbenh ha scritto:
> we need a few tweaks for apps that create multiple jackclients.
> in order to not call cgroup_init more than once.
> i am going to do the finishing touches once i have my mainsystem using
> the cgroups.
One last question (I couldn't go through your patch, however): what
about those additional real-time threads that are provided by
"pthread_t" to the jack library ? Those cannot be attached to the proper
jack group by pthread_t (and we don't know what their Linux tid is), right ?
In those cases, what about pulling the whole client process in the jack
group ?

     T

--
Tommaso Cucinotta, Computer Engineering PhD, Researcher
ReTiS Lab, Scuola Superiore Sant'Anna, Pisa, Italy
Tel +39 050 882 024, Fax +39 050 882 003
http://retis.sssup.it/people/tommaso

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

Fons Adriaensen-2
In reply to this post by torbenh
On Sat, Dec 18, 2010 at 02:21:23PM +0100, torbenh wrote:

> if using jack, you can use jacks thread api, to create RT threads.

Not if the the threads are created inside a library which does
not depend on Jack and could well be used without it. Or within
any piece of code which does not otherwise depend on Jack.
On example is jconvolver, its internal threads are even invisible
to the user.

Also not if the threads are created by an existing C++ thread class
which is part of an multithreaded application framework and hence
can't be replaced without breaking everything.

> > are still trying to figure out a mechanism which will not need changes
> > in clients. Give us a little bit of time, and we will have everything
> > working just fine!
>
> this mechanism would move the whole process into the RT cgroup.

Which is what we need.

> it would be nice if we could provide something that allowed moving
> individual threads.

What would be the advantage, except being 'nice' ?

Ciao,

--
FA

There are three of them, and Alleline.

_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroup] add -g option to join a specific cgroup

torbenh
On Sat, Dec 18, 2010 at 11:33:15PM +0100, [hidden email] wrote:

> On Sat, Dec 18, 2010 at 02:21:23PM +0100, torbenh wrote:
>
> > if using jack, you can use jacks thread api, to create RT threads.
>
> Not if the the threads are created inside a library which does
> not depend on Jack and could well be used without it. Or within
> any piece of code which does not otherwise depend on Jack.
> On example is jconvolver, its internal threads are even invisible
> to the user.
>
> Also not if the threads are created by an existing C++ thread class
> which is part of an multithreaded application framework and hence
> can't be replaced without breaking everything.

maybe we need to expose this function then:

static void
maybe_get_capabilities (jack_client_t* client)

with a bit different semantics probably.

http://trac.jackaudio.org/browser/trunk/jack/libjack/thread.c#L60

note how its touched in the patch to move the current thread into the
cgroup. it still does not really solve your problem, when not depending
on jack. but at least you can just stick it into your threads init code.

we are just reacting to changes happening. i really dont think its
possible to stop them. we can only delay them a bit.

> > > are still trying to figure out a mechanism which will not need changes
> > > in clients. Give us a little bit of time, and we will have everything
> > > working just fine!
> >
> > this mechanism would move the whole process into the RT cgroup.
>
> Which is what we need.

why do you need that ?
i would prefer leaving the GUI threads in the cgroup where systemd will
have dumped them.
(except if we give a really big SCHED_OTHER share to RT cgroup)


>
> > it would be nice if we could provide something that allowed moving
> > individual threads.
>
> What would be the advantage, except being 'nice' ?

because if we only move whole processes, all RT apps gui threads would be in a
single cgroup, and firefox will probably end up having the same amount
of SCHED_OTHER slices as all RT guis combined.

>
> Ciao,
>
> --
> FA
>
> There are three of them, and Alleline.
>
> _______________________________________________
> Jack-Devel mailing list
> [hidden email]
> http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org

--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

torbenh
In reply to this post by Tommaso Cucinotta-2
On Sat, Dec 18, 2010 at 09:40:51PM +0100, Tommaso Cucinotta wrote:
> Il 17/12/2010 17:15, torbenh ha scritto:
> >we need a few tweaks for apps that create multiple jackclients.
> >in order to not call cgroup_init more than once.
> >i am going to do the finishing touches once i have my mainsystem using
> >the cgroups.
> One last question (I couldn't go through your patch, however): what
> about those additional real-time threads that are provided by
> "pthread_t" to the jack library ?

i dont understand this. nothing provides threads to the libjack library.
these threads are always created by the library itself.
the thread acquires sched_fifo itself. so....


> Those cannot be attached to the
> proper jack group by pthread_t (and we don't know what their Linux
> tid is), right ?

wrong. i just run gettid() in the current thread.

> In those cases, what about pulling the whole client process in the
> jack group ?

pulling the whole client process into the RT cgroup is wrong IMO.
if you think its correct to do this, please give compelling arguments,



--
torben Hohn
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
12