[firefly] [PATCH 2/2] kernel: rcu: Fix unprotected use of jiffies_stall in tree.c

[Valentina Manea]

On Tue, Mar 11, 2014 at 11:03 AM, Iulia Manda <iulia.manda21 at gmail.com>wrote:

> Protect use of jiffies_stall in tree.c and also some warnings from
> checkpatch.pl.
>
> Signed-off-by: Iulia Manda <iulia.manda21 at gmail.com>
> ---
>  kernel/rcu/tree.c |   24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index b3d116c..2090a94 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -195,7 +195,8 @@ void rcu_sched_qs(int cpu)
>         struct rcu_data *rdp = &per_cpu(rcu_sched_data, cpu);
>
>         if (rdp->passed_quiesce == 0)
> -               trace_rcu_grace_period(TPS("rcu_sched"), rdp->gpnum,
> TPS("cpuqs"));
> +               trace_rcu_grace_period(TPS("rcu_sched"),
> +                       rdp->gpnum, TPS("cpuqs"));
>         rdp->passed_quiesce = 1;
>  }
>
> @@ -377,7 +378,8 @@ static void rcu_eqs_enter_common(struct rcu_dynticks
> *rdtp, long long oldval,
>                 struct task_struct *idle __maybe_unused =
>                         idle_task(smp_processor_id());
>
> -               trace_rcu_dyntick(TPS("Error on entry: not idle task"),
> oldval, 0);
> +               trace_rcu_dyntick(TPS("Error on entry: not idle task"),
> +                       oldval, 0);
>                 ftrace_dump(DUMP_ORIG);
>                 WARN_ONCE(1, "Current pid: %d comm: %s / Idle pid: %d
> comm: %s",
>                           current->pid, current->comm,
> @@ -767,7 +769,6 @@ static int dyntick_save_progress_counter(struct
> rcu_data *rdp,
>   * This function really isn't for public consumption, but RCU is special
> in
>   * that context switches can allow the state machine to make progress.
>   */
> -extern void resched_cpu(int cpu);
>
>  /*
>   * Return true if the specified CPU has passed through a quiescent
> @@ -853,7 +854,7 @@ static void record_gp_stall_check_time(struct
> rcu_state *rsp)
>         rsp->gp_start = j;
>         smp_wmb(); /* Record start time before stall time. */
>         j1 = rcu_jiffies_till_stall_check();
> -       rsp->jiffies_stall = j + j1;
> +       ACCESS_ONCE(rsp->jiffies_stall) = j + j1;
>         rsp->jiffies_resched = j + j1 / 2;
>  }
>
> @@ -892,12 +893,13 @@ static void print_other_cpu_stall(struct rcu_state
> *rsp)
>         /* Only let one CPU complain about others per time interval. */
>
>         raw_spin_lock_irqsave(&rnp->lock, flags);
> -       delta = jiffies - rsp->jiffies_stall;
> +       delta = jiffies - ACCESS_ONCE(rsp->jiffies_stall);
>         if (delta < RCU_STALL_RAT_DELAY || !rcu_gp_in_progress(rsp)) {
>                 raw_spin_unlock_irqrestore(&rnp->lock, flags);
>                 return;
>         }
> -       rsp->jiffies_stall = jiffies + 3 * rcu_jiffies_till_stall_check()
> + 3;
> +       rsp->jiffies_stall = jiffies +
> +               3 * rcu_jiffies_till_stall_check() + 3;
>         raw_spin_unlock_irqrestore(&rnp->lock, flags);
>
>         /*
> @@ -953,7 +955,6 @@ static void print_other_cpu_stall(struct rcu_state
> *rsp)
>   * This function really isn't for public consumption, but RCU is special
> in
>   * that context switches can allow the state machine to make progress.
>   */
> -extern void resched_cpu(int cpu);
>
>  static void print_cpu_stall(struct rcu_state *rsp)
>  {
> @@ -979,8 +980,8 @@ static void print_cpu_stall(struct rcu_state *rsp)
>                 dump_stack();
>
>         raw_spin_lock_irqsave(&rnp->lock, flags);
> -       if (ULONG_CMP_GE(jiffies, rsp->jiffies_stall))
> -               rsp->jiffies_stall = jiffies +
> +       if (ULONG_CMP_GE(jiffies, ACCESS_ONCE(rsp->jiffies_stall)))
> +               ACCESS_ONCE(rsp->jiffies_stall) = jiffies +
>                                      3 * rcu_jiffies_till_stall_check() +
> 3;
>         raw_spin_unlock_irqrestore(&rnp->lock, flags);
>
> @@ -1064,7 +1065,7 @@ void rcu_cpu_stall_reset(void)
>         struct rcu_state *rsp;
>
>         for_each_rcu_flavor(rsp)
> -               rsp->jiffies_stall = jiffies + ULONG_MAX / 2;
> +               ACCESS_ONCE(rsp->jiffies_stall) = jiffies + ULONG_MAX / 2;
>  }
>
>  /*
> @@ -1293,7 +1294,8 @@ static void rcu_accelerate_cbs(struct rcu_state
> *rsp, struct rcu_node *rnp,
>         if (!*rdp->nxttail[RCU_WAIT_TAIL])
>                 trace_rcu_grace_period(rsp->name, rdp->gpnum,
> TPS("AccWaitCB"));
>         else
> -               trace_rcu_grace_period(rsp->name, rdp->gpnum,
> TPS("AccReadyCB"));
> +               trace_rcu_grace_period(rsp->name, rdp->gpnum,
> +                       TPS("AccReadyCB"));
>  }
>
>  /*
>
>
Hi,

A patch should focus on one thing only. Thus, you can't add jiffies_stall
protection and fix checkpatch.pl in the same patch. Please split them
accordingly.

As a side note, when working on checkpatch.pl, it's recommended that you
either:
a) fix one type of error - only trailing whitespaces or split lines over 80
characters
b) fix everything in the given file :)

Same goes for sparse errors such as functions that should be declared as
static.

Thanks,
Valentina
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rosedu.org/pipermail/firefly/attachments/20140311/9cb1b018/attachment.html>