Date: Tue, 13 Dec 2016 12:13:22 +0100
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
In-Reply-To: <20161212150228.GA17032@lerouge>
References: <1480991543-6557-1-git-send-email-fweisbec@gmail.com>
        <1480991543-6557-10-git-send-email-fweisbec@gmail.com>
        <20161210014804.GA3023@lerouge>
        <20161212112754.5ad104cf@mschwideX1>
        <20161212150228.GA17032@lerouge>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Message-Id: <20161213121322.5fdbbb28@mschwideX1>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1277
Lines: 29

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.