All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Roland McGrath <roland@redhat.com>
Cc: Frank Mayhar <fmayhar@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Christoph Lameter <cl@linux-foundation.org>,
	Doug Chapman <doug.chapman@hp.com>,
	mingo@elte.hu, adobriyan@gmail.com, akpm@linux-foundation.org,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: regression introduced by - timers: fix itimer/many thread hang
Date: Mon, 17 Nov 2008 23:18:57 +0100	[thread overview]
Message-ID: <20081117221857.GA29423@redhat.com> (raw)
In-Reply-To: <20081117181655.8BDE9154221@magilla.localdomain>

On 11/17, Roland McGrath wrote:
>
> > > 	-	if (!->signal)
> > > 	+	if (->exit_state)
> > > 			return;
> >
> > Yes, unless I missed something again, this should work. I'll send
> > the (simple) patches soon, but I have no idea how to test them.
>
> That certainly will exclude the problem of crashing in the tick interrupt
> after exit_notify.  Unfortunately, it's moving in an undesireable direction
> for the long run.  That is, it loses from the accounting even more of the
> CPU time spent on the exit path.

Yes, I thought about this too.

But please note that currently this already happens for sub-threads (and
if we protect ->signal with rcu too), the exiting sub-thread does not
contribute to accounting after release_task(self). Also, when the last
thread exits the process can be reaped by its parent, but after that
the threads can still use CPU.

IOW, when ->exit_signal != 0 we already sent the notification to parent
with utime/stime, the parent can reap current at any moment before it
does the final schedule. I don't think we can do something here.

But if we make ->signal refcountable, we can improve the case with the
exiting subthreads at least.

(Just in case, anyway I completeley agree, this hack (and unlock_wait)
 should be killed in 2.6.29).

> > However, I'm afraid there is another problem. On 32 bit cpus we can't
> > read "u64 sum_exec_runtime" atomically, and so thread_group_cputime()
> > can "overestimate" ->sum_exec_runtime by (up to) UINT_MAX if it races
> > with the thread which updates its per_cpu_ptr(.totals). This for example
> > means that check_process_timers() can fire the CPUCLOCK_SCHED timers
> > before time.
> >
> > No?
>
> Yes, I think you're right.  The best solution that comes to mind off hand
> is to protect the update/read of that u64 with a seqcount_t on 32-bit.

Oh, but we need them to be per-cpu, and both read and write need memory
barriers... Not that I argue, this will fix the problem of course, just
I don't know how this impacts the perfomance.

Oleg.


  reply	other threads:[~2008-11-17 21:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1224694989.8431.23.camel@oberon>
     [not found] ` <1225132746.14792.13.camel@bobble.smo.corp.google.com>
     [not found]   ` <1225219114.24204.37.camel@oberon>
2008-11-06  1:58     ` regression introduced by - timers: fix itimer/many thread hang Frank Mayhar
2008-11-06 11:03       ` Peter Zijlstra
2008-11-06 15:03         ` Christoph Lameter
2008-11-06 15:08           ` Peter Zijlstra
2008-11-06 16:08             ` Christoph Lameter
2008-11-06 23:52             ` Frank Mayhar
2008-11-07  8:35               ` Ingo Molnar
2008-11-07 10:29               ` Peter Zijlstra
2008-11-07 18:10                 ` Frank Mayhar
2008-11-07 20:26                   ` Peter Zijlstra
2008-11-10 14:38                     ` Christoph Lameter
2008-11-10 14:42                       ` Peter Zijlstra
2008-11-10 15:41                         ` Christoph Lameter
2008-11-10 18:00                         ` Frank Mayhar
2008-11-14  2:42                           ` Roland McGrath
2008-11-14 16:41                             ` Oleg Nesterov
2008-11-17 14:36                               ` Oleg Nesterov
2008-11-17 18:16                                 ` Roland McGrath
2008-11-17 22:18                                   ` Oleg Nesterov [this message]
2008-11-17 21:49                                     ` Roland McGrath
2008-11-11  0:20                         ` Ingo Oeser
2008-11-11 13:58                           ` Christoph Lameter
2008-11-21 18:42                 ` Petr Tesarik
2008-11-21 19:26                   ` Frank Mayhar
2008-11-23 14:24                   ` Peter Zijlstra
2008-11-24  8:46                     ` Petr Tesarik
2008-11-24  9:33                       ` Peter Zijlstra
2008-11-24 12:32                         ` Petr Tesarik
2008-11-24 12:59                           ` Peter Zijlstra
2008-11-24 16:06                             ` Peter Zijlstra
2008-11-06 16:31         ` [PATCH] revert: " Peter Zijlstra
2008-11-06 21:44           ` Ingo Molnar
2008-11-06 21:53             ` Christoph Lameter
2008-11-07 10:19               ` Peter Zijlstra
2008-11-13 16:00           ` Doug Chapman
2008-11-13 16:08             ` Ingo Molnar
2008-11-14 14:10               ` Doug Chapman
     [not found] <20081105191211.c0316b94.akpm@linux-foundation.org>
2008-11-06 12:59 ` regression introduced by - " Oleg Nesterov

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=20081117221857.GA29423@redhat.com \
    --to=oleg@redhat.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux-foundation.org \
    --cc=doug.chapman@hp.com \
    --cc=fmayhar@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=roland@redhat.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 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.