On Mon, Dec 12, 2016 at 11:27:54AM +0100, Martin Schwidefsky wrote:
> On Sat, 10 Dec 2016 02:48:06 +0100
> Frederic Weisbecker <[email protected]> wrote:
> > We should get rid of the hardirq_offset argument, it doesn't really make
> > sense
> > anymore. Also it makes the accounting buggy now. It's called from the tick
> > through account_user_time() with hardirq_offset=1, so the irq time is
> > incorrectly
> > accumulated as system time. Guest time may be incorrect too.
> >
> > In fact it may have been buggy even before this patchset because
> > vtime_account_user()
> > isn't only called from the tick but also from task switch, and
> > hardirq_offset remains 1
> > for those two cases. Not good.
>
> For s390 the do_account_vtime function is called from vtime_task_switch and
> vtime_flush.
> 1) vtime_task_switch is exclusively called from finish_task_switch outside of
> irq context.
> The call to do_account_vtime with hardirq_offset==0 from vtime_task_switch
> is correct.
Yes that one is fine.
> 2) The call to vtime_flush in vtime_common_task_switch is irrelevant for s390
> as we
> define __ARCH_HAS_VTIME_TASK_SWITCH
That's right, I missed that. And now I remember that special case remains
because s390 has its
own way to account idle time.
> 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.
> > > void vtime_task_switch(struct task_struct *prev)
> > > {
> > > do_account_vtime(prev, 0);
> >
> > This call should be removed, the task switch already calls
> > vtime_account_user().
>
> The vtime_account_user function is empty for s390..
That's right.
