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


Groups > linux.kernel > #1475174 > unrolled thread

[PATCH 0/5] sched/cputime: irqtime cleanups

Started byFrederic Weisbecker <fweisbec@gmail.com>
First post2016-09-02 16:10 +0200
Last post2016-09-06 19:00 +0200
Articles 12 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/5] sched/cputime: irqtime cleanups Frederic Weisbecker <fweisbec@gmail.com> - 2016-09-02 16:10 +0200
    [PATCH 1/5] irqtime: No need for preempt-safe accessors Frederic Weisbecker <fweisbec@gmail.com> - 2016-09-02 16:10 +0200
      Re: [PATCH 1/5] irqtime: No need for preempt-safe accessors Rik van Riel <riel@redhat.com> - 2016-09-06 19:10 +0200
    [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update Frederic Weisbecker <fweisbec@gmail.com> - 2016-09-02 16:10 +0200
      Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat  update Paolo Bonzini <pbonzini@redhat.com> - 2016-09-02 17:00 +0200
        Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on  kcpustat update Peter Zijlstra <peterz@infradead.org> - 2016-09-02 18:40 +0200
      Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on  kcpustat update Rik van Riel <riel@redhat.com> - 2016-09-06 19:10 +0200
      Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on  kcpustat update Peter Zijlstra <peterz@infradead.org> - 2016-09-07 10:00 +0200
    [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers Frederic Weisbecker <fweisbec@gmail.com> - 2016-09-02 16:10 +0200
      Re: [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers Paolo Bonzini <pbonzini@redhat.com> - 2016-09-02 16:40 +0200
    [PATCH 5/5] irqtime: Consolidate irqtime flushing code Frederic Weisbecker <fweisbec@gmail.com> - 2016-09-02 16:10 +0200
      Re: [PATCH 5/5] irqtime: Consolidate irqtime flushing code Rik van Riel <riel@redhat.com> - 2016-09-06 19:00 +0200

#1475174 — [PATCH 0/5] sched/cputime: irqtime cleanups

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-09-02 16:10 +0200
Subject[PATCH 0/5] sched/cputime: irqtime cleanups
Message-ID<scXkm-s8-19@gated-at.bofh.it>
This series contains a few optimizations against irq disabling, and the
rest is consolidation.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	sched/irqtime

HEAD: 63024a0947091e0a20bafa33ccccb685ae33c275

Thanks,
	Frederic
---

Frederic Weisbecker (5):
      irqtime: No need for preempt-safe accessors
      irqtime: Remove needless IRQs disablement on kcpustat update
      u64_stats: Introduce IRQs disabled helpers
      irqtime: Consolidate accounting synchronization with u64_stats API
      irqtime: Consolidate irqtime flushing code


 include/linux/u64_stats_sync.h | 49 +++++++++++++++++++++-----------------
 kernel/sched/cputime.c         | 51 ++++++++++++++++------------------------
 kernel/sched/sched.h           | 53 ++++++++++++------------------------------
 3 files changed, 63 insertions(+), 90 deletions(-)

[toc] | [next] | [standalone]


#1475176 — [PATCH 1/5] irqtime: No need for preempt-safe accessors

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-09-02 16:10 +0200
Subject[PATCH 1/5] irqtime: No need for preempt-safe accessors
Message-ID<scXkm-s8-21@gated-at.bofh.it>
In reply to#1475174
We can safely use the preempt-unsafe accessors for irqtime when we
flush its counters to kcpustat as IRQs are disabled at this time.

Cc: Rik van Riel <riel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/sched/cputime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index b93c72d..f111076 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -82,7 +82,7 @@ static cputime_t irqtime_account_hi_update(cputime_t maxtime)
 	cputime_t irq_cputime;
 
 	local_irq_save(flags);
-	irq_cputime = nsecs_to_cputime64(this_cpu_read(cpu_hardirq_time)) -
+	irq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_hardirq_time)) -
 		      cpustat[CPUTIME_IRQ];
 	irq_cputime = min(irq_cputime, maxtime);
 	cpustat[CPUTIME_IRQ] += irq_cputime;
@@ -97,7 +97,7 @@ static cputime_t irqtime_account_si_update(cputime_t maxtime)
 	cputime_t softirq_cputime;
 
 	local_irq_save(flags);
-	softirq_cputime = nsecs_to_cputime64(this_cpu_read(cpu_softirq_time)) -
+	softirq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_softirq_time)) -
 			  cpustat[CPUTIME_SOFTIRQ];
 	softirq_cputime = min(softirq_cputime, maxtime);
 	cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
-- 
2.7.0

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


#1477659 — Re: [PATCH 1/5] irqtime: No need for preempt-safe accessors

FromRik van Riel <riel@redhat.com>
Date2016-09-06 19:10 +0200
SubjectRe: [PATCH 1/5] irqtime: No need for preempt-safe accessors
Message-ID<ses2K-4Ps-33@gated-at.bofh.it>
In reply to#1475176

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2016-09-02 at 16:03 +0200, Frederic Weisbecker wrote:
> We can safely use the preempt-unsafe accessors for irqtime when we
> flush its counters to kcpustat as IRQs are disabled at this time.
> 
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
Reviewed-by: Rik van Riel <riel@redhat.com>

-- 

All Rights Reversed.

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


#1475177 — [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-09-02 16:10 +0200
Subject[PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update
Message-ID<scXkm-s8-29@gated-at.bofh.it>
In reply to#1475174
The callers of the functions performing irqtime kcpustat updates have
IRQS disabled, no need to disable them again.

Cc: Rik van Riel <riel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/sched/cputime.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index f111076..d4d12a9 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -78,30 +78,26 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
 static cputime_t irqtime_account_hi_update(cputime_t maxtime)
 {
 	u64 *cpustat = kcpustat_this_cpu->cpustat;
-	unsigned long flags;
 	cputime_t irq_cputime;
 
-	local_irq_save(flags);
 	irq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_hardirq_time)) -
 		      cpustat[CPUTIME_IRQ];
 	irq_cputime = min(irq_cputime, maxtime);
 	cpustat[CPUTIME_IRQ] += irq_cputime;
-	local_irq_restore(flags);
+
 	return irq_cputime;
 }
 
 static cputime_t irqtime_account_si_update(cputime_t maxtime)
 {
 	u64 *cpustat = kcpustat_this_cpu->cpustat;
-	unsigned long flags;
 	cputime_t softirq_cputime;
 
-	local_irq_save(flags);
 	softirq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_softirq_time)) -
 			  cpustat[CPUTIME_SOFTIRQ];
 	softirq_cputime = min(softirq_cputime, maxtime);
 	cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
-	local_irq_restore(flags);
+
 	return softirq_cputime;
 }
 
-- 
2.7.0

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


#1475222 — Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-09-02 17:00 +0200
SubjectRe: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update
Message-ID<scY6K-KW-23@gated-at.bofh.it>
In reply to#1475177

On 02/09/2016 16:03, Frederic Weisbecker wrote:
> The callers of the functions performing irqtime kcpustat updates have
> IRQS disabled, no need to disable them again.

They do, but perhaps this should be annotated through some sparse magic.
 It's starting to be hairy, with the requirement spanning many separate
files.

Something like

#define __irq_disabled __must_hold(IRQ)

together with __acquire and __release annotations in
include/linux/irqflags.h would do.  I'm not sure how to handle
local_irq_save/local_irq_restore, but I guess sparse would be fine with

((void)({
   raw_local_irq_save(flags);
   if (flags) __acquire(IRQ);
}))

and

((void)({
   if (flags) __release(IRQ);
   raw_local_irq_restore(flags);
}))

since below that it's assembly.

Starting from irqtime_account_hi_update, irqtime_account_si_update and
irqtime_account_irq you'd get quite a few functions annotated.

Paolo

> Cc: Rik van Riel <riel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  kernel/sched/cputime.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index f111076..d4d12a9 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -78,30 +78,26 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
>  static cputime_t irqtime_account_hi_update(cputime_t maxtime)
>  {
>  	u64 *cpustat = kcpustat_this_cpu->cpustat;
> -	unsigned long flags;
>  	cputime_t irq_cputime;
>  
> -	local_irq_save(flags);
>  	irq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_hardirq_time)) -
>  		      cpustat[CPUTIME_IRQ];
>  	irq_cputime = min(irq_cputime, maxtime);
>  	cpustat[CPUTIME_IRQ] += irq_cputime;
> -	local_irq_restore(flags);
> +
>  	return irq_cputime;
>  }
>  
>  static cputime_t irqtime_account_si_update(cputime_t maxtime)
>  {
>  	u64 *cpustat = kcpustat_this_cpu->cpustat;
> -	unsigned long flags;
>  	cputime_t softirq_cputime;
>  
> -	local_irq_save(flags);
>  	softirq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_softirq_time)) -
>  			  cpustat[CPUTIME_SOFTIRQ];
>  	softirq_cputime = min(softirq_cputime, maxtime);
>  	cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
> -	local_irq_restore(flags);
> +
>  	return softirq_cputime;
>  }
>  
> 

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


#1475339 — Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-02 18:40 +0200
SubjectRe: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update
Message-ID<scZFw-1Qn-21@gated-at.bofh.it>
In reply to#1475222
On Fri, Sep 02, 2016 at 04:53:47PM +0200, Paolo Bonzini wrote:
> 
> 
> On 02/09/2016 16:03, Frederic Weisbecker wrote:
> > The callers of the functions performing irqtime kcpustat updates have
> > IRQS disabled, no need to disable them again.
> 
> They do, but perhaps this should be annotated through some sparse magic.
>  It's starting to be hairy, with the requirement spanning many separate
> files.

Sparse sucks for those things...

maybe just add something like lockdep_assert_irqsoff(). Such a call both
documents the requirement and validates at runtime when CONFIG_LOCKDEP.

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


#1477662 — Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

FromRik van Riel <riel@redhat.com>
Date2016-09-06 19:10 +0200
SubjectRe: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update
Message-ID<ses2K-4Ps-35@gated-at.bofh.it>
In reply to#1475177

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2016-09-02 at 16:03 +0200, Frederic Weisbecker wrote:
> The callers of the functions performing irqtime kcpustat updates have
> IRQS disabled, no need to disable them again.
> 
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 

All Rights Reversed.

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


#1478041 — Re: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update

FromPeter Zijlstra <peterz@infradead.org>
Date2016-09-07 10:00 +0200
SubjectRe: [PATCH 2/5] irqtime: Remove needless IRQs disablement on kcpustat update
Message-ID<seFW2-5g7-19@gated-at.bofh.it>
In reply to#1475177
On Fri, Sep 02, 2016 at 04:03:02PM +0200, Frederic Weisbecker wrote:

> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index f111076..d4d12a9 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -78,30 +78,26 @@ EXPORT_SYMBOL_GPL(irqtime_account_irq);
>  static cputime_t irqtime_account_hi_update(cputime_t maxtime)
>  {
>  	u64 *cpustat = kcpustat_this_cpu->cpustat;
> -	unsigned long flags;
>  	cputime_t irq_cputime;
>  
+	lockdep_assert_irqs_disabled();

> -	local_irq_save(flags);
>  	irq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_hardirq_time)) -
>  		      cpustat[CPUTIME_IRQ];
>  	irq_cputime = min(irq_cputime, maxtime);
>  	cpustat[CPUTIME_IRQ] += irq_cputime;
> -	local_irq_restore(flags);
> +
>  	return irq_cputime;
>  }
>  
>  static cputime_t irqtime_account_si_update(cputime_t maxtime)
>  {
>  	u64 *cpustat = kcpustat_this_cpu->cpustat;
> -	unsigned long flags;
>  	cputime_t softirq_cputime;
>  
+	lockdep_assert_irqs_disabled();

> -	local_irq_save(flags);
>  	softirq_cputime = nsecs_to_cputime64(__this_cpu_read(cpu_softirq_time)) -
>  			  cpustat[CPUTIME_SOFTIRQ];
>  	softirq_cputime = min(softirq_cputime, maxtime);
>  	cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
> -	local_irq_restore(flags);
> +
>  	return softirq_cputime;
>  }

---
Subject: locking/lockdep: Provide IRQ state assertion helpers

Provide a cheap alternative to:

	WARN_ON(!irqs_disabled());

This patch provides:

	lockdep_assert_irqs_disabled();
	lockdep_assert_softirqs_disabled();

Which compile away for CONFIG_LOCKDEP=n kernels.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -526,6 +526,23 @@ static inline void print_irqtrace_events
 }
 #endif
 
