public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Alessio Igor Bogani <abogani@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Chris Metcalf <cmetcalf@tilera.com>,
	Christoph Lameter <cl@linux.com>,
	Geoff Levand <geoff@infradead.org>,
	Gilad Ben Yossef <gilad@benyossef.com>,
	Hakan Akkan <hakanakkan@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	Li Zhong <zhong@linux.vnet.ibm.com>,
	Namhyung Kim <namhyung.kim@lge.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 06/33] cputime: Safely read cputime of full dynticks CPUs
Date: Wed, 9 Jan 2013 19:35:06 +0100	[thread overview]
Message-ID: <20130109183454.GB8293@somewhere.redhat.com> (raw)
In-Reply-To: <1357743251.5190.57.camel@gandalf.local.home>

On Wed, Jan 09, 2013 at 09:54:11AM -0500, Steven Rostedt wrote:
> On Tue, 2013-01-08 at 03:08 +0100, Frederic Weisbecker wrote:
> 
> > diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> > index 07912dd..bf4f72d 100644
> > --- a/kernel/sched/cputime.c
> > +++ b/kernel/sched/cputime.c
> > @@ -484,7 +484,7 @@ void vtime_task_switch(struct task_struct *prev)
> >   * vtime_account().
> >   */
> >  #ifndef __ARCH_HAS_VTIME_ACCOUNT
> > -void vtime_account(struct task_struct *tsk)
> > +void vtime_account_irq_enter(struct task_struct *tsk)
> >  {
> >  	if (!in_interrupt()) {
> >  		/*
> > @@ -505,7 +505,7 @@ void vtime_account(struct task_struct *tsk)
> >  	}
> >  	vtime_account_system(tsk);
> >  }
> > -EXPORT_SYMBOL_GPL(vtime_account);
> > +EXPORT_SYMBOL_GPL(vtime_account_irq_enter);
> >  #endif /* __ARCH_HAS_VTIME_ACCOUNT */
> >  #endif /* CONFIG_VIRT_CPU_ACCOUNTING */
> >  
> > @@ -616,41 +616,67 @@ void thread_group_cputime_adjusted(struct task_struct *p, cputime_t *ut, cputime
> >  #endif /* !CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
> >  
> >  #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
> > -static DEFINE_PER_CPU(long, last_jiffies) = INITIAL_JIFFIES;
> > -
> > -static cputime_t get_vtime_delta(void)
> > +static cputime_t get_vtime_delta(struct task_struct *tsk)
> >  {
> >  	long delta;
> >  
> > -	delta = jiffies - __this_cpu_read(last_jiffies);
> > -	__this_cpu_add(last_jiffies, delta);
> > +	delta = jiffies - tsk->prev_jiffies;
> > +	tsk->prev_jiffies += delta;
> >  
> >  	return jiffies_to_cputime(delta);
> >  }
> >  
> > -void vtime_account_system(struct task_struct *tsk)
> > +static void __vtime_account_system(struct task_struct *tsk)
> >  {
> > -	cputime_t delta_cpu = get_vtime_delta();
> > +	cputime_t delta_cpu = get_vtime_delta(tsk);
> >  
> >  	account_system_time(tsk, irq_count(), delta_cpu, cputime_to_scaled(delta_cpu));
> >  }
> >  
> > +void vtime_account_system(struct task_struct *tsk)
> > +{
> > +	write_seqlock(&tsk->vtime_seqlock);
> > +	__vtime_account_system(tsk);
> 
> __vtime_account_system() calls account_system_time()
> account_system_time() calls __account_system_time()
> __account_system_time() calls acct_update_integrals()
> (when CONFIG_TASK_XACCT is set)
> acct_update_integrals() calls task_cputime()
> task_cputime() grabs t->vtime_seqlock for read
> 
>  DEADLOCK
> 
> ironically the subject says *Safely* read cputime ;-)

Well at least it crashes safely. Safely as in "deterministic".

  reply	other threads:[~2013-01-09 18:35 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-08  2:08 [ANNOUNCE] 3.8-rc2-nohz2 Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 01/33] context_tracking: Add comments on interface and internals Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 02/33] context_tracking: Export context state for generic vtime Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 03/33] cputime: Generic on-demand virtual cputime accounting Frederic Weisbecker
2013-01-08 20:23   ` Steven Rostedt
2013-01-08 20:26   ` Steven Rostedt
2013-01-08 21:00     ` Paul E. McKenney
2013-01-08 20:45   ` Steven Rostedt
2013-01-09 13:46   ` Steven Rostedt
2013-01-09 13:50     ` Steven Rostedt
2013-01-08  2:08 ` [PATCH 04/33] cputime: Allow dynamic switch between tick/virtual based " Frederic Weisbecker
2013-01-08 21:20   ` Steven Rostedt
2013-01-08 23:22     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 05/33] cputime: Use accessors to read task cputime stats Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 06/33] cputime: Safely read cputime of full dynticks CPUs Frederic Weisbecker
2013-01-09 14:54   ` Steven Rostedt
2013-01-09 18:35     ` Frederic Weisbecker [this message]
2013-01-08  2:08 ` [PATCH 07/33] nohz: Basic full dynticks interface Frederic Weisbecker
2013-02-11 14:35   ` Borislav Petkov
2013-02-20 16:32     ` Borislav Petkov
2013-03-07 23:41       ` Frederic Weisbecker
2013-03-07 23:35     ` Frederic Weisbecker
2013-03-08 10:17       ` Borislav Petkov
2013-03-08 13:45         ` Frederic Weisbecker
2013-03-08 14:32           ` Borislav Petkov
2013-03-08 16:55             ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 08/33] nohz: Assign timekeeping duty to a non-full-nohz CPU Frederic Weisbecker
2013-02-15 11:57   ` Borislav Petkov
2013-02-20 15:57     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 09/33] nohz: Trace timekeeping update Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 10/33] nohz: Wake up full dynticks CPUs when a timer gets enqueued Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 11/33] rcu: Restart the tick on non-responding full dynticks CPUs Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 12/33] sched: Comment on rq->clock correctness in ttwu_do_wakeup() in nohz Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 13/33] sched: Update rq clock on nohz CPU before migrating tasks Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 14/33] sched: Update rq clock on nohz CPU before setting fair group shares Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 15/33] sched: Update rq clock on tickless CPUs before calling check_preempt_curr() Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 16/33] sched: Update rq clock earlier in unthrottle_cfs_rq Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 17/33] sched: Update clock of nohz busiest rq before balancing Frederic Weisbecker
2013-01-08 10:20   ` Li Zhong
2013-03-07 23:51     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 18/33] sched: Update rq clock before idle balancing Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 19/33] sched: Update nohz rq clock before searching busiest group on load balancing Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 20/33] nohz: Move nohz load balancer selection into idle logic Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 21/33] nohz: Full dynticks mode Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 22/33] nohz: Only stop the tick on RCU nocb CPUs Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 23/33] nohz: Don't turn off the tick if rcu needs it Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 24/33] nohz: Don't stop the tick if posix cpu timers are running Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 25/33] nohz: Add some tracing Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 26/33] rcu: Don't keep the tick for RCU while in userspace Frederic Weisbecker
2013-01-08  4:06   ` Paul E. McKenney
2013-01-08  2:08 ` [PATCH 27/33] profiling: Remove unused timer hook Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 28/33] timer: Don't run non-pinned timer to full dynticks CPUs Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 29/33] sched: Use an accessor to read rq clock Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 30/33] sched: Debug nohz " Frederic Weisbecker
2013-03-20 23:23   ` Kevin Hilman
2013-04-11 16:47     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 31/33] sched: Remove broken check for skip clock update Frederic Weisbecker
2013-01-08  2:11   ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 32/33] sched: Update rq clock before rt sched average scale Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 33/33] sched: Disable lb_bias feature for full dynticks Frederic Weisbecker

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=20130109183454.GB8293@somewhere.redhat.com \
    --to=fweisbec@gmail.com \
    --cc=abogani@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=cmetcalf@tilera.com \
    --cc=geoff@infradead.org \
    --cc=gilad@benyossef.com \
    --cc=hakanakkan@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung.kim@lge.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=zhong@linux.vnet.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox