Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1610014 > unrolled thread

Re: [BUG nohz]: wrong user and system time accounting

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-03-27 19:40 +0200
    Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-28 09:30 +0200
    Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-03-28 23:10 +0200
      Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-03-28 23:30 +0200
        Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-29 12:00 +0200
          Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-29 15:00 +0200
    Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-03-28 23:30 +0200
      Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-03-28 23:40 +0200
    Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-03-29 22:20 +0200
      Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-30 01:00 +0200
        Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-03-30 15:00 +0200
      Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-30 04:00 +0200
        Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-30 14:50 +0200
          Re: [BUG nohz]: wrong user and system time accounting Mike Galbraith <efault@gmx.de> - 2017-03-30 15:20 +0200
      Re: [BUG nohz]: wrong user and system time accounting Mike Galbraith <efault@gmx.de> - 2017-03-30 06:30 +0200
        Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-30 08:50 +0200
          Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-30 14:00 +0200
            Re: [BUG nohz]: wrong user and system time accounting Mike Galbraith <efault@gmx.de> - 2017-03-30 14:40 +0200
          Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-30 15:40 +0200
            Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-30 16:10 +0200
              Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-30 16:30 +0200
                Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-03-30 23:30 +0200
                  Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-03-31 22:10 +0200
                    Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-04-01 01:30 +0200
                      Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-04-01 05:20 +0200
                        Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-04-03 17:30 +0200
                          Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-04-03 21:10 +0200
                            Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-04-04 20:10 +0200
                              Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-04-05 16:30 +0200
        Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-30 15:00 +0200
          Re: [BUG nohz]: wrong user and system time accounting Rik van Riel <riel@redhat.com> - 2017-03-30 15:10 +0200
            Re: [BUG nohz]: wrong user and system time accounting Mike Galbraith <efault@gmx.de> - 2017-03-30 15:40 +0200
              Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-04-03 16:50 +0200
                Re: [BUG nohz]: wrong user and system time accounting Mike Galbraith <efault@gmx.de> - 2017-04-04 09:40 +0200
            Re: [BUG nohz]: wrong user and system time accounting Frederic Weisbecker <fweisbec@gmail.com> - 2017-03-30 15:50 +0200
    Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-30 00:50 +0200
      Re: [BUG nohz]: wrong user and system time accounting Luiz Capitulino <lcapitulino@redhat.com> - 2017-03-30 04:20 +0200
        Re: [BUG nohz]: wrong user and system time accounting Wanpeng Li <kernellwp@gmail.com> - 2017-03-30 14:30 +0200

Page 1 of 2  [1] 2  Next page →

#1610014 — Re: [BUG nohz]: wrong user and system time accounting

On Mon, 2017-03-27 at 09:56 +0800, Wanpeng Li wrote:
> 
> Actually after I bisect, the first bad commit is ff9a9b4c4334
> ("sched,
> time: Switch VIRT_CPU_ACCOUNTING_GEN to jiffy granularity"). The bug
> can be reproduced readily if CONFIG_CONTEXT_TRACKING_FORCE is true

At the time, we thought it was an "occasionally bad" / "unlucky"
kind of bug, not a systemic issue, like your observations seem
to suggest.

> Let's consider the cpu which has responsibility for the global
> timekeeping, as the tracing posted above, the vtime_account_user() is
> called before tick_sched_timer() which will update jiffies, so
> jiffies
> is stale in vtime_account_user() and the run time in userspace is
> skipped, the vtime_user_enter() is called after jiffies update, so
> both the time in userspace and in  kernel are accumulated to sys
> time.
> If the housekeeping cpu is idle when CONFIG_NO_HZ_FULL, everything is
> fine. However, if you give stress to the housekeeping cpu, top will
> show 100% sys-time of both the housekeeping cpu and the other cpus
> who
> have at least two tasks running on and in full_nohz mode. I think it
> is because the stress delays the timer interrupt handling in some
> degree, then the jiffies is not updated timely before other cpus
> access it in vtime_account_user().
> 
> I think we can keep syscalls/exceptions context tracking still in
> jiffies based sampling and utilize local_clock() in vtime_delta()
> again for irqs which avoids jiffies stale influence. I can make a
> patch if the idea is acceptable or there is any better proposal. :)

Making that patch seems worthwhile, but I would like to
know what the root cause is of the issue that is being
observed.

Is the problem due to the nohz_full CPU receiving an
interrupt at the same time the timer interrupt fires on
the housekeeping CPU?

Is it due to a nohz_full CPU updating jiffies all by
itself from irq context?  In that case, could it be
better to always have that be done by the housekeeping
CPU?

What exactly is going on here?

[toc] | [next] | [standalone]

#1610378

2017-03-28 1:35 GMT+08:00 Rik van Riel <riel@redhat.com>:
> On Mon, 2017-03-27 at 09:56 +0800, Wanpeng Li wrote:
>>
>> Actually after I bisect, the first bad commit is ff9a9b4c4334
>> ("sched,
>> time: Switch VIRT_CPU_ACCOUNTING_GEN to jiffy granularity"). The bug
>> can be reproduced readily if CONFIG_CONTEXT_TRACKING_FORCE is true
>
> At the time, we thought it was an "occasionally bad" / "unlucky"
> kind of bug, not a systemic issue, like your observations seem
> to suggest.
>
>> Let's consider the cpu which has responsibility for the global
>> timekeeping, as the tracing posted above, the vtime_account_user() is
>> called before tick_sched_timer() which will update jiffies, so
>> jiffies
>> is stale in vtime_account_user() and the run time in userspace is
>> skipped, the vtime_user_enter() is called after jiffies update, so
>> both the time in userspace and in  kernel are accumulated to sys
>> time.
>> If the housekeeping cpu is idle when CONFIG_NO_HZ_FULL, everything is
>> fine. However, if you give stress to the housekeeping cpu, top will
>> show 100% sys-time of both the housekeeping cpu and the other cpus
>> who
>> have at least two tasks running on and in full_nohz mode. I think it
>> is because the stress delays the timer interrupt handling in some
>> degree, then the jiffies is not updated timely before other cpus
>> access it in vtime_account_user().
>>
>> I think we can keep syscalls/exceptions context tracking still in
>> jiffies based sampling and utilize local_clock() in vtime_delta()
>> again for irqs which avoids jiffies stale influence. I can make a
>> patch if the idea is acceptable or there is any better proposal. :)
>
> Making that patch seems worthwhile, but I would like to
> know what the root cause is of the issue that is being
> observed.
>
> Is the problem due to the nohz_full CPU receiving an
> interrupt at the same time the timer interrupt fires on
> the housekeeping CPU?
>
> Is it due to a nohz_full CPU updating jiffies all by
> itself from irq context?  In that case, could it be
> better to always have that be done by the housekeeping
> CPU?

