RE: [tip:sched/core] sched/cpuacct: Split usage accounting into user_usage and sys_usage

[Zhao Lei <zhaolei@cn.fujitsu.com>]

Hi, Ingo and Srikar,

> -----Original Message-----
> From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo
> Molnar
> Sent: Wednesday, April 06, 2016 2:55 PM
> To: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: tglx@linutronix.de; efault@gmx.de; htejun@gmail.com;
> linux-kernel@vger.kernel.org; tj@kernel.org; torvalds@linux-foundation.org;
> zhaolei@cn.fujitsu.com; peterz@infradead.org; yangds.fnst@cn.fujitsu.com;
> hpa@zytor.com
> Subject: Re: [tip:sched/core] sched/cpuacct: Split usage accounting into
> user_usage and sys_usage
> 
> 
> * Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> 
> > * tip-bot for Dongsheng Yang <tipbot@zytor.com> [2016-03-31 02:27:39]:
> >
> > > Commit-ID:  d740037fac7052e49450f6fa1454f1144a103b55
> > > Gitweb:
> http://git.kernel.org/tip/d740037fac7052e49450f6fa1454f1144a103b55
> > > Author:     Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> > > AuthorDate: Tue, 22 Mar 2016 16:37:08 +0800
> > > Committer:  Ingo Molnar <mingo@kernel.org>
> > > 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