Messages in this thread | | | Date | Tue, 15 Jan 2013 18:19:58 +0800 | Subject | Re: [PATCH v5 03/11] cgroup, sched: let cpu serve the same files as cpuacct | From | Sha Zhengju <> |
| |
On Mon, Jan 14, 2013 at 10:55 PM, Glauber Costa <glommer@parallels.com> wrote: > On 01/14/2013 12:34 AM, Sha Zhengju wrote: >>> + struct kernel_cpustat *kcpustat = this_cpu_ptr(ca->cpustat); >>> > + >>> > kcpustat = this_cpu_ptr(ca->cpustat); >> Is this reassignment unnecessary? >> >> > No. >
No? No!
In task_group_account_field(), the following two hunks have the similar behavior but different codes, there must be a trial in one of them.
Hunk #1: +#ifdef CONFIG_CGROUP_SCHED + rcu_read_lock(); + tg = container_of(task_subsys_state(p, cpu_cgroup_subsys_id), + struct task_group, css);
+ while (tg && (tg != &root_task_group)) { + struct kernel_cpustat *kcpustat = this_cpu_ptr(tg->cpustat); **HERE** + + kcpustat->cpustat[index] += tmp; + tg = tg->parent; + } + rcu_read_unlock(); +#endif
Hunk #2: #ifdef CONFIG_CGROUP_CPUACCT if (unlikely(!cpuacct_subsys.active)) return;
rcu_read_lock(); ca = task_ca(p); while (ca && (ca != &root_cpuacct)) { + struct kernel_cpustat *kcpustat = this_cpu_ptr(ca->cpustat); **HERE** + kcpustat = this_cpu_ptr(ca->cpustat); **HERE, which is unnecessary** kcpustat->cpustat[index] += tmp; ca = parent_ca(ca); } rcu_read_unlock(); #endif
Also you can prove it by the following testcase. #include <stdio.h>
int main(void) { int i = 0; int array[10] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; int *index = &array[0];
while (i < 10) { int *ptr = index;
printf("ptr=%d %p, index = %d\n", *ptr, ptr, *index); index ++; i++; sleep(1); }
return 0; }
-- Thanks, Sha
| |