[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
|

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

Lennart Poettering-16
On Thu, 16.12.10 03:16, 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(-)

Hmmm, while PA is in a similar situation here I will not do a similar
patch for PA in this regard.

I believe the right place to fix this is the "cpu" cgroup controller,
and there is no point at all in fixing this in userspace. Even more,
it's actually impossible to do so correctly since you do not know how
many shares to assign your cgroup unless you assume you are the only
user of RT on the system, which might be an OK (though dirty) assumption
for JACK, but is definitely wrong for PA, since we must allow multi-user
support in PA.

I do believe that the short time fix for this problem is that
libcgroup/Ubuntu should stop sorting processes into any non-root cgroup
hierarchy by default. The long term fix is that the cpu controller needs
to be fixed, before one can make use of it in the general case.

So, yepp, I point the fingers here to the cgroup people: the controller
is broken, not JACK, nor PA. It should be fixed in the controller, and
and as long as it isn't the controller should not be used by default for
generic applications.

Lennart

--
Lennart Poettering - Red Hat, Inc.
_______________________________________________
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 Fri, Dec 24, 2010 at 9:46 AM, Lennart Poettering <[hidden email]> wrote:

> On Thu, 16.12.10 03:16, 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(-)
>
> Hmmm, while PA is in a similar situation here I will not do a similar
> patch for PA in this regard.
>
> I believe the right place to fix this is the "cpu" cgroup controller,
> and there is no point at all in fixing this in userspace. Even more,
> it's actually impossible to do so correctly since you do not know how
> many shares to assign your cgroup unless you assume you are the only
> user of RT on the system, which might be an OK (though dirty) assumption
> for JACK, but is definitely wrong for PA, since we must allow multi-user
> support in PA.
>
> I do believe that the short time fix for this problem is that
> libcgroup/Ubuntu should stop sorting processes into any non-root cgroup
> hierarchy by default.

Ubuntu needs to fix that.

> The long term fix is that the cpu controller needs
> to be fixed, before one can make use of it in the general case.
>
> So, yepp, I point the fingers here to the cgroup people: the controller
> is broken, not JACK, nor PA. It should be fixed in the controller, and
> and as long as it isn't the controller should not be used by default for
> generic applications.
>

Well, it works just fine. You don't get realtime if you don't have the
runtime. It works just fine for generic applications. It is when
real-time applications want to behave like generic applications things
go bad. I still hold to the view that realtime processes will need to
ensure they have rt_runtime available (either by being in a cgroup
that has it, or by creating a cgroup and giving it)/

Dhaval

FWIW, we are already looking at a solution by using the former option
(being in a cgroup that has rt_runtime) which does not need any code
changes (as per theory ;-) )
_______________________________________________
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

Lennart Poettering-16
On Fri, 24.12.10 09:53, Dhaval Giani ([hidden email]) wrote:

> > I do believe that the short time fix for this problem is that
> > libcgroup/Ubuntu should stop sorting processes into any non-root cgroup
> > hierarchy by default.
>
> Ubuntu needs to fix that.

Yes, they do. David?

> > The long term fix is that the cpu controller needs
> > to be fixed, before one can make use of it in the general case.
> >
> > So, yepp, I point the fingers here to the cgroup people: the controller
> > is broken, not JACK, nor PA. It should be fixed in the controller, and
> > and as long as it isn't the controller should not be used by default for
> > generic applications.
> >
>
> Well, it works just fine. You don't get realtime if you don't have the
> runtime. It works just fine for generic applications. It is when
> real-time applications want to behave like generic applications things
> go bad. I still hold to the view that realtime processes will need to
> ensure they have rt_runtime available (either by being in a cgroup
> that has it, or by creating a cgroup and giving it)/

Well, it doesn't work at all on Ubuntu, and afaics this is with blessing
from upstream libcgroup: unless you manually fiddle with the cpu
hierarchy all invocations of sched_setscheduler(FIFO/RR) will fail in
the default setup on Ubuntu. And there is no really good reason for this
change: simply moving everything into a different cgroup does not give
any benefit in scheduling, does it? However, it does not only break
JACK, it breaks *all* applications that might use RT. And that's why it
shouldn't be fixed in JACK, but in the cgroup logic itself.

> FWIW, we are already looking at a solution by using the former option
> (being in a cgroup that has rt_runtime) which does not need any code
> changes (as per theory ;-) )

Hmm, I don't follow. Could you elaborate?

Lennart