I observed that the jiffies is always updated by housekeeping CPU as
we expected.

Regards,
Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1611361

On Tue, 2017-03-28 at 16:14 -0400, Luiz Capitulino wrote:
> On Tue, 28 Mar 2017 13:24:06 -0400
> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > I'm starting to suspect that the nohz code may be programming
> > the tick period to be shorter than 1ms when it re-activates
> > the tick.
> 
> And I think I was right, it looks like the nohz code is programming
> the tick period incorrectly when restarting the tick. The patch below
> fixes things for me, but I still have some homework todo and more
> testing before posting a patch for inclusion. Could you guys test it?

Your patch seems to work. I don't claim to understand why
your patch makes a difference, but for this particular test
case, on this particular setup, it seems to work...

> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 7fe53be..9abe979 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -1152,6 +1152,7 @@ static enum hrtimer_restart
> tick_sched_timer(struct hrtimer *timer)
>         struct pt_regs *regs = get_irq_regs();
>         ktime_t now = ktime_get();
>  
> +       ts->last_tick = now;
>         tick_sched_do_timer(now);
>  
>         /*

[toc] | [prev] | [next] | [standalone]

#1611370

On Tue, 28 Mar 2017 17:01:52 -0400
Rik van Riel <riel@redhat.com> wrote:

> On Tue, 2017-03-28 at 16:14 -0400, Luiz Capitulino wrote:
> > On Tue, 28 Mar 2017 13:24:06 -0400
> > Luiz Capitulino <lcapitulino@redhat.com> wrote:  
> > > I'm starting to suspect that the nohz code may be programming
> > > the tick period to be shorter than 1ms when it re-activates
> > > the tick.  
> > 
> > And I think I was right, it looks like the nohz code is programming
> > the tick period incorrectly when restarting the tick. The patch below
> > fixes things for me, but I still have some homework todo and more
> > testing before posting a patch for inclusion. Could you guys test it?  
> 
> Your patch seems to work. I don't claim to understand why
> your patch makes a difference, but for this particular test
> case, on this particular setup, it seems to work...

I don't fully understand why either yet. I was looking for places
where nohz might be programming the tick period incorrectly and
I found that there's a case in tick_nohz_stop_sched_tick() where
tick_nohz_restart() is called only to reprogram the tick timer,
not cancel the tick. In this case, ts->last_tick seems to be out
of date. Fixing this fixed accounting for me.

> > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> > index 7fe53be..9abe979 100644
> > --- a/kernel/time/tick-sched.c
> > +++ b/kernel/time/tick-sched.c
> > @@ -1152,6 +1152,7 @@ static enum hrtimer_restart
> > tick_sched_timer(struct hrtimer *timer)
> >         struct pt_regs *regs = get_irq_regs();
> >         ktime_t now = ktime_get();
> >  
> > +       ts->last_tick = now;
> >         tick_sched_do_timer(now);
> >  
> >         /*  
> 

[toc] | [prev] | [next] | [standalone]

#1611770

2017-03-29 5:26 GMT+08:00 Luiz Capitulino <lcapitulino@redhat.com>:
> On Tue, 28 Mar 2017 17:01:52 -0400
> Rik van Riel <riel@redhat.com> wrote:
>
>> On Tue, 2017-03-28 at 16:14 -0400, Luiz Capitulino wrote:
>> > On Tue, 28 Mar 2017 13:24:06 -0400
>> > Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> > > I'm starting to suspect that the nohz code may be programming
>> > > the tick period to be shorter than 1ms when it re-activates
>> > > the tick.
>> >
>> > And I think I was right, it looks like the nohz code is programming
>> > the tick period incorrectly when restarting the tick. The patch below
>> > fixes things for me, but I still have some homework todo and more
>> > testing before posting a patch for inclusion. Could you guys test it?
>>
>> Your patch seems to work. I don't claim to understand why
>> your patch makes a difference, but for this particular test
>> case, on this particular setup, it seems to work...
>
> I don't fully understand why either yet. I was looking for places
> where nohz might be programming the tick period incorrectly and

The bug is still present when I config CONTEXT_TRACKING_FORCE and
nohz=off in the boot parameter.

Regards,
Wanpeng Li

> I found that there's a case in tick_nohz_stop_sched_tick() where
> tick_nohz_restart() is called only to reprogram the tick timer,
> not cancel the tick. In this case, ts->last_tick seems to be out
> of date. Fixing this fixed accounting for me.
>
>> > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>> > index 7fe53be..9abe979 100644
>> > --- a/kernel/time/tick-sched.c
>> > +++ b/kernel/time/tick-sched.c
>> > @@ -1152,6 +1152,7 @@ static enum hrtimer_restart
>> > tick_sched_timer(struct hrtimer *timer)
>> >         struct pt_regs *regs = get_irq_regs();
>> >         ktime_t now = ktime_get();
>> >
>> > +       ts->last_tick = now;
>> >         tick_sched_do_timer(now);
>> >
>> >         /*
>>
>

[toc] | [prev] | [next] | [standalone]

#1611906

On Wed, Mar 29, 2017 at 05:56:30PM +0800, Wanpeng Li wrote:
> 2017-03-29 5:26 GMT+08:00 Luiz Capitulino <lcapitulino@redhat.com>:
> > On Tue, 28 Mar 2017 17:01:52 -0400
> > Rik van Riel <riel@redhat.com> wrote:
> >
> >> On Tue, 2017-03-28 at 16:14 -0400, Luiz Capitulino wrote:
> >> > On Tue, 28 Mar 2017 13:24:06 -0400
> >> > Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >> > > I'm starting to suspect that the nohz code may be programming
> >> > > the tick period to be shorter than 1ms when it re-activates
> >> > > the tick.
> >> >
> >> > And I think I was right, it looks like the nohz code is programming
> >> > the tick period incorrectly when restarting the tick. The patch below
> >> > fixes things for me, but I still have some homework todo and more
> >> > testing before posting a patch for inclusion. Could you guys test it?
> >>
> >> Your patch seems to work. I don't claim to understand why
> >> your patch makes a difference, but for this particular test
> >> case, on this particular setup, it seems to work...
> >
> > I don't fully understand why either yet. I was looking for places
> > where nohz might be programming the tick period incorrectly and
> 
> The bug is still present when I config CONTEXT_TRACKING_FORCE and
> nohz=off in the boot parameter.

Indeed I saw something similar a few days ago with:

    !CONFIG_NO_HZ_FULL && CONFIG_VIRT_CPU_ACCOUNTING_GEN && CONTEXT_TRACKING_FORCE

And it disappeared with CONFIG_NO_HZ_FULL=y so I didn't care much because that setting
isn't used in production and in fact I intend to remove CONTEXT_TRACKING_FORCE. But
it could be the sign of something important.

It might be different than Luiz's bug because I can't reproduce his bug yet even with
his config.

[toc] | [prev] | [next] | [standalone]

#1611368

On Tue, 2017-03-28 at 16:14 -0400, Luiz Capitulino wrote:

> And I think I was right, it looks like the nohz code is programming
> the tick period incorrectly when restarting the tick. The patch below
> fixes things for me, but I still have some homework todo and more
> testing before posting a patch for inclusion. Could you guys test it?

I spoke too soon.  After half an hour of runtime,
things have gotten aligned to give me about 50/50
user time and system time with your test case,
again.

This is on an 8 VCPU virtual machine, with
nohz_full=2-7, and the test case running on one
of the nohz_full CPUs.

> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 7fe53be..9abe979 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -1152,6 +1152,7 @@ static enum hrtimer_restart
> tick_sched_timer(struct hrtimer *timer)
>         struct pt_regs *regs = get_irq_regs();
>         ktime_t now = ktime_get();
>  
> +       ts->last_tick = now;
>         tick_sched_do_timer(now);
>  
>         /*

[toc] | [prev] | [next] | [standalone]

#1611373

On Tue, 28 Mar 2017 17:24:11 -0400
Rik van Riel <riel@redhat.com> wrote:

> On Tue, 2017-03-28 at 16:14 -0400, Luiz Capitulino wrote:
> 
> > And I think I was right, it looks like the nohz code is programming
> > the tick period incorrectly when restarting the tick. The patch below
> > fixes things for me, but I still have some homework todo and more
> > testing before posting a patch for inclusion. Could you guys test it?  
> 
> I spoke too soon.  After half an hour of runtime,
> things have gotten aligned to give me about 50/50
> user time and system time with your test case,
> again.

Hmmm, maybe it's incomplete. I still think that nohz might screwing
something up when re-activating the tick.

> 
> This is on an 8 VCPU virtual machine, with
> nohz_full=2-7, and the test case running on one
> of the nohz_full CPUs.
> 
> > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> > index 7fe53be..9abe979 100644
> > --- a/kernel/time/tick-sched.c
> > +++ b/kernel/time/tick-sched.c
> > @@ -1152,6 +1152,7 @@ static enum hrtimer_restart
> > tick_sched_timer(struct hrtimer *timer)
> >         struct pt_regs *regs = get_irq_regs();
> >         ktime_t now = ktime_get();
> >  
> > +       ts->last_tick = now;
> >         tick_sched_do_timer(now);
> >  
> >         /*  
> 

[toc] | [prev] | [next] | [standalone]

#1612290

On Wed, 2017-03-29 at 13:16 -0400, Luiz Capitulino wrote:
> On Tue, 28 Mar 2017 13:24:06 -0400
> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> 
> >  1. In my tracing I'm seeing that sometimes (always?) the
> >     time interval between two timer interrupts is less than 1ms
> 
> I think that's the root cause.
> 
> In this trace, we see the following:
> 
>  1. On CPU15, we transition from user-space to kernel-space because
>     of a timer interrupt (it's the tick)
> 
>  2. vtimer_delta() returns 0, because jiffies didn't change since the
>     last accounting
> 
>  3. While CPU15 is executing in kernel-space, jiffies is updated
>     by CPU0
> 
>  4. When going back to user-space, vtime_delta() returns non-zero
>     and the whole time is accounted for system time (observe how
>     the cputime parameter in account_system_time() is less than 1ms)

In other words, the tick on cpu0 is aligned
with the tick on the nohz_full cpus, and
jiffies is advanced while the nohz_full cpus
with an active tick happen to be in kernel
mode?

Frederic, can you think of any reason why
the tick on nohz_full CPUs would end up aligned
with the tick on cpu0, instead of running at some
random offset?

A random offset, or better yet a somewhat randomized
tick length to make sure that simultaneous ticks are
fairly rare and the vtime sampling does not end up
"in phase" with the jiffies incrementing, could make
the accounting work right again.

Of course, that assumes the above hypothesis is correct :)

[toc] | [prev] | [next] | [standalone]

#1612430

(Adding Thomas in Cc)

On Wed, Mar 29, 2017 at 04:08:45PM -0400, Rik van Riel wrote:
> On Wed, 2017-03-29 at 13:16 -0400, Luiz Capitulino wrote:
> > On Tue, 28 Mar 2017 13:24:06 -0400
> > Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > 
> > >  1. In my tracing I'm seeing that sometimes (always?) the
> > >     time interval between two timer interrupts is less than 1ms
> > 
> > I think that's the root cause.
> > 
> > In this trace, we see the following:
> > 
> >  1. On CPU15, we transition from user-space to kernel-space because
> >     of a timer interrupt (it's the tick)
> > 
> >  2. vtimer_delta() returns 0, because jiffies didn't change since the
> >     last accounting
> > 
> >  3. While CPU15 is executing in kernel-space, jiffies is updated
> >     by CPU0
> > 
> >  4. When going back to user-space, vtime_delta() returns non-zero
> >     and the whole time is accounted for system time (observe how
> >     the cputime parameter in account_system_time() is less than 1ms)
> 
> In other words, the tick on cpu0 is aligned
> with the tick on the nohz_full cpus, and
> jiffies is advanced while the nohz_full cpus
> with an active tick happen to be in kernel
> mode?

Ah you found out faster than me :-)

> Frederic, can you think of any reason why
> the tick on nohz_full CPUs would end up aligned
> with the tick on cpu0, instead of running at some
> random offset?

tick_init_jiffy_update() takes that decision to align all ticks.

I'm not sure why. I don't see anything that could depend on that
wide tick synchronization. The jiffies update itself relies on ktime
to check when to update it. So even if the tick fires a bit later
on CPU 1 than on CPU 0, the jiffies updates should stay coherent and
should never exceed 999us delay in the worst case (for HZ=1000)

Now I might overlook something.

> 
> A random offset, or better yet a somewhat randomized
> tick length to make sure that simultaneous ticks are
> fairly rare and the vtime sampling does not end up
> "in phase" with the jiffies incrementing, could make
> the accounting work right again.
> 
> Of course, that assumes the above hypothesis is correct :)

I'm not sure that randomizing the tick start per CPU would be a
right solution. Somewhere in the world you can be sure the tick
randomization of some nohz_full CPU will coincide with the tick
of CPU 0 :o)

Or we could force that tick on nohz_full CPUs to be far from
CPU 0's tick... I'm not sure such a solution would be accepted though.

[toc] | [prev] | [next] | [standalone]

#1613041

On Thu, 2017-03-30 at 00:54 +0200, Frederic Weisbecker wrote:
> (Adding Thomas in Cc)
> 
> On Wed, Mar 29, 2017 at 04:08:45PM -0400, Rik van Riel wrote:
> > 
> > Frederic, can you think of any reason why
> > the tick on nohz_full CPUs would end up aligned
> > with the tick on cpu0, instead of running at some
> > random offset?
> 
> tick_init_jiffy_update() takes that decision to align all ticks.
> 
> I'm not sure why. 

I don't see why that would matter, either.

> I'm not sure that randomizing the tick start per CPU would be a
> right solution. Somewhere in the world you can be sure the tick
> randomization of some nohz_full CPU will coincide with the tick
> of CPU 0 :o)
> 
> Or we could force that tick on nohz_full CPUs to be far from
> CPU 0's tick... I'm not sure such a solution would be accepted
> though.

I am not sure we would have to force things.

Simply getting rid of tick_init_jiffy_update
and scheduling the next tick for "now + tick
period" might have the same effect, when the
tick gets stopped and restarted on nohz_full
CPUs.

[toc] | [prev] | [next] | [standalone]

#1612492

2017-03-30 4:08 GMT+08:00 Rik van Riel <riel@redhat.com>:
> On Wed, 2017-03-29 at 13:16 -0400, Luiz Capitulino wrote:
>> On Tue, 28 Mar 2017 13:24:06 -0400
>> Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>
>> >  1. In my tracing I'm seeing that sometimes (always?) the
>> >     time interval between two timer interrupts is less than 1ms
>>
>> I think that's the root cause.
>>
>> In this trace, we see the following:
>>
>>  1. On CPU15, we transition from user-space to kernel-space because
>>     of a timer interrupt (it's the tick)
>>
>>  2. vtimer_delta() returns 0, because jiffies didn't change since the
>>     last accounting
>>
>>  3. While CPU15 is executing in kernel-space, jiffies is updated
>>     by CPU0
>>
>>  4. When going back to user-space, vtime_delta() returns non-zero
>>     and the whole time is accounted for system time (observe how
>>     the cputime parameter in account_system_time() is less than 1ms)
>
> In other words, the tick on cpu0 is aligned
> with the tick on the nohz_full cpus, and
> jiffies is advanced while the nohz_full cpus
> with an active tick happen to be in kernel
> mode?
>
> Frederic, can you think of any reason why
> the tick on nohz_full CPUs would end up aligned
> with the tick on cpu0, instead of running at some
> random offset?
>
> A random offset, or better yet a somewhat randomized
> tick length to make sure that simultaneous ticks are
> fairly rare and the vtime sampling does not end up
> "in phase" with the jiffies incrementing, could make
> the accounting work right again.
>
> Of course, that assumes the above hypothesis is correct :)

There is such a feature skew_tick currently, refer to commit
5307c9556bc (tick: add tick skew boot option), w/ skew_tick=1 boot
parameter, the bug disappear, however, the commit also mentioned that
it will hurt power consumption. I will try Frederic's proposal which
is similar to my original idea "how bad would it be to revert to
sched_clock() instead of jiffies in vtime_delta()? We could use
nanosecond granularity to check deltas but only perform an actual
cputime update when that delta >= TICK_NSEC."

Regards,
Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1613034

On Thu, Mar 30, 2017 at 09:58:44AM +0800, Wanpeng Li wrote:
> 2017-03-30 4:08 GMT+08:00 Rik van Riel <riel@redhat.com>:
> >
> > In other words, the tick on cpu0 is aligned
> > with the tick on the nohz_full cpus, and
> > jiffies is advanced while the nohz_full cpus
> > with an active tick happen to be in kernel
> > mode?
> >
> > Frederic, can you think of any reason why
> > the tick on nohz_full CPUs would end up aligned
> > with the tick on cpu0, instead of running at some
> > random offset?
> >
> > A random offset, or better yet a somewhat randomized
> > tick length to make sure that simultaneous ticks are
> > fairly rare and the vtime sampling does not end up
> > "in phase" with the jiffies incrementing, could make
> > the accounting work right again.
> >
> > Of course, that assumes the above hypothesis is correct :)
> 
> There is such a feature skew_tick currently, refer to commit
> 5307c9556bc (tick: add tick skew boot option), w/ skew_tick=1 boot
> parameter, the bug disappear, however, the commit also mentioned that
> it will hurt power consumption.

Oh, I completely missed that!

> I will try Frederic's proposal which
> is similar to my original idea "how bad would it be to revert to
> sched_clock() instead of jiffies in vtime_delta()? We could use
> nanosecond granularity to check deltas but only perform an actual
> cputime update when that delta >= TICK_NSEC."

Thanks! I hope sched_clock() won't introduce too much overhead.
Otherwise we may want to pick up the skew_tick solution.

[toc] | [prev] | [next] | [standalone]

#1613050

On Thu, 2017-03-30 at 14:40 +0200, Frederic Weisbecker wrote:
> On Thu, Mar 30, 2017 at 09:58:44AM +0800, Wanpeng Li wrote:

> > There is such a feature skew_tick currently, refer to commit
> > 5307c9556bc (tick: add tick skew boot option), w/ skew_tick=1 boot
> > parameter, the bug disappear, however, the commit also mentioned that
> > it will hurt power consumption.
> 
> Oh, I completely missed that!

It suggests it'll harm power consumption because skew removal allegedly
saved power.  Recalling what removal did to my 8 socket box, I doubt
adding it back costs large boxen anything at all, rather the opposite.

	-Mike

[toc] | [prev] | [next] | [standalone]

#1612558

On Wed, 2017-03-29 at 16:08 -0400, Rik van Riel wrote:

> In other words, the tick on cpu0 is aligned
> with the tick on the nohz_full cpus, and
> jiffies is advanced while the nohz_full cpus
> with an active tick happen to be in kernel
> mode?

You really want skew_tick=1, especially on big boxen.
 
> Frederic, can you think of any reason why
> the tick on nohz_full CPUs would end up aligned
> with the tick on cpu0, instead of running at some
> random offset?

(I or low rq->clock bits as crude NOHZ collision avoidance)

> A random offset, or better yet a somewhat randomized
> tick length to make sure that simultaneous ticks are
> fairly rare and the vtime sampling does not end up
> "in phase" with the jiffies incrementing, could make
> the accounting work right again.

That improves jitter, especially on big boxen.  I have an 8 socket box
that thinks it's an extra large PC, there, collision avoidance matters
hugely.  I couldn't reproduce bean counting woes, no idea if collision
avoidance will help that.

	-Mike

[toc] | [prev] | [next] | [standalone]

#1612609

Cc Peterz, Thomas,
2017-03-30 12:27 GMT+08:00 Mike Galbraith <efault@gmx.de>:
> On Wed, 2017-03-29 at 16:08 -0400, Rik van Riel wrote:
>
>> In other words, the tick on cpu0 is aligned
>> with the tick on the nohz_full cpus, and
>> jiffies is advanced while the nohz_full cpus
>> with an active tick happen to be in kernel
>> mode?
>
> You really want skew_tick=1, especially on big boxen.
>
>> Frederic, can you think of any reason why
>> the tick on nohz_full CPUs would end up aligned
>> with the tick on cpu0, instead of running at some
>> random offset?
>
> (I or low rq->clock bits as crude NOHZ collision avoidance)
>
>> A random offset, or better yet a somewhat randomized
>> tick length to make sure that simultaneous ticks are
>> fairly rare and the vtime sampling does not end up
>> "in phase" with the jiffies incrementing, could make
>> the accounting work right again.
>
> That improves jitter, especially on big boxen.  I have an 8 socket box
> that thinks it's an extra large PC, there, collision avoidance matters
> hugely.  I couldn't reproduce bean counting woes, no idea if collision
> avoidance will help that.

So I implement two methods, one is from Rik's random offset proposal
through skew tick, the other one is from Frederic's proposal and it is
the same as my original idea through use nanosecond granularity to
check deltas but only perform an actual cputime update when that delta
>= TICK_NSEC. Both methods can solve the bug which Luiz reported.
Peterz, Thomas, any ideas?

--------------------------->8-------------------------------------------------------------

skew tick:

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 7fe53be..9981437 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1198,7 +1198,11 @@ void tick_setup_sched_timer(void)
     hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());

     /* Offset the tick to avert jiffies_lock contention. */
