mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Benjamin Blum <bblum@google.com>
To: Paul Menage <menage@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	lizf@cn.fujitzu.com, serue@us.ibm.com,
	containers@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] Adds a read-only "procs" file similar to "tasks" that  shows only unique tgids
Date: Thu, 2 Jul 2009 18:17:56 -0700	[thread overview]
Message-ID: <2f86c2480907021817o79fce75yd9785aab682f7bb4@mail.gmail.com> (raw)
In-Reply-To: <6599ad830907021808o6f3bb51eh324e4bf13544d83e@mail.gmail.com>

On Thu, Jul 2, 2009 at 6:08 PM, Paul Menage<menage@google.com> wrote:
> On Thu, Jul 2, 2009 at 5:53 PM, Andrew Morton<akpm@linux-foundation.org> wrote:
>>> In the first snippet, count will be at most equal to length. As length
>>> is determined from cgroup_task_count, it can be no greater than the
>>> total number of pids on the system.
>>
>> Well that's a problem, because there can be tens or hundreds of
>> thousands of pids, and there's a fairly low maximum size for kmalloc()s
>> (include/linux/kmalloc_sizes.h).
>>
>> And even if this allocation attempt doesn't exceed KMALLOC_MAX_SIZE,
>> large allocations are less unreliable.  There is a large break point at
>> 8*PAGE_SIZE (PAGE_ALLOC_COSTLY_ORDER).
>
> This has been a long-standing problem with the tasks file, ever since
> the cpusets days.
>
> There are ways around it - Lai Jiangshan <laijs@cn.fujitsu.com> posted
> a patch that allocated an array of pages to store pids in, with a
> custom sorting function that let you specify indirection rather than
> assuming everything was in one contiguous array. This was technically
> the right approach in terms of not needing vmalloc and never doing
> large allocations, but it was very complex; an alternative that was
> mooted was to use kmalloc for small cgroups and vmalloc for large
> ones, so the vmalloc penalty wouldn't be paid generally. The thread
> fizzled AFAICS.

As it is currently, the kmalloc call will simply fail if there are too
many pids, correct? Do we prefer not being able to read the file in
this case, or would we rather use vmalloc?

>
>>
>> One could perhaps create an alias (symlink?) and leave that in place
>> for a few kernel releases and then remove the old names.  The trick to
>> doing this politely is to arrange for a friendly printk to come out
>> when userspace uses the old filename, so people know to change their
>> tools.  That printk should come out once-per-boot, not once-per-access.
>
> Personally, I feel that a bit of ugliness in the naming inconsistency
> is less painful than trying to deprecate something that people might
> be using.

That's what the people who designed x86 said :P

> If we could just flip the names without breaking anyone,
> that would be great, but this is just a style issue rather than a
> functional issue. My experience of such printk() statements scattered
> around in code is that no-one takes much notice of them.

Whether or not we get rid of the old ones, it would be good to put in
aliases with the new style now so there's the option of removing the
old style ones later.

>
> Paul
>

  reply	other threads:[~2009-07-03  1:18 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-02 23:26 [PATCH 0/2] CGroups: cgroup member list enhancement/fix Paul Menage
2009-07-02 23:26 ` [PATCH 1/2] Adds a read-only "procs" file similar to "tasks" that shows only unique tgids Paul Menage
2009-07-02 23:46   ` Andrew Morton
2009-07-03  0:31     ` Benjamin Blum
2009-07-03  0:53       ` Andrew Morton
2009-07-03  1:08         ` Paul Menage
2009-07-03  1:17           ` Benjamin Blum [this message]
2009-07-03  2:08             ` Andrew Morton
2009-07-03  4:16               ` Paul Menage
2009-07-03  6:55                 ` Andrew Morton
2009-07-03  7:54                   ` KAMEZAWA Hiroyuki
2009-07-03 16:11                   ` Paul Menage
2009-07-03 16:50                     ` Andrew Morton
2009-07-03 17:54                       ` Paul Menage
2009-07-03 18:10                         ` Andrew Morton
2009-07-15  8:33                           ` Eric W. Biederman
2009-07-15 16:18                             ` Paul Menage
2009-07-03  2:25             ` Matt Helsley
2009-07-03  3:49               ` Paul Menage
2009-07-03  7:08               ` Benjamin Blum
2009-07-03  1:30           ` Andrew Morton
2009-07-03  5:54             ` KAMEZAWA Hiroyuki
2009-07-03 15:52               ` Paul Menage
2009-07-04  2:07                 ` KAMEZAWA Hiroyuki
2009-07-04 16:10                   ` Paul Menage
2009-07-05 23:53                     ` KAMEZAWA Hiroyuki
2009-07-02 23:26 ` [PATCH 2/2] Ensures correct concurrent opening/reading of pidlists across pid namespaces Paul Menage
2009-07-02 23:54   ` Andrew Morton
2009-07-03  0:22     ` Paul Menage
2009-07-03  0:26       ` Paul Menage
2009-07-03  0:43     ` Benjamin Blum
2009-07-03  1:15 ` [PATCH 0/2] CGroups: cgroup member list enhancement/fix Li Zefan
2009-07-05  6:38 ` Balbir Singh
2009-07-10 23:58   ` Paul Menage
2009-07-13 12:11     ` Balbir Singh
2009-07-13 16:26       ` Paul Menage
2009-07-14  5:56         ` Balbir Singh
2009-07-14  6:49           ` Paul Menage
2009-07-14  7:16             ` Balbir Singh
2009-07-14 17:34               ` Benjamin Blum
2009-07-14 17:43                 ` Paul Menage
2009-07-14 20:38                   ` Paul Menage
2009-07-14 23:08                     ` Matt Helsley

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=2f86c2480907021817o79fce75yd9785aab682f7bb4@mail.gmail.com \
    --to=bblum@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitzu.com \
    --cc=menage@google.com \
    --cc=serue@us.ibm.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

all inboxes | Powered by JetHome®