| Message ID | 6c3484ccb8220d0c58db1cbd9c929fc8d83005fd.1397492119.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 14, 2014 at 09:47:41PM +0530, Viresh Kumar wrote: > sched_can_stop_tick() was using 7 spaces instead of 8 spaces or a 'tab' at the > beginning of each line. Which doesn't align with the Coding Guidelines. > > Also it removes the *rq variable as it was used at only one place and hence we > can directly use this_rq() instead. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > I don't think rq = tihs_rq() has to be done before smp_mb(), in case yes sorry > for this patch :( > > kernel/sched/core.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 268a45e..13299c5 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -666,18 +666,14 @@ static inline bool got_nohz_idle_kick(void) > #ifdef CONFIG_NO_HZ_FULL > bool sched_can_stop_tick(void) > { > + /* Make sure rq->nr_running update is visible after the IPI */ > + smp_rmb(); > > + /* More than one running task need preemption */ > + if (this_rq()->nr_running > 1) > + return false; > > + return true; > } > #endif /* CONFIG_NO_HZ_FULL */ AFAICT the smp_rmb() is entirely spurious, arch interrupts should ensure consistency on their own. That is: CPU 0 CPU 1 [w] X = 1 IPI 1 <int> [r] r = X Should act as if there was a full memory barrier, making it so that the read on CPU1 observes the write on CPU0. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Mon, Apr 14, 2014 at 08:38:38PM +0200, Peter Zijlstra wrote: > On Mon, Apr 14, 2014 at 09:47:41PM +0530, Viresh Kumar wrote: > > sched_can_stop_tick() was using 7 spaces instead of 8 spaces or a 'tab' at the > > beginning of each line. Which doesn't align with the Coding Guidelines. > > > > Also it removes the *rq variable as it was used at only one place and hence we > > can directly use this_rq() instead. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > I don't think rq = tihs_rq() has to be done before smp_mb(), in case yes sorry > > for this patch :( > > > > kernel/sched/core.c | 16 ++++++---------- > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index 268a45e..13299c5 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -666,18 +666,14 @@ static inline bool got_nohz_idle_kick(void) > > #ifdef CONFIG_NO_HZ_FULL > > bool sched_can_stop_tick(void) > > { > > + /* Make sure rq->nr_running update is visible after the IPI */ > > + smp_rmb(); > > > > + /* More than one running task need preemption */ > > + if (this_rq()->nr_running > 1) > > + return false; > > > > + return true; > > } > > #endif /* CONFIG_NO_HZ_FULL */ > > AFAICT the smp_rmb() is entirely spurious, arch interrupts should ensure > consistency on their own. That is: > > CPU 0 CPU 1 > > [w] X = 1 > IPI 1 <int> > [r] r = X > > Should act as if there was a full memory barrier, making it so that the > read on CPU1 observes the write on CPU0. Right, I have a pending patch for that: https://git.kernel.org/cgit/linux/kernel/git/frederic/linux-dynticks.git/commit/?h=nohz/ipi&id=ca981d9f87fe0f113ad972098cfe181180b3675a -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 15 April 2014 04:18, Frederic Weisbecker <fweisbec@gmail.com> wrote: > Right, I have a pending patch for that: > https://git.kernel.org/cgit/linux/kernel/git/frederic/linux-dynticks.git/commit/?h=nohz/ipi&id=ca981d9f87fe0f113ad972098cfe181180b3675a Cool!! I will rebase my patch over yours and resend. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 268a45e..13299c5 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -666,18 +666,14 @@ static inline bool got_nohz_idle_kick(void) #ifdef CONFIG_NO_HZ_FULL bool sched_can_stop_tick(void) { - struct rq *rq; - - rq = this_rq(); - - /* Make sure rq->nr_running update is visible after the IPI */ - smp_rmb(); + /* Make sure rq->nr_running update is visible after the IPI */ + smp_rmb(); - /* More than one running task need preemption */ - if (rq->nr_running > 1) - return false; + /* More than one running task need preemption */ + if (this_rq()->nr_running > 1) + return false; - return true; + return true; } #endif /* CONFIG_NO_HZ_FULL */
sched_can_stop_tick() was using 7 spaces instead of 8 spaces or a 'tab' at the beginning of each line. Which doesn't align with the Coding Guidelines. Also it removes the *rq variable as it was used at only one place and hence we can directly use this_rq() instead. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- I don't think rq = tihs_rq() has to be done before smp_mb(), in case yes sorry for this patch :( kernel/sched/core.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)