--
Lennart Poettering - Red Hat, Inc.
_______________________________________________
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
> Well, it doesn't work at all on Ubuntu, and afaics this is with blessing
> from upstream libcgroup: unless you manually fiddle with the cpu
> hierarchy all invocations of sched_setscheduler(FIFO/RR) will fail in
> the default setup on Ubuntu. And there is no really good reason for this
> change: simply moving everything into a different cgroup does not give
> any benefit in scheduling, does it?

Incorrect. It does. The moment you have the first cgroup come into the
picture. Without any additional cgroups, there is no change (except of
course for rt processes for which reason they are left in the root),
but the moment you have an additional cgroup come in, it gets the
bandwidth that would be expected by a user as opposed to as expected
by the system.

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

Lennart Poettering-16
On Fri, 24.12.10 10:18, Dhaval Giani ([hidden email]) wrote:

>
> > Well, it doesn't work at all on Ubuntu, and afaics this is with blessing
> > from upstream libcgroup: unless you manually fiddle with the cpu
> > hierarchy all invocations of sched_setscheduler(FIFO/RR) will fail in
> > the default setup on Ubuntu. And there is no really good reason for this
> > change: simply moving everything into a different cgroup does not give
> > any benefit in scheduling, does it?
>
> Incorrect. It does. The moment you have the first cgroup come into the
> picture. Without any additional cgroups, there is no change (except of
> course for rt processes for which reason they are left in the root),
> but the moment you have an additional cgroup come in, it gets the
> bandwidth that would be expected by a user as opposed to as expected
> by the system.

Well, but that other cgroup does not appear from nowhere, but requires
user configuration. Sorting everything into a new non-root cgroup alone
does not offer *any* benefits. If you do create additional groups and
hence modify configuration then of course you should think about what
you should do with the remaining processes, but only then.

Hence I am saying: the current Ubuntu config does not offer any
benefits: it is half-baked. If people do not create their own additional
cgroups they have no beenfits from this default config. And if they do
it they need to reconfigure things anyway in which case they can easily
enable the default cgroup, too.

"cpu" as it is now, is not really usable in any setup that we could ship
by default. It is only useful if people actually manually configure
something.

Lennart

--
Lennart Poettering - Red Hat, Inc.
_______________________________________________
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 Fri, Dec 24, 2010 at 10:35 AM, Lennart Poettering <[hidden email]> wrote:

> On Fri, 24.12.10 10:18, Dhaval Giani ([hidden email]) wrote:
>
>>
>> > Well, it doesn't work at all on Ubuntu, and afaics this is with blessing
>> > from upstream libcgroup: unless you manually fiddle with the cpu
>> > hierarchy all invocations of sched_setscheduler(FIFO/RR) will fail in
>> > the default setup on Ubuntu. And there is no really good reason for this
>> > change: simply moving everything into a different cgroup does not give
>> > any benefit in scheduling, does it?
>>
>> Incorrect. It does. The moment you have the first cgroup come into the
>> picture. Without any additional cgroups, there is no change (except of
>> course for rt processes for which reason they are left in the root),
>> but the moment you have an additional cgroup come in, it gets the
>> bandwidth that would be expected by a user as opposed to as expected
>> by the system.
>
> Well, but that other cgroup does not appear from nowhere, but requires
> user configuration. Sorting everything into a new non-root cgroup alone
> does not offer *any* benefits. If you do create additional groups and
> hence modify configuration then of course you should think about what
> you should do with the remaining processes, but only then.
>

Which is also why Fedora does not enable it. This configuration option
is needed for some other applications which do use cgroups in a sane
fashion.

> Hence I am saying: the current Ubuntu config does not offer any
> benefits: it is half-baked. If people do not create their own additional
> cgroups they have no beenfits from this default config. And if they do
> it they need to reconfigure things anyway in which case they can easily
> enable the default cgroup, too.
>

Right, so Ubuntu needs to fix this.

> "cpu" as it is now, is not really usable in any setup that we could ship
> by default. It is only useful if people actually manually configure
> something.

Which is why you did the default cgroup grouping patch, and Mike did
the autogrouping patch.

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

Lennart Poettering-16
On Fri, 24.12.10 10:38, Dhaval Giani ([hidden email]) wrote:

> > "cpu" as it is now, is not really usable in any setup that we could ship
> > by default. It is only useful if people actually manually configure
> > something.
>
> Which is why you did the default cgroup grouping patch, and Mike did
> the autogrouping patch.

