public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Paul Menage <menage@google.com>, Benjamin Blum <bblum@google.com>,
	containers@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, lizf@cn.fujitzu.com
Subject: Re: [PATCH 1/2] Adds a read-only "procs" file similar to "tasks" that  shows only unique tgids
Date: Wed, 15 Jul 2009 01:33:05 -0700	[thread overview]
Message-ID: <m1ocrm73cu.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20090703111016.ceb28541.akpm@linux-foundation.org> (Andrew Morton's message of "Fri\, 3 Jul 2009 11\:10\:16 -0700")

Andrew Morton <akpm@linux-foundation.org> writes:

> On Fri, 3 Jul 2009 10:54:48 -0700 Paul Menage <menage@google.com> wrote:
>
>> >__Unfortunately radix-trees are presented as operating on
>> > void* data, so one would need to do some typecasting when storing
>> > BITS_PER_LONG-sized bitfields inside them.
>> 
>> That would mean adding something a bit like the IDA wrapper that
>> converts IDR to deal with bitfields?
>
> I guess so.
>
>> Is the benefit of avoiding a vmalloc() at all costs really worth the
>> additional complexity
>
> Well no.  But nor was it worth the additional complexity the last twenty
> times someone resorted to vmalloc to solve a problem of this nature.  Taking
> a kernel-wide perspective here gives a different answer.
>
> However I don't think a little scoreboarding thing (what's the correct
> term) built around radix-trees would suffice to solve many of those
> past sins.  Whereas a general dynamic array thing would be applicable
> in many cases.

It is even easier.  Just grab the logic from proc_pid_readdir.
It uses rcu locking.
It returns pids in order.
It needs no mallocs to use.

The people who ran benchmarks tell me I actually sped up proc,
when I started traversing the existing bitmap of pids.

It makes the guarantee that for every process that existed for
the length of the operation you will see it's pid.  Processes
that die or were born half way through we don't say anything about
but if they stay around you will get them next time.

I think guaranteeing a truly atomic snapshot is likely to be a
horrible idea requiring all kinds of nasty locking, and smp
scalability issues.  So please walk the list of pids and
just return those that belong to your cgroup.

Compare to the rest of the implementations everyone is balking
at it should be simple.

Eric

  reply	other threads:[~2009-07-15  8:33 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
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 [this message]
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=m1ocrm73cu.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=bblum@google.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitzu.com \
    --cc=menage@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