+#if defined(CONFIG_LOCKDEP) && defined(CONFIG_TRACE_IRQFLAGS)
+
+#define lockdep_assert_irqs_disabled() do {				\
+		WARN_ON(debug_locks && current->hardirqs_enabled);	\
+	} while (0)
+
+#define lockdep_assert_softirqs_disabled() do {				\
+		WARN_ON(debug_locks && current->softirqs_enabled);	\
+	} while (0)
+
+#else
+
+#define lockdep_assert_irqs_disabled()		do { } while (0)
+#define lockdep_assert_softirqs_disabled()	do { } while (0)
+
+#endif
+
 /*
  * For trivial one-depth nesting of a lock-class, the following
  * global define can be used. (Subsystems with multiple levels

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


#1475179 — [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-09-02 16:10 +0200
Subject[PATCH 3/5] u64_stats: Introduce IRQs disabled helpers
Message-ID<scXkm-s8-31@gated-at.bofh.it>
In reply to#1475174
Introduce light versions of u64_stats helpers for context where
either preempt or IRQs are disabled. This way we can make this library
usable by scheduler irqtime accounting which currenty implement its
ad-hoc version.

Cc: Rik van Riel <riel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/u64_stats_sync.h | 55 ++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
index d3a2bb7..9101d23 100644
--- a/include/linux/u64_stats_sync.h
+++ b/include/linux/u64_stats_sync.h
@@ -103,28 +103,41 @@ static inline void u64_stats_update_end_raw(struct u64_stats_sync *syncp)
 #endif
 }
 
+static inline unsigned int __u64_stats_fetch_begin(const struct u64_stats_sync *syncp)
+{
+#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+	return read_seqcount_begin(&syncp->seq);
+#else
+	return 0;
+#endif
+}
+
 static inline unsigned int u64_stats_fetch_begin(const struct u64_stats_sync *syncp)
 {
-#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
-	return read_seqcount_begin(&syncp->seq);
-#else
-#if BITS_PER_LONG==32
+#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
 	preempt_disable();
+#else
+	return __u64_stats_fetch_begin(syncp);
 #endif
-	return 0;
+}
+
+static inline bool __u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
+					 unsigned int start)
+{
+#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+	return read_seqcount_retry(&syncp->seq, start);
+#else
+	return false;
 #endif
 }
 
 static inline bool u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
 					 unsigned int start)
 {
-#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
-	return read_seqcount_retry(&syncp->seq, start);
-#else
-#if BITS_PER_LONG==32
+#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
 	preempt_enable();
-#endif
-	return false;
+#else
+	return __u64_stats_fetch_retry(syncp, start);
 #endif
 }
 
@@ -136,26 +149,20 @@ static inline bool u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
  */
 static inline unsigned int u64_stats_fetch_begin_irq(const struct u64_stats_sync *syncp)
 {
-#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
-	return read_seqcount_begin(&syncp->seq);
-#else
-#if BITS_PER_LONG==32
+#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
 	local_irq_disable();
-#endif
-	return 0;
+#else
+	return __u64_stats_fetch_begin(syncp);
 #endif
 }
 
 static inline bool u64_stats_fetch_retry_irq(const struct u64_stats_sync *syncp,
-					 unsigned int start)
+					     unsigned int start)
 {
-#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
-	return read_seqcount_retry(&syncp->seq, start);
-#else
-#if BITS_PER_LONG==32
+#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
 	local_irq_enable();
-#endif
-	return false;
+#else
+	return __u64_stats_fetch_retry(syncp, start);
 #endif
 }
 
-- 
2.7.0

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


#1475199 — Re: [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers

FromPaolo Bonzini <pbonzini@redhat.com>
Date2016-09-02 16:40 +0200
SubjectRe: [PATCH 3/5] u64_stats: Introduce IRQs disabled helpers
Message-ID<scXNo-El-11@gated-at.bofh.it>
In reply to#1475179

On 02/09/2016 16:03, Frederic Weisbecker wrote:
>  static inline unsigned int u64_stats_fetch_begin(const struct u64_stats_sync *syncp)
>  {
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> -	return read_seqcount_begin(&syncp->seq);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>  	preempt_disable();
> +#else

This should be #endif, or this side ends without a "return" statement.

> +	return __u64_stats_fetch_begin(syncp);
>  #endif
> -	return 0;
> +}

...