+#ifdef CONFIG_NO_HZ_FULL
+    if (sched_skew_tick || tick_nohz_full_running) {
+#else
     if (sched_skew_tick) {
+#endif
         u64 offset = ktime_to_ns(tick_period) >> 1;
         do_div(offset, num_possible_cpus());
         offset *= smp_processor_id();

-------------------------------------->8-----------------------------------------------------

use nanosecond granularity to check deltas but only perform an actual
cputime update when that delta >= TICK_NSEC.

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index f3778e2b..f1ee393 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -676,18 +676,21 @@ void thread_group_cputime_adjusted(struct
task_struct *p, u64 *ut, u64 *st)
 #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
 static u64 vtime_delta(struct task_struct *tsk)
 {
-    unsigned long now = READ_ONCE(jiffies);
+    u64 now = local_clock();
+    u64 delta;
+
+    delta = now - tsk->vtime_snap;

-    if (time_before(now, (unsigned long)tsk->vtime_snap))
+    if (delta < TICK_NSEC)
         return 0;

-    return jiffies_to_nsecs(now - tsk->vtime_snap);
+    return jiffies_to_nsecs(delta / TICK_NSEC);
 }

 static u64 get_vtime_delta(struct task_struct *tsk)
 {
-    unsigned long now = READ_ONCE(jiffies);
-    u64 delta, other;
+    u64 delta = vtime_delta(tsk);
+    u64 other;

     /*
      * Unlike tick based timing, vtime based timing never has lost
@@ -696,10 +699,9 @@ static u64 get_vtime_delta(struct task_struct *tsk)
      * elapsed time. Limit account_other_time to prevent rounding
      * errors from causing elapsed vtime to go negative.
      */
-    delta = jiffies_to_nsecs(now - tsk->vtime_snap);
     other = account_other_time(delta);
     WARN_ON_ONCE(tsk->vtime_snap_whence == VTIME_INACTIVE);
-    tsk->vtime_snap = now;
+    tsk->vtime_snap += delta;

     return delta - other;
 }
@@ -776,7 +778,7 @@ void arch_vtime_task_switch(struct task_struct *prev)

     write_seqcount_begin(&current->vtime_seqcount);
     current->vtime_snap_whence = VTIME_SYS;
-    current->vtime_snap = jiffies;
+    current->vtime_snap = sched_clock_cpu(smp_processor_id());
     write_seqcount_end(&current->vtime_seqcount);
 }

@@ -787,7 +789,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)
     local_irq_save(flags);
     write_seqcount_begin(&t->vtime_seqcount);
     t->vtime_snap_whence = VTIME_SYS;
