linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: David Rientjes <rientjes@google.com>
Cc: Mike Galbraith <mgalbraith@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Menage <paul@paulmenage.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Li Zefan <lizefan@huawei.com>
Subject: Re: [patch 0/2] cpusets, cpu_cgroup: disallow attaching kthreadd
Date: Thu, 5 Apr 2012 15:24:00 -0700	[thread overview]
Message-ID: <20120405222400.GC29517@google.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1204051454070.17852@chino.kir.corp.google.com>

Hello, David.

On Thu, Apr 05, 2012 at 03:03:06PM -0700, David Rientjes wrote:
> Yes, they act system-wide but that doesn't mean their memory usage or cpu 
> need to be accounted together.  The key is that all cgroups, current or 
> future, aren't necessarily for limiting those system-wide resources but 
> rather can provide useful insight into their cost by monitoring either 
> their memory or cpu through these two cgroups.
> 
> kthreadd certainly is not the only system-wide resource on the kernel; so 
> why are you not arguing that all PF_KTHREAD threads not be allowed into 
> non-root cgroups?  We actually have many kthreads in a memcg that _is_ 
> limited to a specific amount of memory together with other system daemons 
> that are killable if it becomes oom.
> 
> The reason we're discussing kthreadd here is because it has the funny 
> ability to fork other kthreads that become PF_THREAD_BOUND.  Usually the 
> only PF_THREAD_BOUND threads are ones created at boot.  However, if a 
> kthread is started for a specific cpu, it gets this bit set to stay on 
> that cpu via kthread_bind().  That's what causes an inconsistency for only 
> two types of cgroups: cpusets and cpu and we don't allow them to move 
> amongst them because of that.

That and because ramifications of putting kthreadd is difficult to
predict it being the root of all kthreads.  It's fine to be able to
put specific kthreads into cgroups if doing such makes sense for that
type of kthreads.

> That's what this patch series addresses.  Please don't unnecessarily limit 
> our ability with kthreadd or kthreads in general for accounting of system 
> activity.

I can see your point but the problem is that it conflicts with the
long term direction cgroup is taking and that cgroup seems generally
over-engineered to allow too many things which aren't too necessary to
the point where it's a giant pain in the ass for the subsystems and
people involved, so I'm far more likely to go for chopping down and
restricting stuff if it's not strictly necessary.  Somethings are just
stupid to allow and constrict future development and maintenance
unnecessarily which tends to be more important than supporting some
acrobatic use cases.

It's not like we have concrete plan regarding single hierarchy thing
at the moment, so it could be possible to support the use case you're
describing but please bear in mind that such use case might not be of
high priority.  Hmm... I'll think more about it.

Thanks.

-- 
tejun

  reply	other threads:[~2012-04-05 22:24 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-03 17:58 [patch] cgroups: disallow attaching kthreadd Mike Galbraith
2012-04-03 18:49 ` Tejun Heo
2012-04-04  2:34   ` Mike Galbraith
2012-04-04 23:06     ` Tejun Heo
2012-04-04 23:10       ` Tejun Heo
2012-04-04  7:15 ` David Rientjes
2012-04-04 10:38   ` Mike Galbraith
2012-04-04 11:29     ` David Rientjes
2012-04-04 12:30       ` Mike Galbraith
2012-04-04 21:17         ` David Rientjes
2012-04-04 23:09           ` Tejun Heo
2012-04-04 23:14             ` David Rientjes
2012-04-05  4:47             ` Mike Galbraith
2012-04-05  4:58               ` David Rientjes
2012-04-05  5:49                 ` Mike Galbraith
2012-04-05  6:07                   ` David Rientjes
2012-04-05  6:42                     ` Mike Galbraith
2012-04-05  6:49                       ` David Rientjes
2012-04-05  7:14                         ` [patch 0/2] cpusets, cpu_cgroup: " David Rientjes
2012-04-05  7:14                           ` [patch 1/2] cpusets: " David Rientjes
2012-04-05  7:14                           ` [patch 2/2] cpu_cgroup: " David Rientjes
2012-04-05 16:08                           ` [patch 0/2] cpusets, " Tejun Heo
2012-04-05 21:26                             ` David Rientjes
2012-04-05 21:37                               ` Tejun Heo
2012-04-05 21:46                                 ` Tejun Heo
2012-04-06  7:50                                   ` Li Zefan
2012-04-06 15:54                                     ` Tejun Heo
2012-04-05 22:03                                 ` David Rientjes
2012-04-05 22:24                                   ` Tejun Heo [this message]
2012-04-05 22:31                                     ` Tejun Heo
2012-04-05 22:55                                     ` David Rientjes
2012-04-05 22:58                                       ` Tejun Heo
2012-04-05 23:05                                         ` David Rientjes
2012-04-05 23:13                                           ` Tejun Heo
2012-04-05 23:40                                             ` David Rientjes
2012-04-06 15:52                                               ` Tejun Heo
2012-04-06 18:26                                                 ` Peter Zijlstra
2012-04-06 20:49                                                   ` David Rientjes
2012-04-07 15:02                                                     ` Hiroyuki Kamezawa
2012-04-10  0:52                                                       ` David Rientjes
2012-04-14 11:35                                                         ` Peter Zijlstra
2012-04-20 17:55                                                         ` Tejun Heo
2012-04-06 20:46                                                 ` David Rientjes
2012-04-14 11:28                                                   ` Peter Zijlstra
2012-04-05  7:36                         ` [patch] cgroups: " Mike Galbraith
2012-04-05  8:00                           ` David Rientjes
2012-04-14 11:17                     ` Peter Zijlstra
2012-04-05 16:11               ` Tejun Heo
2012-04-20 17:57 ` Tejun Heo
2012-04-21  5:31   ` Mike Galbraith
2012-04-21  6:54     ` Li Zefan
2012-04-21  7:13       ` Mike Galbraith
2012-04-23 18:05         ` Tejun Heo

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20120405222400.GC29517@google.com \
    --to=tj@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=mgalbraith@suse.de \
    --cc=mingo@redhat.com \
    --cc=paul@paulmenage.org \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

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

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