* dynticks: Remove atomic ops for dynticks_idle
@ 2014-09-04 15:18 Christoph Lameter
2014-09-04 16:18 ` Paul E. McKenney
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Lameter @ 2014-09-04 15:18 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Frederic Weisbecker, linux-kernel
This is on top of the first patch (but still not against your tree). I
wonder what you think about this one. No special operations.
Since dynticks_idle is only ever modified by the local cpu we do
not need to use atomics there.
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/kernel/rcu/tree.c
===================================================================
--- linux.orig/kernel/rcu/tree.c
+++ linux/kernel/rcu/tree.c
@@ -213,7 +213,7 @@ static DEFINE_PER_CPU(struct rcu_dyntick
.dynticks = ATOMIC_INIT(1),
#ifdef CONFIG_NO_HZ_FULL_SYSIDLE
.dynticks_idle_nesting = DYNTICK_TASK_NEST_VALUE,
- .dynticks_idle = ATOMIC_INIT(1),
+ .dynticks_idle = 1,
#endif /* #ifdef CONFIG_NO_HZ_FULL_SYSIDLE */
};
Index: linux/kernel/rcu/tree.h
===================================================================
--- linux.orig/kernel/rcu/tree.h
+++ linux/kernel/rcu/tree.h
@@ -91,7 +91,7 @@ struct rcu_dynticks {
#ifdef CONFIG_NO_HZ_FULL_SYSIDLE
long long dynticks_idle_nesting;
/* irq/process nesting level from idle. */
- atomic_t dynticks_idle; /* Even value for idle, else odd. */
+ long dynticks_idle; /* Even value for idle, else odd. */
/* "Idle" excludes userspace execution. */
unsigned long dynticks_idle_jiffies;
/* End of last non-NMI non-idle period. */
Index: linux/kernel/rcu/tree_plugin.h
===================================================================
--- linux.orig/kernel/rcu/tree_plugin.h
+++ linux/kernel/rcu/tree_plugin.h
@@ -2644,9 +2644,9 @@ static void rcu_sysidle_enter(int irq)
j = jiffies;
ACCESS_ONCE(rdtp->dynticks_idle_jiffies) = j;
smp_mb__before_atomic();
- atomic_inc(&rdtp->dynticks_idle);
+ rdtp->dynticks_idle++;
smp_mb__after_atomic();
- WARN_ON_ONCE(atomic_read(&rdtp->dynticks_idle) & 0x1);
+ WARN_ON_ONCE(rdtp->dynticks_idle & 0x1);
}
/*
@@ -2712,9 +2712,9 @@ static void rcu_sysidle_exit(int irq)
/* Record end of idle period. */
smp_mb__before_atomic();
- atomic_inc(&rdtp->dynticks_idle);
+ rdtp->dynticks_idle++;
smp_mb__after_atomic();
- WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks_idle) & 0x1));
+ WARN_ON_ONCE(!(rdtp->dynticks_idle & 0x1));
/*
* If we are the timekeeping CPU, we are permitted to be non-idle
@@ -2755,7 +2755,7 @@ static void rcu_sysidle_check_cpu(struct
WARN_ON_ONCE(smp_processor_id() != tick_do_timer_cpu);
/* Pick up current idle and NMI-nesting counter and check. */
- cur = atomic_read(&rdtp->dynticks_idle);
+ cur = rdtp->dynticks_idle;
if (cur & 0x1) {
*isidle = false; /* We are not idle! */
return;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: dynticks: Remove atomic ops for dynticks_idle
2014-09-04 15:18 dynticks: Remove atomic ops for dynticks_idle Christoph Lameter
@ 2014-09-04 16:18 ` Paul E. McKenney
2014-09-04 18:21 ` Christoph Lameter
0 siblings, 1 reply; 3+ messages in thread
From: Paul E. McKenney @ 2014-09-04 16:18 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Frederic Weisbecker, linux-kernel
On Thu, Sep 04, 2014 at 10:18:31AM -0500, Christoph Lameter wrote:
> This is on top of the first patch (but still not against your tree). I
> wonder what you think about this one. No special operations.
>
>
> Since dynticks_idle is only ever modified by the local cpu we do
> not need to use atomics there.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>
Sorry, as discussed earlier, this change is non-trivial. In addition,
you have not justified the change.
> Index: linux/kernel/rcu/tree.c
> ===================================================================
> --- linux.orig/kernel/rcu/tree.c
> +++ linux/kernel/rcu/tree.c
> @@ -213,7 +213,7 @@ static DEFINE_PER_CPU(struct rcu_dyntick
> .dynticks = ATOMIC_INIT(1),
> #ifdef CONFIG_NO_HZ_FULL_SYSIDLE
> .dynticks_idle_nesting = DYNTICK_TASK_NEST_VALUE,
> - .dynticks_idle = ATOMIC_INIT(1),
> + .dynticks_idle = 1,
> #endif /* #ifdef CONFIG_NO_HZ_FULL_SYSIDLE */
> };
>
> Index: linux/kernel/rcu/tree.h
> ===================================================================
> --- linux.orig/kernel/rcu/tree.h
> +++ linux/kernel/rcu/tree.h
> @@ -91,7 +91,7 @@ struct rcu_dynticks {
> #ifdef CONFIG_NO_HZ_FULL_SYSIDLE
> long long dynticks_idle_nesting;
> /* irq/process nesting level from idle. */
> - atomic_t dynticks_idle; /* Even value for idle, else odd. */
> + long dynticks_idle; /* Even value for idle, else odd. */
> /* "Idle" excludes userspace execution. */
> unsigned long dynticks_idle_jiffies;
> /* End of last non-NMI non-idle period. */
> Index: linux/kernel/rcu/tree_plugin.h
> ===================================================================
> --- linux.orig/kernel/rcu/tree_plugin.h
> +++ linux/kernel/rcu/tree_plugin.h
> @@ -2644,9 +2644,9 @@ static void rcu_sysidle_enter(int irq)
> j = jiffies;
> ACCESS_ONCE(rdtp->dynticks_idle_jiffies) = j;
> smp_mb__before_atomic();
> - atomic_inc(&rdtp->dynticks_idle);
> + rdtp->dynticks_idle++;
> smp_mb__after_atomic();
And this is completely broken.
> - WARN_ON_ONCE(atomic_read(&rdtp->dynticks_idle) & 0x1);
> + WARN_ON_ONCE(rdtp->dynticks_idle & 0x1);
> }
>
> /*
> @@ -2712,9 +2712,9 @@ static void rcu_sysidle_exit(int irq)
>
> /* Record end of idle period. */
> smp_mb__before_atomic();
> - atomic_inc(&rdtp->dynticks_idle);
> + rdtp->dynticks_idle++;
> smp_mb__after_atomic();
As is this.
> - WARN_ON_ONCE(!(atomic_read(&rdtp->dynticks_idle) & 0x1));
> + WARN_ON_ONCE(!(rdtp->dynticks_idle & 0x1));
>
> /*
> * If we are the timekeeping CPU, we are permitted to be non-idle
> @@ -2755,7 +2755,7 @@ static void rcu_sysidle_check_cpu(struct
> WARN_ON_ONCE(smp_processor_id() != tick_do_timer_cpu);
>
> /* Pick up current idle and NMI-nesting counter and check. */
> - cur = atomic_read(&rdtp->dynticks_idle);
> + cur = rdtp->dynticks_idle;
And this as well.
Thanx, Paul
> if (cur & 0x1) {
> *isidle = false; /* We are not idle! */
> return;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: dynticks: Remove atomic ops for dynticks_idle
2014-09-04 16:18 ` Paul E. McKenney
@ 2014-09-04 18:21 ` Christoph Lameter
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Lameter @ 2014-09-04 18:21 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Frederic Weisbecker, linux-kernel
On Thu, 4 Sep 2014, Paul E. McKenney wrote:
> On Thu, Sep 04, 2014 at 10:18:31AM -0500, Christoph Lameter wrote:
> > This is on top of the first patch (but still not against your tree). I
> > wonder what you think about this one. No special operations.
> >
> >
> > Since dynticks_idle is only ever modified by the local cpu we do
> > not need to use atomics there.
> >
> > Signed-off-by: Christoph Lameter <cl@linux.com>
>
> Sorry, as discussed earlier, this change is non-trivial. In addition,
> you have not justified the change.
The rationale is quite clear: Use of atomics without the need to use them.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-06 1:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-04 15:18 dynticks: Remove atomic ops for dynticks_idle Christoph Lameter
2014-09-04 16:18 ` Paul E. McKenney
2014-09-04 18:21 ` Christoph Lameter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox