public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	Wanpeng Li <wanpeng.li@hotmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@kernel.org>,
	Fenghua Yu <fenghua.yu@intel.com>, Rik van Riel <riel@redhat.com>,
	Stanislaw Gruszka <sgruszka@redhat.com>
Subject: Re: [PATCH 09/10] s390/cputime: delayed accounting of system time
Date: Tue, 13 Dec 2016 12:13:22 +0100	[thread overview]
Message-ID: <20161213121322.5fdbbb28@mschwideX1> (raw)
In-Reply-To: <20161212150228.GA17032@lerouge>

On Mon, 12 Dec 2016 16:02:30 +0100
Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Mon, Dec 12, 2016 at 11:27:54AM +0100, Martin Schwidefsky wrote:
> > 3) The call to vtime_flush in account_process_tick is done in irq context from
> >    update_process_times. hardirq_offset==1 is also correct.  
> 
> Let's see this for example:
> 
> +       if ((tsk->flags & PF_VCPU) && (irq_count() - hardirq_offset == 0))
> +               S390_lowcore.guest_timer += timer;
> 
> If the tick is interrupting guest, we have accounted the guest time on tick IRQ entry.
> Now we are in the middle of the tick interrupt and since hardirq_offset is 1, we
> are taking the above path by accounting half of the tick-IRQ time as guest, which is wrong,
> it's actually IRQ time.

Hmm, you got me there. The system time from irq_enter until account_process_tick
is reached is indeed IRQ time. It is not much but it is incorrect. The best fix
would be to rip out the accounting of the system time from account_process_tick
as irq_enter / irq_exit will do system time accounting anyway. To do that
do_account_vtime needs to be split, because for the task switch we need to
account the system time of the previous task.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

  reply	other threads:[~2016-12-13 11:13 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-06  2:32 [PATCH 00/10] vtime: Delay cputime accounting to tick Frederic Weisbecker
2016-12-06  2:32 ` [FIX][PATCH 01/10] powerpc32: Fix stale scaled stime on context switch Frederic Weisbecker
2016-12-06  2:32 ` [FIX][PATCH 02/10] ia64: Fix wrong start cputime assignment on task switch Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 03/10] cputime: Allow accounting system time using cpustat index Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 04/10] cputime: Export account_guest_time Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 05/10] powerpc: Prepare accounting structure for cputime flush on tick Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 06/10] powerpc: Migrate stolen_time field to accounting structure Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 07/10] powerpc/vtime: Accumulate cputime and account only on tick/task switch Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 08/10] ia64: " Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 09/10] s390/cputime: delayed accounting of system time Frederic Weisbecker
2016-12-10  1:48   ` Frederic Weisbecker
2016-12-12 10:27     ` Martin Schwidefsky
2016-12-12 15:02       ` Frederic Weisbecker
2016-12-13 11:13         ` Martin Schwidefsky [this message]
2016-12-13 13:21           ` Martin Schwidefsky
2016-12-14  1:44             ` Frederic Weisbecker
2016-12-20 14:13               ` Martin Schwidefsky
2016-12-20 14:30                 ` Frederic Weisbecker
2016-12-13 14:38           ` Frederic Weisbecker
2016-12-06  2:32 ` [PATCH 10/10] vtime: Rename vtime_account_user() to vtime_flush() Frederic Weisbecker
2016-12-06  4:20 ` [PATCH 00/10] vtime: Delay cputime accounting to tick Paul Mackerras
2016-12-06  7:04   ` Martin Schwidefsky
2016-12-06 14:34   ` Frederic Weisbecker
2016-12-06  8:40 ` Christian Borntraeger
  -- strict thread matches above, loose matches on Subject: below --
2017-01-05 17:11 [PATCH 00/10] vtime: Delay cputime accounting to tick / context switch Frederic Weisbecker
2017-01-05 17:11 ` [PATCH 09/10] s390/cputime: delayed accounting of system time Frederic Weisbecker

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=20161213121322.5fdbbb28@mschwideX1 \
    --to=schwidefsky@de.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=fenghua.yu@intel.com \
    --cc=fweisbec@gmail.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=sgruszka@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=wanpeng.li@hotmail.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