-    t->vtime_snap = jiffies;
+    t->vtime_snap = sched_clock_cpu(cpu);
     write_seqcount_end(&t->vtime_seqcount);
     local_irq_restore(flags);
 }

Regards,
Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1612988

2017-03-30 14:47 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> Cc Peterz, Thomas,
> 2017-03-30 12:27 GMT+08:00 Mike Galbraith <efault@gmx.de>:
>> On Wed, 2017-03-29 at 16:08 -0400, Rik van Riel wrote:
>>
>>> In other words, the tick on cpu0 is aligned
>>> with the tick on the nohz_full cpus, and
>>> jiffies is advanced while the nohz_full cpus
>>> with an active tick happen to be in kernel
>>> mode?
>>
>> You really want skew_tick=1, especially on big boxen.
>>
>>> Frederic, can you think of any reason why
>>> the tick on nohz_full CPUs would end up aligned
>>> with the tick on cpu0, instead of running at some
>>> random offset?
>>
>> (I or low rq->clock bits as crude NOHZ collision avoidance)
>>
>>> A random offset, or better yet a somewhat randomized
>>> tick length to make sure that simultaneous ticks are
>>> fairly rare and the vtime sampling does not end up
>>> "in phase" with the jiffies incrementing, could make
>>> the accounting work right again.
>>
>> That improves jitter, especially on big boxen.  I have an 8 socket box
>> that thinks it's an extra large PC, there, collision avoidance matters
>> hugely.  I couldn't reproduce bean counting woes, no idea if collision
>> avoidance will help that.
>
> So I implement two methods, one is from Rik's random offset proposal

