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


Groups > linux.kernel > #1569382 > unrolled thread

[GIT PULL] cputime: Convert core use of cputime_t to nsecs

Started byFrederic Weisbecker <fweisbec@gmail.com>
First post2017-01-30 05:50 +0100
Last post2017-01-30 17:00 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [GIT PULL] cputime: Convert core use of cputime_t to nsecs Frederic Weisbecker <fweisbec@gmail.com> - 2017-01-30 05:50 +0100
    Re: [GIT PULL] cputime: Convert core use of cputime_t to nsecs Stanislaw Gruszka <sgruszka@redhat.com> - 2017-01-30 15:40 +0100
      Re: [GIT PULL] cputime: Convert core use of cputime_t to nsecs Frederic Weisbecker <fweisbec@gmail.com> - 2017-01-30 16:10 +0100
        Re: [GIT PULL] cputime: Convert core use of cputime_t to nsecs Stanislaw Gruszka <sgruszka@redhat.com> - 2017-01-30 17:00 +0100

#1569382 — [GIT PULL] cputime: Convert core use of cputime_t to nsecs

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2017-01-30 05:50 +0100
Subject[GIT PULL] cputime: Convert core use of cputime_t to nsecs
Message-ID<t5c4F-7WV-7@gated-at.bofh.it>
Ingo,

Please pull the cputime/nsecs-for-tip branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	cputime/nsecs-for-tip

HEAD: 14d889bef71ff808c450f82bcd257b10f05bb061

The patches are the same than on my previous post: https://lwn.net/Articles/712213/
If you apply them by email, just please ignore the very last one
([PATCH 37/37] s390: Prevent from cputime leaks) because we need to find
a better solution with Martin. The branch doesn't have this patch.


--- Summary ---

cputime_t is a type that can map to different time units and
granularities: jiffies, nsecs or architecture clock.

This type and its accessors and mutators have been designed to deal
with all these time units that can vary depending on the kernel config
in order to support a model where the cputime is stored as-is under
the source unit.

The pro here with this model is to avoid expensive conversions from the
source unit cputime to a more generic type during the accounting hotpath.
Especially for config that have CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y.

Now there are several cons:

   * we need to maintain a whole set of cputime_t mutators and accessors
     for all implementations of cputime_t (currently 4 of them). And we
     need such function for every kind of time conversion: to/from
     jiffies, nsecs, usecs, timeval, clock_t, ...

   * The core code needs to deal with different possible granularities
     of cputime_t while converting to/from another time unit.
     Especially functions like nsecs_to_cputime() can leak some nsecs
     remainder. This adds more complexity and even sometimes performance
     loss (involving reverse conversion) in order to avoid losing such
     time remainder (eg: irqtime accounting, steal time accounting, posix
     cpu timers, ...).

   * Kernel developers are seldom familiar with these granularity issues:
     cputime leaks often appear in patches dealing with cputime code.

   * In general cputime_t, as a varying type, is more opaque and harder
     to deal with than static nsecs. Making the core code less readable.

This patchset removes all core use of cputime_t and stores the cputime
into nsecs units. Only s390 and powerpc
(with CONFIG_VIRT_CPU_ACCOUNTING_NATIVE for the latter) now still use
cputime_t.

Many code get simplified, the diffstat is appealing and some fastpath
should even have a small performance gain (irqtime and steal time
accounting).

Now lets admit one drawback: s390 and powerpc with
CONFIG_VIRT_CPU_ACCOUNTING_NATIVE have new cputime_t to nsecs conversion
on cputime accounting path. But this should be leveraged by the recent
changes which delay the cputime accounting to tick and context switch. 

Thanks,
	Frederic
---

