mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Venkatesh Pallipadi <venki@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@elte.hu>, "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	linux-kernel@vger.kernel.org, Paul Turner <pjt@google.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Shaun Ruffell <sruffell@digium.com>,
	Yong Zhang <yong.zhang0@gmail.com>
Subject: Re: [PATCH 5/6] Export ns irqtimes through /proc/stat -v1
Date: Tue, 26 Oct 2010 10:35:04 -0700	[thread overview]
Message-ID: <AANLkTimFQvszKyXPOSjjQ9ohBd1fSspiGhsqs96Ly2mr@mail.gmail.com> (raw)
In-Reply-To: <1288105046.15336.343.camel@twins>

On Tue, Oct 26, 2010 at 7:57 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, 2010-10-26 at 11:45 +0200, Peter Zijlstra wrote:
>> On Mon, 2010-10-25 at 15:30 -0700, Venkatesh Pallipadi wrote:
>> > +static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
>> > +                                               struct rq *rq)
>> > +{
>> > +       cputime_t one_jiffy_scaled = cputime_to_scaled(cputime_one_jiffy);
>> > +       cputime64_t tmp = cputime_to_cputime64(cputime_one_jiffy);
>> > +       struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat;
>> > +
>> > +       if (irqtime_account_hi_update()) {
>> > +               cpustat->irq = cputime64_add(cpustat->irq, tmp);
>> > +       } else if (irqtime_account_si_update()) {
>> > +               cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
>> > +       } else
>>
>> I'm still not sure about this else stmt, the above two conditions can
>> basically 'eat' user/system ticks. What we need to show is that there is
>> no bias towards either kind so the ratio is not affected -- can we make
>> such an argument?
>
> I think I can made a counter-argument: if either or both of these checks
> are true we had system time in the last tick, hence there is a larger
> chance this tick is a system tick.
>
> Therefore it will not provide the same user/system ratio.
>
> Hmm?
>

This is about task user/system time. Right?
With the earlier changes, hardirq/softirq won't be part of task
sum_exec_runtime anymore. So, if we have had significant hardirq or
softirq during this tick, then the task sum_exec_runtime wouldn't have
changed much. So, accounting this as task system time will tweak task
user/system ratio needlessly. So, eating up system time should be the
right thing to do here :-)

The other case of significant hardirq/softirq during last tick, but
not enough to cause the irq folding and so may get accounted as
system/user and we have a tiny hardirq/softirq in the next tick which
results in folding and so system/user wont be accounted. So, this can
affect user/system ratio if last tick was user and current tick is
system or vice-versa. This I feel, is no different from variation with
tick instance based sampling we have now.

Only other option I can think of, is to forget about this folding
business and micro account hardirq/softirq fraction on every tick and
account the remaining time (one_tick - (hardirq+softirq)) as
user/idle/guest/system. The problem with that is the task times and
kstat are done in cputime64, which in x86 is jiffies and so we dont
have enough resolution for this approach.

Thanks,
Venki

>> >                if (user_tick) {
>> > +               account_user_time(p, cputime_one_jiffy, one_jiffy_scaled);
>> > +       } else if (p == rq->idle) {
>> > +               account_idle_time(cputime_one_jiffy);
>> > +       } else if (p->flags & PF_VCPU) { /* System time or guest time */
>> > +               account_guest_time(p, cputime_one_jiffy, one_jiffy_scaled);
>> > +       } else {
>> > +               __account_system_time(p, cputime_one_jiffy, one_jiffy_scaled,
>> > +                                       &cpustat->system);
>> > +       }
>> > +}
>
>

  reply	other threads:[~2010-10-26 17:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-25 22:30 [PATCH 0/6] Proper kernel irq time reporting -v1 Venkatesh Pallipadi
2010-10-25 22:30 ` [PATCH 1/6] Free up pf flag PF_KSOFTIRQD -v1 Venkatesh Pallipadi
2010-10-25 22:30 ` [PATCH 2/6] cleanup account_system_vtime with this_cpu_* -v1 Venkatesh Pallipadi
2010-10-25 22:30 ` [PATCH 3/6] Add nsecs_to_cputime64 interface for asm-generic -v1 Venkatesh Pallipadi
2010-10-25 22:30 ` [PATCH 4/6] Refactor account_system_time separating id-update -v1 Venkatesh Pallipadi
2010-10-25 22:30 ` [PATCH 5/6] Export ns irqtimes through /proc/stat -v1 Venkatesh Pallipadi
2010-10-26  9:45   ` Peter Zijlstra
2010-10-26 14:57     ` Peter Zijlstra
2010-10-26 17:35       ` Venkatesh Pallipadi [this message]
2010-10-25 22:30 ` [PATCH 6/6] Account ksoftirqd time as cpustat softirq -v1 Venkatesh Pallipadi
2010-10-26  9:33   ` Peter Zijlstra
2010-10-26  9:50     ` Peter Zijlstra
2010-10-26 17:35       ` Venkatesh Pallipadi

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=AANLkTimFQvszKyXPOSjjQ9ohBd1fSspiGhsqs96Ly2mr@mail.gmail.com \
    --to=venki@google.com \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=schwidefsky@de.ibm.com \
    --cc=sruffell@digium.com \
    --cc=tglx@linutronix.de \
    --cc=yong.zhang0@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®