[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+55aFxaGKEyhXdHXNxvPrPQ-SGSpbXdfoeXrxfjPx3VXsgvtg@mail.gmail.com>
Date: Mon, 17 Oct 2011 07:57:56 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Simon Kirby <sim@...tway.ca>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Dave Jones <davej@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Martin Schwidefsky <schwidefsky@...ibm.com>,
Ingo Molnar <mingo@...e.hu>
Subject: Re: Linux 3.1-rc9
On Mon, Oct 17, 2011 at 3:34 AM, Peter Zijlstra <a.p.zijlstra@...llo.nl> wrote:
>
> Well, I thought atomic64_t would be more expensive on 32bit archs, i386
> uses the horridly expensive cmpxchg8b thing to implement it.
Ugh, yes. And some of those paths seem to be hot-paths too.
Perhaps more importantly, there are way more accesses to that
'sum_exec_runtime' than the spinlock-variant of the patch implied.
So now with the atomic64 variant, the readers are protected too, and
that ends up being really expensive. That may be the "right thing" to
do, but I'm not sure if it's really acceptable. Also, I see that some
of the atomic regions (that weren't protected by the spinlock *either*
aren't just simple adds: they are code like
+ if (atomic64_read(&cputime_expires->sched_exp) == 0 ||
+ atomic64_read(&cputime_expires->sched_exp)
> exp->sched)
+
atomic64_set(&cputime_expires->sched_exp, exp->sched);
in arm_timer(), which was apparently totally unprotected before, and
which is just inappropriate with atomic accesses.
So seeing this, I'm not confident that atomic64 works at all, after all.
Grrr..
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists