All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Olivier Langlois <olivier@trillion01.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 6/8] sched: task_sched_runtime introduce micro optimization
Date: Tue, 18 Jun 2013 19:18:20 +0200	[thread overview]
Message-ID: <20130618171819.GJ17619@somewhere.redhat.com> (raw)
In-Reply-To: <CAHGf_=qhooSDvZVBUqct0B9khTZ-8Ju5YZ7b0yA34=3UgY+OoA@mail.gmail.com>

On Tue, Jun 18, 2013 at 11:17:41AM -0400, KOSAKI Motohiro wrote:
> >> +#ifdef CONFIG_64BIT
> >> +     /*
> >> +      * 64-bit doesn't need locks to atomically read a 64bit value. So we
> >> +      * have two optimization chances, 1) when caller doesn't need
> >> +      * delta_exec and 2) when the task's delta_exec is 0. The former is
> >> +      * obvious. The latter is complicated. reading ->on_cpu is racy, but
> >> +      * this is ok. If we race with it leaving cpu, we'll take a lock. So
> >> +      * we're correct. If we race with it entering cpu, unaccounted time
> >> +      * is 0. This is indistinguishable from the read occurring a few
> >> +      * cycles earlier.
> >> +      */
> >> +     if (!add_delta || !p->on_cpu)
> >> +             return p->se.sum_exec_runtime;
> >
> > I'm not sure this is correct from an smp ordering POV. p->on_cpu may appear
> > to be 0 whereas the task is actually running for a while and p->se.sum_exec_runtime
> > can then be past the actual value on the remote CPU.
> 
> Quate form Paul's last e-mail
> 
> >Stronger:
> >
> >+#ifdef CONFIG_64BIT
> >+       if (!p->on_cpu)
> >+               return p->se.sum_exec_runtime;
> >+#endif
> >
> >[ Or !p->on_cpu || !add_delta ].
> >
> >We can take the racy read versus p->on_cpu since:
> >  If we race with it leaving cpu: we take lock, we're correct
> >  If we race with it entering cpu: unaccounted time ---> 0, this is
> >indistinguishable from the read occurring a few cycles earlier.

Yeah, my worry was more about both p->on_cpu and p->se.sum_exec_runtime being
stale for too long. How much time can happen in the worst case before CPU X sees
the updates done by a CPU Y under rq(Y)->lock considering that CPU X doesn't take rq(Y)
to read that update? I guess it depends on the hardware, locking and ordering
that happened before.

Bah it probably doesn't matter in practice.

Thanks.

  parent reply	other threads:[~2013-06-18 17:18 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-26 21:35 [PATCH v5 0/8] posix timers fixlet kosaki.motohiro
2013-05-26 21:35 ` [PATCH 1/8] posix-cpu-timers: don't account cpu timer after stopped thread runtime accounting kosaki.motohiro
2013-05-26 21:35 ` [PATCH 2/8] posix-cpu-timers: fix acounting delta_exec twice kosaki.motohiro
2013-06-03 19:07   ` Frederic Weisbecker
2013-05-26 21:35 ` kosaki.motohiro
2013-05-26 21:35 ` [PATCH 3/8] posix-cpu-timers: fix wrong timer initialization kosaki.motohiro
2013-06-18 14:20   ` Frederic Weisbecker
2013-06-18 15:12     ` KOSAKI Motohiro
2013-06-19 20:56       ` Frederic Weisbecker
2013-05-26 21:35 ` [PATCH 4/8] posix-cpu-timers: timer functions should use timer time instead of clock time kosaki.motohiro
2013-05-26 21:35 ` [PATCH 5/8] posix-cpu-timers: check_thread_timers() uses task_sched_runtime() kosaki.motohiro
2013-05-26 21:35 ` [PATCH 6/8] sched: task_sched_runtime introduce micro optimization kosaki.motohiro
2013-06-18 14:27   ` Frederic Weisbecker
2013-06-18 15:17     ` KOSAKI Motohiro
2013-06-18 15:28       ` KOSAKI Motohiro
2013-06-18 17:18       ` Frederic Weisbecker [this message]
2013-06-20  8:41         ` KOSAKI Motohiro
2013-05-26 21:35 ` [PATCH 7/8] posix-cpu-timers: cleanup cpu_{clock,timer}_sample{,_group} kosaki.motohiro
2013-05-26 21:35 ` [PATCH 8/8] posix-cpu-timers: fix cputimer initialization mistake for {u,s}time kosaki.motohiro
2013-05-31 16:26 ` [PATCH v5 0/8] posix timers fixlet KOSAKI Motohiro

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=20130618171819.GJ17619@somewhere.redhat.com \
    --to=fweisbec@gmail.com \
    --cc=kosaki.motohiro@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=olivier@trillion01.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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.