Frederic Weisbecker (36):
      jiffies: Reuse TICK_NSEC instead of NSEC_PER_JIFFY
      time: Introduce jiffies64_to_nsecs()
      sched: Remove unused INIT_CPUTIME macro
      cputime: Convert kcpustat to nsecs
      macintosh/rack-meter: Remove cputime_t internal use
      cputime: Convert guest time accounting to nsecs
      cputime: Special API to return old-typed cputime
      cputime: Convert task/group cputime to nsecs
      alpha: Convert obsolete cputime_t to nsecs
      x86: Convert obsolete cputime type to nsecs
      isdn: Convert obsolete cputime type to nsecs
      binfmt: Convert obsolete cputime type to nsecs
      acct: Convert obsolete cputime type to nsecs
      delaycct: Convert obsolete cputime type to nsecs
      tsacct: Convert obsolete cputime type to nsecs
      signal: Convert obsolete cputime type to nsecs
      cputime: Increment kcpustat directly on irqtime account
      posix-timers: Use TICK_NSEC instead of a dynamically ad-hoc calculated version
      posix-timers: Convert internals to use nsecs
      itimer: Convert internal cputime_t units to nsec
      sched: Remove temporary cputime_t accessors
      cputime: Push time to account_user_time() in nsecs
      cputime: Push time to account_steal_time() in nsecs
      cputime: Push time to account_idle_time() in nsecs
      cputime: Push time to account_system_time() in nsecs
      cputime: Complete nsec conversion of tick based accounting
      vtime: Return nsecs instead of cputime_t to account
      cputime: Remove jiffies based cputime
      ia64: Move nsecs based cputime headers to the last arch using it
      ia64: Convert vtime to use nsec units directly
      ia64: Remove unused cputime definitions
      s390: Make arch_cpu_idle_time() to return nsecs
      powerpc: Remove unused cputime definitions
      s390: Remove unused cputime definitions
      cputime: Remove unused nsec_to_cputime
      cputime: Remove asm generic headers


 arch/alpha/include/asm/Kbuild         |   1 -
 arch/alpha/kernel/osf_sys.c           |  10 +-
 arch/arc/include/asm/Kbuild           |   1 -
 arch/arm/include/asm/Kbuild           |   1 -
 arch/arm64/include/asm/Kbuild         |   1 -
 arch/avr32/include/asm/Kbuild         |   1 -
 arch/blackfin/include/asm/Kbuild      |   1 -
 arch/c6x/include/asm/Kbuild           |   1 -
 arch/cris/include/asm/Kbuild          |   1 -
 arch/frv/include/asm/Kbuild           |   1 -
 arch/h8300/include/asm/Kbuild         |   1 -
 arch/hexagon/include/asm/Kbuild       |   1 -
 arch/ia64/include/asm/cputime.h       |   6 +-
 arch/ia64/kernel/head.S               |   4 +-
 arch/ia64/kernel/time.c               |  17 ++--
 arch/m32r/include/asm/Kbuild          |   1 -
 arch/m68k/include/asm/Kbuild          |   1 -
 arch/metag/include/asm/Kbuild         |   1 -
 arch/microblaze/include/asm/Kbuild    |   1 -
 arch/mips/include/asm/Kbuild          |   1 -
 arch/mips/kernel/binfmt_elfn32.c      |  12 +--
 arch/mips/kernel/binfmt_elfo32.c      |  12 +--
 arch/mn10300/include/asm/Kbuild       |   1 -
 arch/nios2/include/asm/Kbuild         |   1 -
 arch/openrisc/include/asm/Kbuild      |   1 -
 arch/parisc/include/asm/Kbuild        |   1 -
 arch/parisc/kernel/binfmt_elf32.c     |  11 +--
 arch/powerpc/include/asm/cputime.h    | 177 +---------------------------------
 arch/powerpc/kernel/time.c            |  45 +++------
 arch/s390/appldata/appldata_os.c      |  16 +--
 arch/s390/include/asm/cputime.h       | 109 +--------------------
 arch/s390/kernel/idle.c               |   9 +-
 arch/s390/kernel/vtime.c              |  16 +--
 arch/score/include/asm/Kbuild         |   1 -
 arch/sh/include/asm/Kbuild            |   1 -
 arch/sparc/include/asm/Kbuild         |   1 -
 arch/tile/include/asm/Kbuild          |   1 -
 arch/um/include/asm/Kbuild            |   1 -
 arch/unicore32/include/asm/Kbuild     |   1 -
 arch/x86/include/asm/Kbuild           |   1 -
 arch/x86/kernel/apm_32.c              |   6 +-
 arch/x86/kvm/hyperv.c                 |   5 +-
 arch/xtensa/include/asm/Kbuild        |   1 -
 drivers/cpufreq/cpufreq.c             |   6 +-
 drivers/cpufreq/cpufreq_governor.c    |   2 +-
 drivers/cpufreq/cpufreq_stats.c       |   1 -
 drivers/isdn/mISDN/stack.c            |   4 +-
 drivers/macintosh/rack-meter.c        |  28 +++---
 fs/binfmt_elf.c                       |  15 +--
 fs/binfmt_elf_fdpic.c                 |  14 +--
 fs/compat_binfmt_elf.c                |  18 +---
 fs/proc/array.c                       |  16 +--
 fs/proc/stat.c                        |  64 ++++++------
 fs/proc/uptime.c                      |   7 +-
 include/asm-generic/cputime.h         |  15 ---
 include/asm-generic/cputime_jiffies.h |  75 --------------
 include/asm-generic/cputime_nsecs.h   | 121 -----------------------
 include/linux/compat.h                |  20 +++-
 include/linux/cputime.h               |   7 +-
 include/linux/jiffies.h               |   2 +
 include/linux/kernel_stat.h           |  13 ++-
 include/linux/posix-timers.h          |  14 +--
 include/linux/sched.h                 |  62 +++++-------
 include/trace/events/timer.h          |  26 ++---
 kernel/acct.c                         |   7 +-
 kernel/delayacct.c                    |   6 +-
 kernel/exit.c                         |   4 +-
 kernel/fork.c                         |   2 +-
 kernel/sched/cpuacct.c                |   2 +-
 kernel/sched/cputime.c                | 166 +++++++++++++------------------
 kernel/sched/sched.h                  |   7 +-
 kernel/sched/stats.h                  |   4 +-
 kernel/signal.c                       |  12 +--
 kernel/sys.c                          |  16 +--
 kernel/time/itimer.c                  |  60 ++++--------
 kernel/time/jiffies.c                 |  32 +++---
 kernel/time/posix-cpu-timers.c        | 170 ++++++++++++--------------------
 kernel/time/time.c                    |  10 ++
 kernel/time/timeconst.bc              |   6 ++
 kernel/tsacct.c                       |  21 ++--
 80 files changed, 436 insertions(+), 1102 deletions(-)

