On Mon, 2014-08-18 at 10:03 -0400, Rik van Riel wrote:
> On 08/18/2014 12:44 AM, Mike Galbraith wrote:
> > On Sat, 2014-08-16 at 19:50 +0200, Oleg Nesterov wrote:
> >> On 08/16, Rik van Riel wrote:
> >>>
> >>> + do {
> >>> + seq = nextseq;
> >>> + read_seqbegin_or_lock(&sig->stats_lock, &seq);
> >>> + times->utime = sig->utime;
> >>> + times->stime = sig->stime;
> >>> + times->sum_exec_runtime = sig->sum_sched_runtime;
> >>> +
> >>> + for_each_thread(tsk, t) {
> >>> + task_cputime(t, &utime, &stime);
> >>> + times->utime += utime;
> >>> + times->stime += stime;
> >>> + times->sum_exec_runtime += task_sched_runtime(t);
> >>> + }
> >>> + /* If lockless access failed, take the lock. */
> >>> + nextseq = 1;
> >>
> >> Yes, thanks, this answers my concerns.
> >>
> >> Cough... can't resist, and I still think that we should take
> >> rcu_read_lock()
> >> only around for_each_thread() and the patch expands the critical section
> >> for
> >> no reason. But this is minor, I won't insist.
> >
> > Hm. Should traversal not also disable preemption to preserve the error
> > bound Peter mentioned?
>
> The second traversal takes the spinlock, which automatically
> disables preemption.
According to my testing, a PREEMPT kernel can get all the way through
thread_group_cputime() lockless, preemption can/does happen during
traversal, the call can and does then take more than ticks * CPUs (can
take LOTS more if you get silly), so Peter's bound appears to be toast
for PREEMPT.
Not that I really care mind you, just seemed the folks who don't do
zillion threads, would never feel the pain you're alleviating, now get
some accuracy loss if running PREEMPT.
BTW, something else that doesn't matter one bit but I was curious about,
as noted, clock_gettime() used to use the tasklist_lock, which is loads
better than siglock, at least on a modest box. On a 64 core box with
200 threads, crusty old 3.0 kernel is faster than patched up master, and
configs are both NOPREEMPT tune-for-maximum-bloat.
('course what zillion cores + zillion threads does with tasklist_lock
ain't _at all_ pretty, but it doesn't demolish modest boxen)
patched master
vogelweide:/abuild/mike/:[0]# time ./pound_clock_gettime
real 0m2.953s
user 0m0.036s
sys 3m2.588s
vogelweide:/abuild/mike/:[0]# time ./pound_clock_gettime
real 0m2.930s
user 0m0.076s
sys 3m1.800s
vogelweide:/abuild/mike/:[0]# time ./pound_clock_gettime
real 0m2.988s
user 0m0.052s
sys 3m5.208s
sle11-sp3 (3.0.101)
vogelweide:/abuild/mike/:[0]# time ./pound_clock_gettime
real 0m1.521s
user 0m0.072s
sys 0m8.397s
vogelweide:/abuild/mike/:[0]# time ./pound_clock_gettime
real 0m1.260s
user 0m0.032s
sys 0m6.244s
vogelweide:/abuild/mike/:[0]# time ./pound_clock_gettime
real 0m1.391s
user 0m0.020s
sys 0m7.016s
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
