public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: linux-kernel@vger.kernel.org, torvalds@osdl.org,
	Zhang Le <r0bertz@gentoo.org>, Al Viro <viro@ZenIV.linux.org.uk>,
	Oleg Nesterov <oleg@tv-sign.ru>
Subject: Re: [PATCH] proc:  Fix proc_tid_readdir so it handles the weird cases.
Date: Wed, 18 Mar 2009 20:40:20 -0700	[thread overview]
Message-ID: <m13adaxjrv.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20090318135718.GA32106@hawkmoon.kerlabs.com> (Louis Rilling's message of "Wed\, 18 Mar 2009 14\:57\:18 +0100")

Louis Rilling <Louis.Rilling@kerlabs.com> writes:

> On 18/03/09  6:39 -0700, Eric W. Biederman wrote:
>> 
>> Fix the logic in proc_task_readdir so that we provide the
>> guarantee that in the sequence:
>> opendir
>> readdir
>> readdir
>> ....
>> readdir
>> closedir
>> If a thread exists from the beginning until the end we are
>> guaranteed to see it, even if some threads are created or
>> worse are destroyed during the readdir.
>> 
>> This guarantee is provided by reusing the same logic used
>> by proc_pid_readdir for the root directory of /proc.  The
>> pid is encoded into the offset, and we walk the pid bitmap
>> of all pids in the system, and test each of them to see if
>> it belongs to the specified thread group.
>> 
>> If we seek to a bad location or if the task we say last
>> exits it is ok because we can numerically find the next
>> task we would have returned.
>> 
>> This is slower that the previous version, but it should
>> not be too slow as this techinique is already use for
>> the root directory of /proc without problems.
>
> This makes 'ps -T x' an O(n^2) thing, compared to O(n) before the patch, right?
> It would be good to, at least, check for thread_group_empty().

Sort of.  It is more like it adds a constant to the traversing each thread
directory.  With everything in cache I don't expect it to be a big constant,
as I have reports that when I changed the pid directory it sped things up.

We definitely don't hold any locks so only the process doing the traversal will pay
the price.

Since this is actually a correctness issue I'm willing to pay a little more to get
the right result.

Eric


      reply	other threads:[~2009-03-19  3:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-16  6:44 [PATCH] filp->f_pos not correctly updated in proc_task_readdir Zhang Le
2009-03-16  8:34 ` Al Viro
2009-03-16 16:27   ` Zhang Le
2009-03-17 10:37   ` Alexey Dobriyan
2009-03-17 12:53     ` Zhang Le
2009-03-17 13:48       ` Alexey Dobriyan
2009-03-17 18:11       ` Al Viro
2009-03-17 18:12 ` Eric W. Biederman
2009-03-18  7:27   ` Zhang Le
2009-03-18 10:36     ` Eric W. Biederman
2009-03-18 13:39       ` [PATCH] proc: Fix proc_tid_readdir so it handles the weird cases Eric W. Biederman
2009-03-18 13:57         ` Louis Rilling
2009-03-19  3:40           ` Eric W. Biederman [this message]

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=m13adaxjrv.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=adobriyan@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --cc=r0bertz@gentoo.org \
    --cc=torvalds@osdl.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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