If we should just add random offset to the cpu in the nohz_full mode?

> through skew tick, the other one is from Frederic's proposal and it is
> the same as my original idea through use nanosecond granularity to
> check deltas but only perform an actual cputime update when that delta
>>= TICK_NSEC. Both methods can solve the bug which Luiz reported.

This can just solves two cpu hogs running on the cpu in nohz_full
mode. However, Luiz's testcase w/ ./acct-bug 1 995 shows idle 100%.

Regards,
Wanpeng Li

> Peterz, Thomas, any ideas?
>
> --------------------------->8-------------------------------------------------------------
>
> skew tick:
>
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 7fe53be..9981437 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -1198,7 +1198,11 @@ void tick_setup_sched_timer(void)
>      hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());
>
>      /* Offset the tick to avert jiffies_lock contention. */
> +#ifdef CONFIG_NO_HZ_FULL
> +    if (sched_skew_tick || tick_nohz_full_running) {
> +#else
>      if (sched_skew_tick) {
> +#endif
>          u64 offset = ktime_to_ns(tick_period) >> 1;
>          do_div(offset, num_possible_cpus());
>          offset *= smp_processor_id();
>
> -------------------------------------->8-----------------------------------------------------
>
> use nanosecond granularity to check deltas but only perform an actual
> cputime update when that delta >= TICK_NSEC.
>
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index f3778e2b..f1ee393 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -676,18 +676,21 @@ void thread_group_cputime_adjusted(struct
> task_struct *p, u64 *ut, u64 *st)
>  #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
>  static u64 vtime_delta(struct task_struct *tsk)
>  {
> -    unsigned long now = READ_ONCE(jiffies);
> +    u64 now = local_clock();
> +    u64 delta;
> +
> +    delta = now - tsk->vtime_snap;
>
> -    if (time_before(now, (unsigned long)tsk->vtime_snap))
> +    if (delta < TICK_NSEC)
>          return 0;
>
> -    return jiffies_to_nsecs(now - tsk->vtime_snap);
> +    return jiffies_to_nsecs(delta / TICK_NSEC);
>  }
>
>  static u64 get_vtime_delta(struct task_struct *tsk)
>  {
> -    unsigned long now = READ_ONCE(jiffies);
> -    u64 delta, other;
> +    u64 delta = vtime_delta(tsk);
> +    u64 other;
>
>      /*
>       * Unlike tick based timing, vtime based timing never has lost
> @@ -696,10 +699,9 @@ static u64 get_vtime_delta(struct task_struct *tsk)
>       * elapsed time. Limit account_other_time to prevent rounding
>       * errors from causing elapsed vtime to go negative.
>       */
> -    delta = jiffies_to_nsecs(now - tsk->vtime_snap);
>      other = account_other_time(delta);
>      WARN_ON_ONCE(tsk->vtime_snap_whence == VTIME_INACTIVE);
> -    tsk->vtime_snap = now;
> +    tsk->vtime_snap += delta;
>
>      return delta - other;
>  }
> @@ -776,7 +778,7 @@ void arch_vtime_task_switch(struct task_struct *prev)
>
>      write_seqcount_begin(&current->vtime_seqcount);
>      current->vtime_snap_whence = VTIME_SYS;
> -    current->vtime_snap = jiffies;
> +    current->vtime_snap = sched_clock_cpu(smp_processor_id());
>      write_seqcount_end(&current->vtime_seqcount);
>  }
>
> @@ -787,7 +789,7 @@ void vtime_init_idle(struct task_struct *t, int cpu)
>      local_irq_save(flags);
>      write_seqcount_begin(&t->vtime_seqcount);
>      t->vtime_snap_whence = VTIME_SYS;
> -    t->vtime_snap = jiffies;
> +    t->vtime_snap = sched_clock_cpu(cpu);
>      write_seqcount_end(&t->vtime_seqcount);
>      local_irq_restore(flags);
>  }
>
> Regards,
> Wanpeng Li

