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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox