Hi, Ingo and Srikar,
> -----Original Message-----
> From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, April 06, 2016 2:55 PM
> To: Srikar Dronamraju <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [tip:sched/core] sched/cpuacct: Split usage accounting into
> user_usage and sys_usage
>
>
> * Srikar Dronamraju <[email protected]> wrote:
>
> > * tip-bot for Dongsheng Yang <[email protected]> [2016-03-31 02:27:39]:
> >
> > > Commit-ID: d740037fac7052e49450f6fa1454f1144a103b55
> > > Gitweb:
> http://git.kernel.org/tip/d740037fac7052e49450f6fa1454f1144a103b55
> > > Author: Dongsheng Yang <[email protected]>
> > > AuthorDate: Tue, 22 Mar 2016 16:37:08 +0800
> > > Committer: Ingo Molnar <[email protected]>
> > > CommitDate: Thu, 31 Mar 2016 10:48:54 +0200
> > >
> > > sched/cpuacct: Split usage accounting into user_usage and sys_usage
> > >
> > > Sometimes, cpuacct.usage is not detailed enough to see how much CPU
> > > usage a group had. We want to know how much time it used in user mode
> > > and how much in kernel mode.
> > >
> >
> > Unfortunately this commit causes boot to fail on my power 7 box.
> >
> > > @@ -238,10 +316,18 @@ static struct cftype files[] = {
> > > void cpuacct_charge(struct task_struct *tsk, u64 cputime)
> > > {
> > > struct cpuacct *ca;
> > > + int index;
> > > +
> > > + if (user_mode(task_pt_regs(tsk)))
> > > + index = CPUACCT_USAGE_USER;
> > > + else
> > > + index = CPUACCT_USAGE_SYSTEM;
> > >
> > > rcu_read_lock();
> > > +
> > > for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
> > > - *this_cpu_ptr(ca->cpuusage) += cputime;
> > > + this_cpu_ptr(ca->cpuusage)->usages[index] += cputime;
> >
> > The above line seems to be the cause of oops. Attached the complete console
> log below.
>
Thanks for reporting this bug.
I'll take over this patch.
> Weird - not much changed wrt. the cpuusage logic, we only increased its size.
>
Thanks for concern.
I trying to review this patch, but had not found something strange, except:
> If you change the above loop to something like:
>
> for (ca = task_ca(tsk); ca; ca = parent_ca(ca)) {
> if (WARN_ON_ONCE(!ca->cpuusage))
> continue;
> this_cpu_ptr(ca->cpuusage)->usages[index] += cputime;
Or s/ this_cpu_ptr/get_cpu_var to avoid preempt?
But preempt problem should not cause problem in every boot,
And Srikar's bisect means it failed in every boot.
> }
>
> then do you get the warning and the bootup succeeds?
>
I also building a power7 vm for test, still in building...
Thanks
Zhaolei
