On Wednesday 19 May 2010 21:02:23 Peter Zijlstra wrote:
> On Wed, 2010-05-19 at 20:58 +0200, Thomas Renninger wrote:
> > and avoid locking on 32 bit.
> > This resolves an ugly dependency in cgroups_cpuaccount.c
> > to per_cpu runqueues lock variable.
>
> While I totally agree with the sentiments here, atomic64_t isn't
> available on all 32bit archs and is _terribly_ expensive on ARCH=i386.
Please ignore the 2nd patch.
If nobody objects about the creation of kernel/sched.h which should get
used for further cleanups IMO, the first patch should get in.
I thought a bit about it, but taking the per_cpu rq lock is the most sane
thing I could think of.
Here is a fix (bug always was there), for "on top" patching of the first.
It should also be possible to fix this by exchanging:
CONFIG_64BIT to CONFIG_X86_64, but removing the #ifdefs should
be preferred over a lock that is only grabbed via sysfs access...
Thanks,
Thomas
scheduler: cgroup_cpuaccount - Fix race on cpuusage
A u64 access is not atomic on all 64 bit archs.
Especially this:
*cpuusage += cputime;
should not necessarily be atomic on non X86_64 archs.
As cpuacct_cpuusage_{read,write} is only used during
sysfs access, the lock should be acquired rather rarely
and the code is nicer as well.
Signed-off-by: Thomas Renninger <trenn@xxxxxxx>
CC: linux-kernel@xxxxxxxxxxxxxxx
CC: mike@xxxxxxxxxxx
CC: menage@xxxxxxxxxx
CC: lizf@xxxxxxxxxxxxxx
CC: containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
CC: mingo@xxxxxxx
CC: peterz@xxxxxxxxxxxxx
diff --git a/kernel/cgroup_cpuaccount.c b/kernel/cgroup_cpuaccount.c
index 0ad356a..a9e0345 100644
--- a/kernel/cgroup_cpuaccount.c
+++ b/kernel/cgroup_cpuaccount.c
@@ -95,16 +95,12 @@ static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
u64 data;
-#ifndef CONFIG_64BIT
/*
- * Take rq->lock to make 64-bit read safe on 32-bit platforms.
+ * Take rq->lock to make 64-bit read safe
*/
lock_runqueue(cpu);
data = *cpuusage;
unlock_runqueue(cpu);
-#else
- data = *cpuusage;
-#endif
return data;
}
@@ -113,16 +109,12 @@ static void cpuacct_cpuusage_write(struct cpuacct *ca, int cpu, u64 val)
{
u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
-#ifndef CONFIG_64BIT
/*
- * Take rq->lock to make 64-bit write safe on 32-bit platforms.
+ * Take rq->lock to make 64-bit write safe
*/
lock_runqueue(cpu);
*cpuusage = val;
unlock_runqueue(cpu);
-#else
- *cpuusage = val;
-#endif
}
/* return total cpu usage (in nanoseconds) of a group */
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
