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
prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.