> 
>  static inline bool u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
>  					 unsigned int start)
>  {
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> -	return read_seqcount_retry(&syncp->seq, start);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>  	preempt_enable();
> -#endif
> -	return false;
> +#else

Same here.

> +	return __u64_stats_fetch_retry(syncp, start);
>  #endif
>  }


...

> 
> -	return read_seqcount_begin(&syncp->seq);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>  	local_irq_disable();
> -#endif
> -	return 0;
> +#else

Same here.

> +	return __u64_stats_fetch_begin(syncp);
>  #endif


> 
> -#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> -	return read_seqcount_retry(&syncp->seq, start);
> -#else
> -#if BITS_PER_LONG==32
> +#if BITS_PER_LONG==32 && !defined(CONFIG_SMP)
>  	local_irq_enable();
> -#endif
> -	return false;
> +#else

Same here.

> +	return __u64_stats_fetch_retry(syncp, start);
>  #endif


Thanks,

Paolo

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


#1475182 — [PATCH 5/5] irqtime: Consolidate irqtime flushing code

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2016-09-02 16:10 +0200
Subject[PATCH 5/5] irqtime: Consolidate irqtime flushing code
Message-ID<scXkm-s8-37@gated-at.bofh.it>
In reply to#1475174
The code performing irqtime nsecs stats flushing to kcpustat is roughly
the same for hardirq and softirq. So lets consolidate that common code.

Cc: Rik van Riel <riel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/sched/cputime.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index f51c9a9..f8de5d3 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -70,32 +70,28 @@ void irqtime_account_irq(struct task_struct *curr)
 }
 EXPORT_SYMBOL_GPL(irqtime_account_irq);
 
-static cputime_t irqtime_account_hi_update(cputime_t maxtime)
+static cputime_t irqtime_account_update(u64 irqtime, int idx, cputime_t maxtime)
 {
 	u64 *cpustat = kcpustat_this_cpu->cpustat;
 	cputime_t irq_cputime;
-	u64 nsecs;
 
-	nsecs = __this_cpu_read(cpu_irqtime.hardirq_time);
-	irq_cputime = nsecs_to_cputime64(nsecs) - cpustat[CPUTIME_IRQ];
+	irq_cputime = nsecs_to_cputime64(irqtime) - cpustat[idx];
 	irq_cputime = min(irq_cputime, maxtime);
-	cpustat[CPUTIME_IRQ] += irq_cputime;
+	cpustat[idx] += irq_cputime;
 
 	return irq_cputime;
 }
 
+static cputime_t irqtime_account_hi_update(cputime_t maxtime)
+{
+	return irqtime_account_update(__this_cpu_read(cpu_irqtime.hardirq_time),
+				      CPUTIME_IRQ, maxtime);
+}
+
 static cputime_t irqtime_account_si_update(cputime_t maxtime)
 {
-	u64 *cpustat = kcpustat_this_cpu->cpustat;
-	cputime_t softirq_cputime;
-	u64 nsecs;
-
-	nsecs = __this_cpu_read(cpu_irqtime.softirq_time);
-	softirq_cputime = nsecs_to_cputime64(nsecs) - cpustat[CPUTIME_SOFTIRQ];
-	softirq_cputime = min(softirq_cputime, maxtime);
-	cpustat[CPUTIME_SOFTIRQ] += softirq_cputime;
-
-	return softirq_cputime;
+	return irqtime_account_update(__this_cpu_read(cpu_irqtime.softirq_time),
+				      CPUTIME_SOFTIRQ, maxtime);
 }
 
 #else /* CONFIG_IRQ_TIME_ACCOUNTING */
-- 
2.7.0

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


#1477638 — Re: [PATCH 5/5] irqtime: Consolidate irqtime flushing code

FromRik van Riel <riel@redhat.com>
Date2016-09-06 19:00 +0200
SubjectRe: [PATCH 5/5] irqtime: Consolidate irqtime flushing code
Message-ID<serT3-4wT-9@gated-at.bofh.it>
In reply to#1475182

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2016-09-02 at 16:03 +0200, Frederic Weisbecker wrote:
> The code performing irqtime nsecs stats flushing to kcpustat is
> roughly
> the same for hardirq and softirq. So lets consolidate that common
> code.
> 
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Wanpeng Li <wanpeng.li@hotmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
Reviewed-by: Rik van Riel <riel@redhat.com>

-- 

All Rights Reversed.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web