public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: linux-kernel@vger.kernel.org, ebiederm@xmission.com
Subject: Re: [RFC] ps command race fix
Date: Mon, 24 Jul 2006 19:06:26 -0700	[thread overview]
Message-ID: <20060724190626.75b71d02.akpm@osdl.org> (raw)
In-Reply-To: <20060725105339.e8c25775.kamezawa.hiroyu@jp.fujitsu.com>

On Tue, 25 Jul 2006 10:53:39 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Mon, 24 Jul 2006 18:20:00 -0700
> Andrew Morton <akpm@osdl.org> wrote:
> > 
> > It allocates a potentially-significant amount of memory per-task, until
> > that tasks exits (we could release it earlier, but the problem remains) and
> > it adds yet another global lock in the process exit path.
> > 
> I see.
> 
> > >  5 files changed, 138 insertions(+), 62 deletions(-)
> > 
> > And it adds complexity and code.
> > 
> > So I think we're still seeking a solution to this.
> > 
> > Options might be:
> > 
> > a) Pin the most-recently-visited task in some manner, so that it is
> >    still on the global task list when we return.  That's fairly simple to
> >    do (defer the release_task()) but it affects task lifetime and visibility
> >    in rare and worrisome ways.
> > 
> > b) Change proc_pid_readdir() so that it walks the pid_hash[] array
> >    instead of the task list.  Need to do something clever when traversing
> >    each bucket's list, but I'm not sure what ;) It's the same problem.
> > 
> >    Possibly what we could do here is to permit the task which is walking
> >    /proc to pin a particular `struct pid': take a ref on it then when we
> >    next start walking one of the pid_hash[] chains, we _know_ that the
> >    `struct pid' which we're looking for will still be there.  Even if it
> >    now refers to a departed process.
> > 
> > c) Nuke the pid_hash[], convert the whole thing to a radix-tree. 
> >    They're super-simple to traverse.  Not sure what we'd index it by
> >    though.
> > 
> > I guess b) is best.
> > 
> 
> I tried b) at the first place.

OK.

> but it was not very good because 
> proc_pid_readdir() has to traverse all pids, not tgids.

You mean "all tgids, not pids".

> So, I had to access
> task_struct of the pid. I wanted to avoid to access task struct itself,

Why do you wish to avoid accessing the task_struct?

> my patch implemented a table made only from tgids.
> 
> But as you say, my patch is much intrusive. 
> I'll dig this more. thank you for advise.

Thanks.

  reply	other threads:[~2006-07-25  2:06 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-14 11:39 [RFC] ps command race fix KAMEZAWA Hiroyuki
2006-07-25  1:20 ` Andrew Morton
2006-07-25  1:48   ` Paul Jackson
2006-07-25  2:00     ` Andrew Morton
2006-07-25  2:08     ` KAMEZAWA Hiroyuki
2006-07-25  2:33       ` Andrew Morton
2006-07-25  2:50         ` KAMEZAWA Hiroyuki
2006-07-25  3:16           ` KAMEZAWA Hiroyuki
2006-08-13 16:29             ` Eric W. Biederman
2006-08-13 17:34               ` Andrew Morton
2006-08-13 19:00                 ` Eric W. Biederman
2006-08-13 19:12                   ` Paul Jackson
2006-08-16  1:23                     ` KAMEZAWA Hiroyuki
2006-08-17  4:59                       ` Eric W. Biederman
2006-08-17  6:32                         ` KAMEZAWA Hiroyuki
2006-08-17 13:39                           ` Eric W. Biederman
2006-08-17 18:16                             ` Jean Delvare
2006-08-18  0:21                             ` KAMEZAWA Hiroyuki
2006-08-18  3:53                           ` Eric W. Biederman
2006-08-13 20:08               ` Albert Cahalan
2006-08-16  2:20                 ` Kyle Moffett
2006-07-25  7:22         ` Paul Jackson
2006-07-25  1:53   ` KAMEZAWA Hiroyuki
2006-07-25  2:06     ` Andrew Morton [this message]
2006-07-25  2:34       ` KAMEZAWA Hiroyuki
2006-07-25  6:09   ` Eric W. Biederman
  -- strict thread matches above, loose matches on Subject: below --
2006-07-25  6:47 Albert Cahalan

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=20060724190626.75b71d02.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=ebiederm@xmission.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    /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