Well, but as it turns out now my "4 line bash patch" as known from
slashdot apparently breaks RT, and I am wondering whether the
auto-grouping kernel patch might break RT too.

Lennart

--
Lennart Poettering - Red Hat, Inc.
_______________________________________________
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 Fri, Dec 24, 2010 at 10:44 AM, Lennart Poettering <[hidden email]> wrote:

> On Fri, 24.12.10 10:38, Dhaval Giani ([hidden email]) wrote:
>
>> > "cpu" as it is now, is not really usable in any setup that we could ship
>> > by default. It is only useful if people actually manually configure
>> > something.
>>
>> Which is why you did the default cgroup grouping patch, and Mike did
>> the autogrouping patch.
>
> Well, but as it turns out now my "4 line bash patch" as known from
> slashdot apparently breaks RT, and I am wondering whether the
> auto-grouping kernel patch might break RT too.
>
 autogrouping does not.
_______________________________________________
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 Dhaval Giani
On Fri, Dec 24, 2010 at 10:18:33AM +0100, Dhaval Giani wrote:

> > Well, it doesn't work at all on Ubuntu, and afaics this is with blessing
> > from upstream libcgroup: unless you manually fiddle with the cpu
> > hierarchy all invocations of sched_setscheduler(FIFO/RR) will fail in
> > the default setup on Ubuntu. And there is no really good reason for this
> > change: simply moving everything into a different cgroup does not give
> > any benefit in scheduling, does it?
>
> Incorrect. It does. The moment you have the first cgroup come into the
> picture. Without any additional cgroups, there is no change (except of
> course for rt processes for which reason they are left in the root),
> but the moment you have an additional cgroup come in, it gets the
> bandwidth that would be expected by a user as opposed to as expected
> by the system.

you are always talking about rt PROCESSES !!!
every jack application is multithreaded.
there is normally a single thread that runs sched_fifo.




--
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

Tommaso Cucinotta-2
In reply to this post by torbenh
Sorry for the late reply . . .

Il 19/12/2010 15:21, torbenh ha scritto:
> On Sat, Dec 18, 2010 at 09:40:51PM +0100, Tommaso Cucinotta wrote:
>> 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.

AFAIK, sometimes Jack clients create threads by themselves, then
they rely on the Jack lib in order to give them the same RT settings
as the other Jack client threads. The Jack lib function being called
should be:

       int jack_acquire_real_time_scheduling (pthread_t thread, int
priority);
 
http://jackaudio.org/files/docs/html/group__ClientThreads.html#ga09d1c5ce0bb854eac0dba1ec950a3197

which in turn calls

   int JackThread::AcquireRealTimeImp(pthread_t thread, int priority);

and this constitutes an issue because in this case the Jack framework
has only access to the pthread_t, and it does not create the thread
in the 1st place (like it happens instead with jack_client_create_thread()).

I agree that for threads created by the Jack library itself there are no
problems at all.

Bye,

     T.

> 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,
>
>
>


--
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] [cgroups] add basic support to move jack client threads into the same cgroup as jackd

David Henningsson-3
In reply to this post by Lennart Poettering-16
On 2010-12-24 10:08, Lennart Poettering wrote:
> On Fri, 24.12.10 09:53, Dhaval Giani ([hidden email]) wrote:
>
>>> I do believe that the short time fix for this problem is that
>>> libcgroup/Ubuntu should stop sorting processes into any non-root cgroup
>>> hierarchy by default.
>>
>> Ubuntu needs to fix that.
>
> Yes, they do. David?

Guess that's me. I'm not really a scheduling expert and had never heard
of cgroups before this bug showed up, but I'll try to grab the right
people at Canonical's next sprint, which is 10-14 January. That is,
unless somebody fixes it sooner.

While we're at it, there is this page:
http://jackaudio.org/linux_group_sched - which gives the impression that
Jack-RT is not working on Ubuntu 10.10 (Maverick). AFAIK, this is wrong
(at least I can run both PA, Rtkit and Jack in 10.10 without them
complaining about missing RT), the only version that needs fixing is
11.04, which is in development. Paul, can you fix the webpage?

--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic

_______________________________________________
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 Sun, Jan 2, 2011 at 12:56 AM, David Henningsson
<[hidden email]> wrote:

