[PATCH RFC tip/core/rcu 2/2] rcu: Fix broken strings in RCU's source code.

[Paul E. McKenney]

From: "Paul E. McKenney" <pau...@linux.vnet.ibm.com>

Although the C language allows you to break strings across lines, doing
this makes it hard for people to find the Linux kernel code corresponding
to a given console message. This commit therefore fixes broken strings
throughout RCU's source code.

Suggested-by: Josh Triplett <jo...@joshtriplett.org>
Suggested-by: Ingo Molnar <mi...@kernel.org>
Signed-off-by: Paul E. McKenney <pau...@linux.vnet.ibm.com>
---
include/linux/rcupdate.h | 3 +--
kernel/rcutorture.c | 33 ++++++++++++++-------------------
kernel/rcutree_trace.c | 25 ++++++++++++-------------
lib/list_debug.c | 6 ++----
4 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 26d1a47..9fae8db 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -432,8 +432,7 @@ extern int rcu_my_thread_group_empty(void);
static inline void rcu_preempt_sleep_check(void)
{
rcu_lockdep_assert(!lock_is_held(&rcu_lock_map),
- "Illegal context switch in RCU read-side "
- "critical section");
+ "Illegal context switch in RCU read-side critical section");
}
#else /* #ifdef CONFIG_PROVE_RCU */
static inline void rcu_preempt_sleep_check(void)
diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index d7c3cb1..ac6612c 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -49,8 +49,7 @@
#include <asm/byteorder.h>

MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Paul E. McKenney <pau...@us.ibm.com> and "
- "Josh Triplett <jo...@freedesktop.org>");
+MODULE_AUTHOR("Paul E. McKenney <pau...@us.ibm.com> and Josh Triplett <jo...@freedesktop.org>");