[toc] | [prev] | [next] | [standalone]

#1613027

On Thu, 2017-03-30 at 19:52 +0800, Wanpeng Li wrote:

> If we should just add random offset to the cpu in the nohz_full mode?

Up to you, whatever works best.  I left the regular skew alone, just
added some noise to scheduler_tick_max_deferment().

	-Mike

[toc] | [prev] | [next] | [standalone]

#1613065

On Thu, Mar 30, 2017 at 02:47:11PM +0800, Wanpeng Li wrote:
> Cc Peterz, Thomas,
> 2017-03-30 12:27 GMT+08:00 Mike Galbraith <efault@gmx.de>:
> > On Wed, 2017-03-29 at 16:08 -0400, Rik van Riel wrote:
> >
> >> In other words, the tick on cpu0 is aligned
> >> with the tick on the nohz_full cpus, and
> >> jiffies is advanced while the nohz_full cpus
> >> with an active tick happen to be in kernel
> >> mode?
> >
> > You really want skew_tick=1, especially on big boxen.
> >
> >> Frederic, can you think of any reason why
> >> the tick on nohz_full CPUs would end up aligned
> >> with the tick on cpu0, instead of running at some
> >> random offset?
> >
> > (I or low rq->clock bits as crude NOHZ collision avoidance)
> >
> >> A random offset, or better yet a somewhat randomized
> >> tick length to make sure that simultaneous ticks are
> >> fairly rare and the vtime sampling does not end up
> >> "in phase" with the jiffies incrementing, could make
> >> the accounting work right again.
> >
> > That improves jitter, especially on big boxen.  I have an 8 socket box
> > that thinks it's an extra large PC, there, collision avoidance matters
> > hugely.  I couldn't reproduce bean counting woes, no idea if collision
> > avoidance will help that.
> 
> So I implement two methods, one is from Rik's random offset proposal
> through skew tick, the other one is from Frederic's proposal and it is
> the same as my original idea through use nanosecond granularity to
> check deltas but only perform an actual cputime update when that delta
> >= TICK_NSEC. Both methods can solve the bug which Luiz reported.
> Peterz, Thomas, any ideas?
> 
> --------------------------->8-------------------------------------------------------------
> 
> skew tick:
> 
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 7fe53be..9981437 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -1198,7 +1198,11 @@ void tick_setup_sched_timer(void)
>      hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());
> 
>      /* Offset the tick to avert jiffies_lock contention. */
> +#ifdef CONFIG_NO_HZ_FULL
> +    if (sched_skew_tick || tick_nohz_full_running) {
> +#else
>      if (sched_skew_tick) {
> +#endif

Please rather use tick_nohz_full_enabled() to avoid ifdeffery.

>          u64 offset = ktime_to_ns(tick_period) >> 1;
>          do_div(offset, num_possible_cpus());
>          offset *= smp_processor_id();

If it works, we may want to take that solution, likely less performance sensitive
than using sched_clock(). In fact sched_clock() is fast, especially as we require it to
be stable for nohz_full, but using it involves costly conversion back and forth to jiffies.

> 
> -------------------------------------->8-----------------------------------------------------
> 
> use nanosecond granularity to check deltas but only perform an actual
> cputime update when that delta >= TICK_NSEC.
> 
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index f3778e2b..f1ee393 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -676,18 +676,21 @@ void thread_group_cputime_adjusted(struct
> task_struct *p, u64 *ut, u64 *st)
>  #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
>  static u64 vtime_delta(struct task_struct *tsk)
>  {
> -    unsigned long now = READ_ONCE(jiffies);
> +    u64 now = local_clock();

I fear we need a global clock, because the reader (task_cputime()) needs
to compute the delta and therefore use the same clock from any CPU.

Or we can use the local_clock() but the reader must access the same.

So there would be vtime_delta_writer() which uses local_clock and stores
the current CPU to tsk->vtime_cpu (under the vtime_seqcount). And then
vtime_delta_reader() which calls sched_clock_cpu(tsk->vtime_cpu) which
is protected by vtime_seqcount as well.

Although those sched_clock_cpu() things seem to only matter when the
sched_clock() is unstable. And that stability is a condition for nohz_full
to work anyway. So probably sched_clock() alone would be enough.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1613106

2017-03-30 21:38 GMT+08:00 Frederic Weisbecker <fweisbec@gmail.com>:
> On Thu, Mar 30, 2017 at 02:47:11PM +0800, Wanpeng Li wrote:
>> Cc Peterz, Thomas,
>> 2017-03-30 12:27 GMT+08:00 Mike Galbraith <efault@gmx.de>:
>> > On Wed, 2017-03-29 at 16:08 -0400, Rik van Riel wrote:
>> >
>> >> In other words, the tick on cpu0 is aligned
>> >> with the tick on the nohz_full cpus, and
>> >> jiffies is advanced while the nohz_full cpus
>> >> with an active tick happen to be in kernel
>> >> mode?
>> >
>> > You really want skew_tick=1, especially on big boxen.
>> >
>> >> Frederic, can you think of any reason why
>> >> the tick on nohz_full CPUs would end up aligned
>> >> with the tick on cpu0, instead of running at some
>> >> random offset?
>> >
>> > (I or low rq->clock bits as crude NOHZ collision avoidance)
>> >
>> >> A random offset, or better yet a somewhat randomized
>> >> tick length to make sure that simultaneous ticks are
>> >> fairly rare and the vtime sampling does not end up
>> >> "in phase" with the jiffies incrementing, could make
>> >> the accounting work right again.
>> >
>> > That improves jitter, especially on big boxen.  I have an 8 socket box
>> > that thinks it's an extra large PC, there, collision avoidance matters
>> > hugely.  I couldn't reproduce bean counting woes, no idea if collision
>> > avoidance will help that.
>>
>> So I implement two methods, one is from Rik's random offset proposal
>> through skew tick, the other one is from Frederic's proposal and it is
>> the same as my original idea through use nanosecond granularity to
>> check deltas but only perform an actual cputime update when that delta
>> >= TICK_NSEC. Both methods can solve the bug which Luiz reported.
>> Peterz, Thomas, any ideas?
>>
>> --------------------------->8-------------------------------------------------------------
>>
>> skew tick:
>>
>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>> index 7fe53be..9981437 100644
>> --- a/kernel/time/tick-sched.c
>> +++ b/kernel/time/tick-sched.c
>> @@ -1198,7 +1198,11 @@ void tick_setup_sched_timer(void)
>>      hrtimer_set_expires(&ts->sched_timer, tick_init_jiffy_update());
>>
>>      /* Offset the tick to avert jiffies_lock contention. */
>> +#ifdef CONFIG_NO_HZ_FULL
>> +    if (sched_skew_tick || tick_nohz_full_running) {
>> +#else
>>      if (sched_skew_tick) {
>> +#endif
>
> Please rather use tick_nohz_full_enabled() to avoid ifdeffery.
>
>>          u64 offset = ktime_to_ns(tick_period) >> 1;
>>          do_div(offset, num_possible_cpus());
>>          offset *= smp_processor_id();
>
> If it works, we may want to take that solution, likely less performance sensitive
> than using sched_clock(). In fact sched_clock() is fast, especially as we require it to
> be stable for nohz_full, but using it involves costly conversion back and forth to jiffies.

So both Rik and you agree with the skew tick solution, I will try it
tomorrow. Btw, if we should just add random offset to the cpu in the
nohz_full mode or add random offset to all cpus like the codes above?

Regards,
Wanpeng Li

>
>>
>> -------------------------------------->8-----------------------------------------------------
>>
>> use nanosecond granularity to check deltas but only perform an actual
>> cputime update when that delta >= TICK_NSEC.
>>
>> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
>> index f3778e2b..f1ee393 100644
>> --- a/kernel/sched/cputime.c
>> +++ b/kernel/sched/cputime.c
>> @@ -676,18 +676,21 @@ void thread_group_cputime_adjusted(struct
>> task_struct *p, u64 *ut, u64 *st)
>>  #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
>>  static u64 vtime_delta(struct task_struct *tsk)
>>  {
>> -    unsigned long now = READ_ONCE(jiffies);
>> +    u64 now = local_clock();
>
> I fear we need a global clock, because the reader (task_cputime()) needs
> to compute the delta and therefore use the same clock from any CPU.
>
> Or we can use the local_clock() but the reader must access the same.
>
> So there would be vtime_delta_writer() which uses local_clock and stores
> the current CPU to tsk->vtime_cpu (under the vtime_seqcount). And then
> vtime_delta_reader() which calls sched_clock_cpu(tsk->vtime_cpu) which
> is protected by vtime_seqcount as well.
>
> Although those sched_clock_cpu() things seem to only matter when the
> sched_clock() is unstable. And that stability is a condition for nohz_full
> to work anyway. So probably sched_clock() alone would be enough.
>
> Thanks.

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web