> On 2010-12-24 10:08, Lennart Poettering wrote:
>>
>> On Fri, 24.12.10 09:53, Dhaval Giani ([hidden email]) wrote:
>>
>>>> I do believe that the short time fix for this problem is that
>>>> libcgroup/Ubuntu should stop sorting processes into any non-root cgroup
>>>> hierarchy by default.
>>>
>>> Ubuntu needs to fix that.
>>
>> Yes, they do. David?
>
> Guess that's me. I'm not really a scheduling expert and had never heard of
> cgroups before this bug showed up, but I'll try to grab the right people at
> Canonical's next sprint, which is 10-14 January. That is, unless somebody
> fixes it sooner.
>

Its quite straightforward, the config file at /etc/sysconfig/cgconfig should say
CREATE_DEFAULT=no as opposed to CREATE_DEFAULT=yes

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

David Henningsson-3
On 2011-01-02 05:18, Dhaval Giani wrote:

> On Sun, Jan 2, 2011 at 12:56 AM, David Henningsson
> <[hidden email]>  wrote:
>> On 2010-12-24 10:08, Lennart Poettering wrote:
>>>
>>> On Fri, 24.12.10 09:53, Dhaval Giani ([hidden email]) wrote:
>>>
>>>>> I do believe that the short time fix for this problem is that
>>>>> libcgroup/Ubuntu should stop sorting processes into any non-root cgroup
>>>>> hierarchy by default.
>>>>
>>>> Ubuntu needs to fix that.
>>>
>>> Yes, they do. David?
>>
>> Guess that's me. I'm not really a scheduling expert and had never heard of
>> cgroups before this bug showed up, but I'll try to grab the right people at
>> Canonical's next sprint, which is 10-14 January. That is, unless somebody
>> fixes it sooner.
>>
>
> Its quite straightforward, the config file at /etc/sysconfig/cgconfig should say
> CREATE_DEFAULT=no as opposed to CREATE_DEFAULT=yes

Hmm, it doesn't seem to be that simple unfortunately. There is no such
file present. There is a package called "cgroup-bin" that adds something
similar into /etc/default/cgconfig, but that package isn't installed
either. Not even the cgroup library package is installed.

http://git.debian.org/?p=collab-maint/libcgroup.git;a=blob;f=debian/cgroup-bin.cgconfig.default;h=5a61bf6e9878250557f3c4993376c7c408135b89;hb=HEAD

I also tried manually adding a /etc/sysconfig/cgconfig file with
"CREATE_DEFAULT=no" in it, but it didn't help either.


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
_______________________________________________
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

David Henningsson-3
It might be that Ubuntu is bitten by this instead:
https://lkml.org/lkml/2011/1/10/52

We indeed added the autogroup patch (or some version of it) to 2.6.37
(for testing only - 11.04 will use 2.6.38) so it seems reasonable.

// David
_______________________________________________
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 Mon, Jan 10, 2011 at 4:28 PM, David Henningsson
<[hidden email]> wrote:
> It might be that Ubuntu is bitten by this instead:
> https://lkml.org/lkml/2011/1/10/52
>
> We indeed added the autogroup patch (or some version of it) to 2.6.37 (for
> testing only - 11.04 will use 2.6.38) so it seems reasonable.

that's not a different bug. the idea of turning off "auto cgroup" is
to leave tasks in the root cgroup with has RT bandwidth already
allocated.
_______________________________________________
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

David Henningsson-3
In reply to this post by Lennart Poettering-16
On 2010-12-24 10:44, Lennart Poettering wrote:

> On Fri, 24.12.10 10:38, Dhaval Giani ([hidden email]) wrote:
>
>>> "cpu" as it is now, is not really usable in any setup that we could ship
>>> by default. It is only useful if people actually manually configure
>>> something.
>>
>> Which is why you did the default cgroup grouping patch, and Mike did
>> the autogrouping patch.
>
> Well, but as it turns out now my "4 line bash patch" as known from
> slashdot apparently breaks RT, and I am wondering whether the
> auto-grouping kernel patch might break RT too.

Epilog: This is now fixed in 2.6.38-rc2 kernel, and the corresponding
2.6.38-1-generic kernel in Ubuntu.

The bug was caused by the autogroup patch, which the development version
of Ubuntu (11.04) applied on top of 2.6.37 for testing/experimental
purposes. That's why Ubuntu got it before anyone else did.

The fix:

commit f44937718ce3b8360f72f6c68c9481712517a867
Author: Mike Galbraith <[hidden email]>
Date:   Thu Jan 13 04:54:50 2011 +0100

sched, autogroup: Fix CONFIG_RT_GROUP_SCHED sched_setscheduler() failure

--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
_______________________________________________
Jack-Devel mailing list
[hidden email]
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org
12