static int nreaders = -1; /* # reader threads, defaults to 2*ncpus */
static int nfakewriters = 4; /* # fake writer threads */
@@ -1184,27 +1183,27 @@ rcu_torture_printk(char *page)
}
cnt += sprintf(&page[cnt], "%s%s ", torture_type, TORTURE_FLAG);
cnt += sprintf(&page[cnt],
- "rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d "
- "rtmbe: %d rtbke: %ld rtbre: %ld "
- "rtbf: %ld rtb: %ld nt: %ld "
- "onoff: %ld/%ld:%ld/%ld "
- "barrier: %ld/%ld:%ld",
+ "rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ",
rcu_torture_current,
rcu_torture_current_version,
list_empty(&rcu_torture_freelist),
atomic_read(&n_rcu_torture_alloc),
atomic_read(&n_rcu_torture_alloc_fail),
- atomic_read(&n_rcu_torture_free),
+ atomic_read(&n_rcu_torture_free));
+ cnt += sprintf(&page[cnt], "rtmbe: %d rtbke: %ld rtbre: %ld ",
atomic_read(&n_rcu_torture_mberror),
n_rcu_torture_boost_ktrerror,
- n_rcu_torture_boost_rterror,
+ n_rcu_torture_boost_rterror);
+ cnt += sprintf(&page[cnt], "rtbf: %ld rtb: %ld nt: %ld ",
n_rcu_torture_boost_failure,
n_rcu_torture_boosts,
- n_rcu_torture_timers,
+ n_rcu_torture_timers);
+ cnt += sprintf(&page[cnt], "onoff: %ld/%ld:%ld/%ld ",
n_online_successes,
n_online_attempts,
n_offline_successes,
- n_offline_attempts,
+ n_offline_attempts);
+ cnt += sprintf(&page[cnt], "barrier: %ld/%ld:%ld",
n_barrier_successes,
n_barrier_attempts,
n_rcu_torture_barrier_error);
@@ -1446,8 +1445,7 @@ rcu_torture_shutdown(void *arg)
delta = shutdown_time - jiffies_snap;
if (verbose)
printk(KERN_ALERT "%s" TORTURE_FLAG
- "rcu_torture_shutdown task: %lu "
- "jiffies remaining\n",
+ "rcu_torture_shutdown task: %lu jiffies remaining\n",
torture_type, delta);
schedule_timeout_interruptible(delta);
jiffies_snap = ACCESS_ONCE(jiffies);
@@ -1499,8 +1497,7 @@ rcu_torture_onoff(void *arg)
if (cpu_down(cpu) == 0) {
if (verbose)
printk(KERN_ALERT "%s" TORTURE_FLAG
- "rcu_torture_onoff task: "
- "offlined %d\n",
+ "rcu_torture_onoff task: offlined %d\n",
torture_type, cpu);
n_offline_successes++;
}
@@ -1513,8 +1510,7 @@ rcu_torture_onoff(void *arg)
if (cpu_up(cpu) == 0) {
if (verbose)
printk(KERN_ALERT "%s" TORTURE_FLAG
- "rcu_torture_onoff task: "
- "onlined %d\n",
+ "rcu_torture_onoff task: onlined %d\n",
torture_type, cpu);
n_online_successes++;
}
@@ -1932,8 +1928,7 @@ rcu_torture_init(void)
return -EINVAL;
}
if (cur_ops->fqs == NULL && fqs_duration != 0) {
- printk(KERN_ALERT "rcu-torture: ->fqs NULL and non-zero "
- "fqs_duration, fqs disabled.\n");
+ printk(KERN_ALERT "rcu-torture: ->fqs NULL and non-zero fqs_duration, fqs disabled.\n");
fqs_duration = 0;
}
if (cur_ops->init)
diff --git a/kernel/rcutree_trace.c b/kernel/rcutree_trace.c
index d4bc16d..ffcfd6d 100644
--- a/kernel/rcutree_trace.c
+++ b/kernel/rcutree_trace.c
@@ -201,8 +201,7 @@ static const struct file_operations rcudata_csv_fops = {

static void print_one_rcu_node_boost(struct seq_file *m, struct rcu_node *rnp)
{
- seq_printf(m, "%d:%d tasks=%c%c%c%c kt=%c ntb=%lu neb=%lu nnb=%lu "
- "j=%04x bt=%04x\n",
+ seq_printf(m, "%d:%d tasks=%c%c%c%c kt=%c ntb=%lu neb=%lu nnb=%lu ",
rnp->grplo, rnp->grphi,
"T."[list_empty(&rnp->blkd_tasks)],
"N."[!rnp->gp_tasks],
@@ -210,11 +209,11 @@ static void print_one_rcu_node_boost(struct seq_file *m, struct rcu_node *rnp)
"B."[!rnp->boost_tasks],
convert_kthread_status(rnp->boost_kthread_status),
rnp->n_tasks_boosted, rnp->n_exp_boosts,
- rnp->n_normal_boosts,
+ rnp->n_normal_boosts);
+ seq_printf(m, "j=%04x bt=%04x\n",
(int)(jiffies & 0xffff),
(int)(rnp->boost_time & 0xffff));
- seq_printf(m, "%s: nt=%lu egt=%lu bt=%lu nb=%lu ny=%lu nos=%lu\n",
- " balk",
+ seq_printf(m, " balk: nt=%lu egt=%lu bt=%lu nb=%lu ny=%lu nos=%lu\n",
rnp->n_balk_blkd_tasks,
rnp->n_balk_exp_gp_tasks,
rnp->n_balk_boost_tasks,
@@ -270,11 +269,11 @@ static void print_one_rcu_state(struct seq_file *m, struct rcu_state *rsp)
struct rcu_node *rnp;

gpnum = rsp->gpnum;
- seq_printf(m, "c=%lu g=%lu s=%d jfq=%ld j=%x "
- "nfqs=%lu/nfqsng=%lu(%lu) fqlh=%lu oqlen=%ld/%ld\n",
+ seq_printf(m, "c=%lu g=%lu s=%d jfq=%ld j=%x ",
rsp->completed, gpnum, rsp->fqs_state,
(long)(rsp->jiffies_force_qs - jiffies),
- (int)(jiffies & 0xffff),
+ (int)(jiffies & 0xffff));
+ seq_printf(m, "nfqs=%lu/nfqsng=%lu(%lu) fqlh=%lu oqlen=%ld/%ld\n",
rsp->n_force_qs, rsp->n_force_qs_ngp,
rsp->n_force_qs - rsp->n_force_qs_ngp,
rsp->n_force_qs_lh, rsp->qlen_lazy, rsp->qlen);
@@ -366,16 +365,16 @@ static const struct file_operations rcugp_fops = {

static void print_one_rcu_pending(struct seq_file *m, struct rcu_data *rdp)
{
- seq_printf(m, "%3d%cnp=%ld "
- "qsp=%ld rpq=%ld cbr=%ld cng=%ld "
- "gpc=%ld gps=%ld nf=%ld nn=%ld\n",
+ seq_printf(m, "%3d%cnp=%ld ",
rdp->cpu,
cpu_is_offline(rdp->cpu) ? '!' : ' ',
- rdp->n_rcu_pending,
+ rdp->n_rcu_pending);
+ seq_printf(m, "qsp=%ld rpq=%ld cbr=%ld cng=%ld ",
rdp->n_rp_qs_pending,
rdp->n_rp_report_qs,
rdp->n_rp_cb_ready,
- rdp->n_rp_cpu_needs_gp,
+ rdp->n_rp_cpu_needs_gp);
+ seq_printf(m, "gpc=%ld gps=%ld nf=%ld nn=%ld\n",
rdp->n_rp_gp_completed,
rdp->n_rp_gp_started,
rdp->n_rp_need_fqs,
diff --git a/lib/list_debug.c b/lib/list_debug.c
index 3810b48..542d408 100644
--- a/lib/list_debug.c
+++ b/lib/list_debug.c
@@ -84,12 +84,10 @@ void __list_add_rcu(struct list_head *new,
struct list_head *prev, struct list_head *next)
{
WARN(next->prev != prev,
- "list_add_rcu corruption. next->prev should be "
- "prev (%p), but was %p. (next=%p).\n",
+ "list_add_rcu corruption. next->prev should be prev (%p), but was %p. (next=%p).\n",
prev, next->prev, next);
WARN(prev->next != next,
- "list_add_rcu corruption. prev->next should be "
- "next (%p), but was %p. (prev=%p).\n",
+ "list_add_rcu corruption. prev->next should be next (%p), but was %p. (prev=%p).\n",
next, prev->next, prev);
new->next = next;
new->prev = prev;
--
1.7.8

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Paul E. McKenney]

From: "Paul E. McKenney" <paul.m...@linaro.org>

The Linux kernel coding style says that single-statement blocks should
omit curly braces unless the other leg of the "if" statement has
multiple statements, in which case the curly braces should be included.
This commit fixes RCU's violations of this rule.

Signed-off-by: Paul E. McKenney <paul.m...@linaro.org>

Signed-off-by: Paul E. McKenney <pau...@linux.vnet.ibm.com>
---

kernel/rcutiny_plugin.h | 3 ++-
kernel/rcutorture.c | 3 ++-
kernel/rcutree.c | 7 ++++---
kernel/rcutree_plugin.h | 18 ++++++++++--------
4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
index fc31a2d..cd9ae33 100644
--- a/kernel/rcutiny_plugin.h
+++ b/kernel/rcutiny_plugin.h
@@ -351,8 +351,9 @@ static int rcu_initiate_boost(void)
rcu_preempt_ctrlblk.boost_tasks =
rcu_preempt_ctrlblk.gp_tasks;
invoke_rcu_callbacks();
- } else
+ } else {
RCU_TRACE(rcu_initiate_boost_trace());
+ }
return 1;
}

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index e66b34a..d7c3cb1 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -407,8 +407,9 @@ rcu_torture_cb(struct rcu_head *p)
if (++rp->rtort_pipe_count >= RCU_TORTURE_PIPE_LEN) {
rp->rtort_mbtest = 0;
rcu_torture_free(rp);
- } else
+ } else {
cur_ops->deferred_free(rp);
+ }
}

static int rcu_no_completed(void)
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 0da7b88..25874a3 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -893,8 +893,9 @@ static void __note_new_gpnum(struct rcu_state *rsp, struct rcu_node *rnp, struct
if (rnp->qsmask & rdp->grpmask) {
rdp->qs_pending = 1;
rdp->passed_quiesce = 0;
- } else
+ } else {
rdp->qs_pending = 0;
+ }
zero_cpu_stall_ticks(rdp);
}
}
@@ -2115,9 +2116,9 @@ void synchronize_sched_expedited(void)
put_online_cpus();

/* No joy, try again later. Or just synchronize_sched(). */
- if (trycount++ < 10)
+ if (trycount++ < 10) {
udelay(trycount * num_online_cpus());
- else {
+ } else {
synchronize_sched();
return;
}
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 2411000..c9b173c 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -398,8 +398,9 @@ static noinline void rcu_read_unlock_special(struct task_struct *t)
rnp->grphi,
!!rnp->gp_tasks);
rcu_report_unblock_qs_rnp(rnp, flags);
- } else
+ } else {
raw_spin_unlock_irqrestore(&rnp->lock, flags);
+ }

#ifdef CONFIG_RCU_BOOST
/* Unboost if we were boosted. */
@@ -429,9 +430,9 @@ void __rcu_read_unlock(void)
{
struct task_struct *t = current;

- if (t->rcu_read_lock_nesting != 1)
+ if (t->rcu_read_lock_nesting != 1) {
--t->rcu_read_lock_nesting;
- else {
+ } else {
barrier(); /* critical section before exit code. */
t->rcu_read_lock_nesting = INT_MIN;
barrier(); /* assign before ->rcu_read_unlock_special load */
@@ -824,9 +825,9 @@ sync_rcu_preempt_exp_init(struct rcu_state *rsp, struct rcu_node *rnp)
int must_wait = 0;

raw_spin_lock_irqsave(&rnp->lock, flags);
- if (list_empty(&rnp->blkd_tasks))
+ if (list_empty(&rnp->blkd_tasks)) {
raw_spin_unlock_irqrestore(&rnp->lock, flags);
- else {
+ } else {
rnp->exp_tasks = rnp->blkd_tasks.next;
rcu_initiate_boost(rnp, flags); /* releases rnp->lock */
must_wait = 1;
@@ -870,9 +871,9 @@ void synchronize_rcu_expedited(void)
* expedited grace period for us, just leave.
*/
while (!mutex_trylock(&sync_rcu_preempt_exp_mutex)) {
- if (trycount++ < 10)
+ if (trycount++ < 10) {
udelay(trycount * num_online_cpus());
- else {
+ } else {
synchronize_rcu();
return;
}
@@ -2213,8 +2214,9 @@ static void rcu_prepare_for_idle(int cpu)
if (rcu_cpu_has_callbacks(cpu)) {
trace_rcu_prep_idle("More callbacks");
invoke_rcu_core();
- } else
+ } else {
trace_rcu_prep_idle("Callbacks drained");
+ }
}

/*

[Josh Triplett]

A change like this placates tools like checkpatch, but doesn't actually
fix the problem. :)

- Josh Triplett

[Josh Triplett]

On Thu, May 17, 2012 at 03:12:44PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paul.m...@linaro.org>
>
> The Linux kernel coding style says that single-statement blocks should
> omit curly braces unless the other leg of the "if" statement has
> multiple statements, in which case the curly braces should be included.
> This commit fixes RCU's violations of this rule.
>
> Signed-off-by: Paul E. McKenney <paul.m...@linaro.org>
> Signed-off-by: Paul E. McKenney <pau...@linux.vnet.ibm.com>

Reviewed-by: Josh Triplett <jo...@joshtriplett.org>

[Paul E. McKenney]

The formatting would frustrate most people trying to grep for the
debugfs output anyway, though.

But the main reason for the change is that it makes it easier to see
which quantity goes with which sprintf() format specifier. (Lazy and
self-indulgent, I know!!!)

Thanx, Paul

[Paul E. McKenney]

On Fri, May 18, 2012 at 12:27:31AM +0200, Peter Zijlstra wrote:

> On Thu, 2012-05-17 at 15:12 -0700, Paul E. McKenney wrote:
> > cnt += sprintf(&page[cnt],
> > - "rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d "
> > - "rtmbe: %d rtbke: %ld rtbre: %ld "
> > - "rtbf: %ld rtb: %ld nt: %ld "
> > - "onoff: %ld/%ld:%ld/%ld "
> > - "barrier: %ld/%ld:%ld",
>

> The paranoid side of me would've used:
>
> cnt += snprintf(page + cnt, PAGE_SIZE - cnt, ...);

OK, added to the todo list. ;-)

Thanx, Paul

[Ingo Molnar]

* Peter Zijlstra <pet...@infradead.org> wrote:

> On Thu, 2012-05-17 at 15:12 -0700, Paul E. McKenney wrote:

> > From: "Paul E. McKenney" <pau...@linux.vnet.ibm.com>
> >
> > Although the C language allows you to break strings across lines, doing
> > this makes it hard for people to find the Linux kernel code corresponding
> > to a given console message. This commit therefore fixes broken strings
> > throughout RCU's source code.
>

> While I'm not a strict 80 chars zealot, I do find exceedingly
> long lines utterly annoying, editors either wrap them in the
> most hideous way or you don't see them.

Well, but if it's exceedingly long in the editor then it's
probably exceedingly long in the syslog as well and need fixing,
right?

Or it's printed inside some exceedingly deep nested block of
code, which too could use improvement, right?

Thanks,

Ingo

[Lai Jiangshan]

On 05/18/2012 06:12 AM, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paul.m...@linaro.org>
>
> The Linux kernel coding style says that single-statement blocks should
> omit curly braces unless the other leg of the "if" statement has
> multiple statements, in which case the curly braces should be included.
> This commit fixes RCU's violations of this rule.
>
> Signed-off-by: Paul E. McKenney <paul.m...@linaro.org>
> Signed-off-by: Paul E. McKenney <pau...@linux.vnet.ibm.com>
> ---
> kernel/rcutiny_plugin.h | 3 ++-
> kernel/rcutorture.c | 3 ++-
> kernel/rcutree.c | 7 ++++---
> kernel/rcutree_plugin.h | 18 ++++++++++--------
> 4 files changed, 18 insertions(+), 13 deletions(-)
>

Acked-by: Lai Jiangshan <la...@cn.fujitsu.com>

[Paul E. McKenney]

On Fri, May 18, 2012 at 08:33:08AM +0200, Ingo Molnar wrote:
>
> * Peter Zijlstra <pet...@infradead.org> wrote:
>
> > On Thu, 2012-05-17 at 15:12 -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <pau...@linux.vnet.ibm.com>
> > >
> > > Although the C language allows you to break strings across lines, doing
> > > this makes it hard for people to find the Linux kernel code corresponding
> > > to a given console message. This commit therefore fixes broken strings
> > > throughout RCU's source code.
> >
> > While I'm not a strict 80 chars zealot, I do find exceedingly
> > long lines utterly annoying, editors either wrap them in the
> > most hideous way or you don't see them.
>
> Well, but if it's exceedingly long in the editor then it's
> probably exceedingly long in the syslog as well and need fixing,
> right?
>
> Or it's printed inside some exceedingly deep nested block of
> code, which too could use improvement, right?

The longest source-code line produced by this patch is 104 characters.

I am hoping that this is not considered excessively long!

Thanx, Paul