[toc] | [next] | [standalone]


#1569783

FromStanislaw Gruszka <sgruszka@redhat.com>
Date2017-01-30 15:40 +0100
Message-ID<t5lhE-55I-25@gated-at.bofh.it>
In reply to#1569382
On Mon, Jan 30, 2017 at 05:46:43AM +0100, Frederic Weisbecker wrote:
> Now lets admit one drawback: s390 and powerpc with
> CONFIG_VIRT_CPU_ACCOUNTING_NATIVE have new cputime_t to nsecs conversion
> on cputime accounting path. But this should be leveraged by the recent
> changes which delay the cputime accounting to tick and context switch. 

I think it would be worth to mention that there are other drawbacks on
32bit architectures that use cputime-jiffies currently, like:
- cache utilization will be worse
- conversion cputime_to_jiffies() and cputime_to_clock_t() (with HZ == USER_HS)
  will no longer be an no-op
- to keep values consistent will need to add protection of u64 store/load,
  which will create additional performance costs

Stanislaw

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


#1569807

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2017-01-30 16:10 +0100
Message-ID<t5lKG-5uN-25@gated-at.bofh.it>
In reply to#1569783
On Mon, Jan 30, 2017 at 03:32:24PM +0100, Stanislaw Gruszka wrote:
> On Mon, Jan 30, 2017 at 05:46:43AM +0100, Frederic Weisbecker wrote:
> > Now lets admit one drawback: s390 and powerpc with
> > CONFIG_VIRT_CPU_ACCOUNTING_NATIVE have new cputime_t to nsecs conversion
> > on cputime accounting path. But this should be leveraged by the recent
> > changes which delay the cputime accounting to tick and context switch. 
> 
> I think it would be worth to mention that there are other drawbacks on
> 32bit architectures that use cputime-jiffies currently, like:
> - cache utilization will be worse

Due to utime and stime becoming 64 bits? Yeah indeed.

> - conversion cputime_to_jiffies() and cputime_to_clock_t() (with HZ == USER_HS)
>   will no longer be an no-op

Yes probably some cputime_to_jiffies() have been replaced with nsecs_to_jiffies().
In turn many cputime_to_nsecs() are now no-op.

Now concerning cputime_to_clock_t(), it seems that USER_HZ=100 most of the time. This
value seems to be seldom used for HZ on distros in general.

> - to keep values consistent will need to add protection of u64 store/load,
>   which will create additional performance costs

Yes but note these issues were already there before this patchset.
Especially for kcpustat. And utime/stime also have these issues under CONFIG_NO_HZ_FULL.

Now this patchset extends the problem for utime/stime to all configs. That's certainly
one more reason to fix it. But that's outside the scope of this patchset.

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


#1569859

FromStanislaw Gruszka <sgruszka@redhat.com>
Date2017-01-30 17:00 +0100
Message-ID<t5mx4-5LI-19@gated-at.bofh.it>
In reply to#1569807
On Mon, Jan 30, 2017 at 03:56:49PM +0100, Frederic Weisbecker wrote:
> On Mon, Jan 30, 2017 at 03:32:24PM +0100, Stanislaw Gruszka wrote:
> > On Mon, Jan 30, 2017 at 05:46:43AM +0100, Frederic Weisbecker wrote:
> > > Now lets admit one drawback: s390 and powerpc with
> > > CONFIG_VIRT_CPU_ACCOUNTING_NATIVE have new cputime_t to nsecs conversion
> > > on cputime accounting path. But this should be leveraged by the recent
> > > changes which delay the cputime accounting to tick and context switch. 
> > 
> > I think it would be worth to mention that there are other drawbacks on
> > 32bit architectures that use cputime-jiffies currently, like:
> > - cache utilization will be worse
> 
> Due to utime and stime becoming 64 bits? Yeah indeed.

:-) Well, it is possible that something that use to fit into one cache
line, now will be consuming 2 cache lines and results more cache misses.

In general, this patchset seems to be nice cleanup of code, but how it
affect runtime efficiency is not clear and might depend on .config
and architecture. However seems 32bit embedded users will be affected
negatively.

Stanislaw

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web