All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Li,Rongqing" <lirongqing@baidu.com>
To: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: "Peter Zijlstra" <peterz@infradead.org>,
	"mingo@redhat.com" <mingo@redhat.com>,
	"juri.lelli@redhat.com" <juri.lelli@redhat.com>,
	"vincent.guittot@linaro.org" <vincent.guittot@linaro.org>,
	"dietmar.eggemann@arm.com" <dietmar.eggemann@arm.com>,
	"rostedt@goodmis.org" <rostedt@goodmis.org>,
	"bsegall@google.com" <bsegall@google.com>,
	"mgorman@suse.de" <mgorman@suse.de>,
	"bristot@redhat.co" <bristot@redhat.co>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"songmuchun@bytedance.com" <songmuchun@bytedance.com>,
	"Michal Koutný" <mkoutny@suse.com>
Subject: 答复: 答复: [PATCH][RFC] sched/cpuacct: Fix cpuacct charge
Date: Tue, 17 Aug 2021 03:55:08 +0000	[thread overview]
Message-ID: <06265d4d0df7410786d0ddb9bdbc4e4c@baidu.com> (raw)
In-Reply-To: <20210816162103.dup2hbho24wtmsgs@oracle.com>

> > diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c index
> > 893eece..48b117e 100644
> > --- a/kernel/sched/cpuacct.c
> > +++ b/kernel/sched/cpuacct.c
> > @@ -340,7 +340,12 @@ void cpuacct_charge(struct task_struct *tsk, u64
> > cputime)  {
> >         struct cpuacct *ca;
> >         int index = CPUACCT_STAT_SYSTEM;
> > -       struct pt_regs *regs = get_irq_regs() ? : task_pt_regs(tsk);
> > +       struct pt_regs *regs;
> > +
> > +       if (task_cpu(tsk) == raw_smp_processor_id())
> > +               regs = get_irq_regs() ? : task_pt_regs(tsk);
> > +       else
> > +               regs = task_pt_regs(tsk);
> >
> >         if (regs && user_mode(regs))
> >                 index = CPUACCT_STAT_USER;
> 
> It still suffers from task_pt_regs().
> 
> Why not make cpuacct use cgroup2's approach?  Remember only delta_exec
> here, then on reading cpuacct.usage_*, use cputime_adjust() to scale the
> user/sys from cpuacct_account_field().
> 

I think your suggestion is reasonable, Could you send a patch


> It's arguably more than just a fix for cgroup1, but there have been a few
> complaints about this function lately.
> 
> >         rcu_read_lock();
> >
> >         for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
> >               __this_cpu_add(ca->cpuusage->usages[index], cputime);
> >
> >         rcu_read_unlock();
> 
> By the way, I think the __this_cpu_add() can be wrong in cases like you originally
> describe.  Seems like a bug in 73e6aafd9ea8 ("sched/cpuacct:
> Simplify the cpuacct code").

We find this issue too.

-Li

  reply	other threads:[~2021-08-17  3:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-20  6:04 [PATCH][RFC] sched/cpuacct: Fix cpuacct charge Li RongQing
2021-07-20 18:11 ` kernel test robot
2021-07-21  4:23 ` kernel test robot
2021-07-29 10:19 ` Peter Zijlstra
2021-07-30  8:16   ` 答复: " Li,Rongqing
2021-08-16 16:21     ` Daniel Jordan
2021-08-17  3:55       ` Li,Rongqing [this message]
2021-08-18 15:33         ` 答复: " Daniel Jordan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=06265d4d0df7410786d0ddb9bdbc4e4c@baidu.com \
    --to=lirongqing@baidu.com \
    --cc=bristot@redhat.co \
    --cc=bsegall@google.com \
    --cc=daniel.m.jordan@oracle.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=mkoutny@suse.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=songmuchun@bytedance.com \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.