[PATCH v3 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
Add more details from a recent kernel newbies mailing list discussion here: http://www.spinics.net/lists/newbies/msg52747.html Also list the operations available and add details about safe accesses. v3: clearly mention that remote write access is discouraged v2: updated with comments from Christoph Lameter Signed-off-by: Pranith Kumar CC: Christoph Lameter --- Documentation/this_cpu_ops.txt | 169 ++--- 1 file changed, 141 insertions(+), 28 deletions(-) diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..b7041f5 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -2,33 +2,62 @@ this_cpu operations --- this_cpu operations are a way of optimizing access to per cpu -variables associated with the *currently* executing processor through -the use of segment registers (or a dedicated register where the cpu -permanently stored the beginning of the per cpu area for a specific -processor). +variables associated with the *currently* executing processor. This is +done through the use of segment registers (or a dedicated register where +the cpu permanently stored the beginning of the per cpuarea for a +specific processor). -The this_cpu operations add a per cpu variable offset to the processor +this_cpu operations add a per cpu variable offset to the processor specific percpu base and encode that operation in the instruction operating on the per cpu variable. -This means there are no atomicity issues between the calculation of +This means that there are no atomicity issues between the calculation of the offset and the operation on the data. Therefore it is not -necessary to disable preempt or interrupts to ensure that the +necessary to disable preemption or interrupts to ensure that the processor is not changed between the calculation of the address and the operation on the data. Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate -without the typical synchronization overhead but still provide some -sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +without the typical synchronization overhead, but still provide some +sort of relaxed atomicity guarantees. The x86, for example, can execute +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but synchronization is not necessary since we are dealing with per cpu data specific to the currently executing processor. Only the current processor should be accessing that variable and therefore there are no -concurrency issues with other processors in the system. +concurrency issues with other processors in the system. Please see the +section "Remote access to per-CPU data" if you need remote access. + +The main use of the this_cpu operations has been to optimize counter +operations. + +The following this_cpu() operations with implied preemption protection +are defined. These operations can be used without worrying about +preemption and interrupts. + + this_cpu_add() + this_cpu_read(pcp) + this_cpu_write(pcp, val) + this_cpu_add(pcp, val) + this_cpu_and(pcp, val) + this_cpu_or(pcp, val) + this_cpu_add_return(pcp, val) + this_cpu_xchg(pcp, nval) + this_cpu_cmpxchg(pcp, oval, nval) + this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) + this_cpu_sub(pcp, val) + this_cpu_inc(pcp) + this_cpu_dec(pcp) + this_cpu_sub_return(pcp, val) + this_cpu_inc_return(pcp) + this_cpu_dec_return(pcp) + + +Inner working of this_cpu operations + On x86 the fs: or the gs: segment registers contain the base of the per cpu area. It is then possible to simply use the segment override @@ -53,12 +82,11 @@ this_cpu_ops such sequence also required preempt disable/enable to prevent the kernel from moving the thread to a different processor while the calculation is performed. -The main use of the this_cpu operations has been to optimize counter -operations. +Consider the following this_cpu operation this_cpu_inc(x) -results in the following single instruction (no lock prefix!) +The above results in the following single instruction (no lock prefix!) inc gs:[x] @@ -100,11 +128,10 @@ Takes the offset of a per cpu variable ( !) and returns the address of the per cpu variable that belongs to the currently executing processor. this_cpu_ptr avoids multiple steps that the common get_cpu/put_cpu sequence requires. No processor number is -availabl
[PATCH v3 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
Add more details from a recent kernel newbies mailing list discussion here: http://www.spinics.net/lists/newbies/msg52747.html Also list the operations available and add details about safe accesses. v3: clearly mention that remote write access is discouraged v2: updated with comments from Christoph Lameter Signed-off-by: Pranith Kumar bobby.pr...@gmail.com CC: Christoph Lameter c...@linux.com --- Documentation/this_cpu_ops.txt | 169 ++--- 1 file changed, 141 insertions(+), 28 deletions(-) diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..b7041f5 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -2,33 +2,62 @@ this_cpu operations --- this_cpu operations are a way of optimizing access to per cpu -variables associated with the *currently* executing processor through -the use of segment registers (or a dedicated register where the cpu -permanently stored the beginning of the per cpu area for a specific -processor). +variables associated with the *currently* executing processor. This is +done through the use of segment registers (or a dedicated register where +the cpu permanently stored the beginning of the per cpuarea for a +specific processor). -The this_cpu operations add a per cpu variable offset to the processor +this_cpu operations add a per cpu variable offset to the processor specific percpu base and encode that operation in the instruction operating on the per cpu variable. -This means there are no atomicity issues between the calculation of +This means that there are no atomicity issues between the calculation of the offset and the operation on the data. Therefore it is not -necessary to disable preempt or interrupts to ensure that the +necessary to disable preemption or interrupts to ensure that the processor is not changed between the calculation of the address and the operation on the data. Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate -without the typical synchronization overhead but still provide some -sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +without the typical synchronization overhead, but still provide some +sort of relaxed atomicity guarantees. The x86, for example, can execute +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but synchronization is not necessary since we are dealing with per cpu data specific to the currently executing processor. Only the current processor should be accessing that variable and therefore there are no -concurrency issues with other processors in the system. +concurrency issues with other processors in the system. Please see the +section Remote access to per-CPU data if you need remote access. + +The main use of the this_cpu operations has been to optimize counter +operations. + +The following this_cpu() operations with implied preemption protection +are defined. These operations can be used without worrying about +preemption and interrupts. + + this_cpu_add() + this_cpu_read(pcp) + this_cpu_write(pcp, val) + this_cpu_add(pcp, val) + this_cpu_and(pcp, val) + this_cpu_or(pcp, val) + this_cpu_add_return(pcp, val) + this_cpu_xchg(pcp, nval) + this_cpu_cmpxchg(pcp, oval, nval) + this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) + this_cpu_sub(pcp, val) + this_cpu_inc(pcp) + this_cpu_dec(pcp) + this_cpu_sub_return(pcp, val) + this_cpu_inc_return(pcp) + this_cpu_dec_return(pcp) + + +Inner working of this_cpu operations + On x86 the fs: or the gs: segment registers contain the base of the per cpu area. It is then possible to simply use the segment override @@ -53,12 +82,11 @@ this_cpu_ops such sequence also required preempt disable/enable to prevent the kernel from moving the thread to a different processor while the calculation is performed. -The main use of the this_cpu operations has been to optimize counter -operations. +Consider the following this_cpu operation this_cpu_inc(x) -results in the following single instruction (no lock prefix!) +The above results in the following single instruction (no lock prefix!) inc gs:[x] @@ -100,11 +128,10 @@ Takes the offset of a per cpu variable (x !) and returns the address of the per cpu variable that belongs to the currently executing processor. this_cpu_ptr avoids multiple steps that the common get_cpu/put_cpu sequence requires. No processor
Re: [PATCH 1/1] rcu: Check for have_rcu_nocb_mask instead of rcu_nocb_mask
On 07/19/2014 02:10 AM, Paul E. McKenney wrote:
>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>> index ce8c331..8f987c1 100644
>> --- a/kernel/rcu/tree_plugin.h
>> +++ b/kernel/rcu/tree_plugin.h
>> @@ -2053,9 +2053,10 @@ static void rcu_init_one_nocb(struct rcu_node *rnp)
>> /* Is the specified CPU a no-CBs CPU? */
>> bool rcu_is_nocb_cpu(int cpu)
>> {
>> -if (have_rcu_nocb_mask)
>> -return cpumask_test_cpu(cpu, rcu_nocb_mask);
>> -return false;
>> +if (!have_rcu_nocb_mask)
>> +return false;
>> +
>> +return cpumask_test_cpu(cpu, rcu_nocb_mask);
>
> Hmmm... That was a subtle way to tell me that my feedback was bogus. ;-)
> I guess I need to stop reviewing patches at 2AM local time.
>
> Anyway, you are correct, this was OK to start with, so I am dropping
> this hunk.
>
I thought you wanted me to change the check of the have_rcu_nocb_mask, and I
really think the new version is better. But yes, more of a style change :)
--
Pranith
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] rcu: Check for have_rcu_nocb_mask instead of rcu_nocb_mask
On 07/19/2014 02:10 AM, Paul E. McKenney wrote:
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index ce8c331..8f987c1 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2053,9 +2053,10 @@ static void rcu_init_one_nocb(struct rcu_node *rnp)
/* Is the specified CPU a no-CBs CPU? */
bool rcu_is_nocb_cpu(int cpu)
{
-if (have_rcu_nocb_mask)
-return cpumask_test_cpu(cpu, rcu_nocb_mask);
-return false;
+if (!have_rcu_nocb_mask)
+return false;
+
+return cpumask_test_cpu(cpu, rcu_nocb_mask);
Hmmm... That was a subtle way to tell me that my feedback was bogus. ;-)
I guess I need to stop reviewing patches at 2AM local time.
Anyway, you are correct, this was OK to start with, so I am dropping
this hunk.
I thought you wanted me to change the check of the have_rcu_nocb_mask, and I
really think the new version is better. But yes, more of a style change :)
--
Pranith
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/1] rcu: Consolidate kthread launches
Hi Paul,
I looked at why my previous change removing checks for
rcu_scheduler_fully_active failed to boot.
The reason is that rcu_init() is called earlier than rest_init() from within
which the early_init() is being called. rcu_init() calls rcu_cpu_notify() which
in turn calls rcu_spawn_all_nocb_kthread().
This patch does the following:
* since we are launching other threads now from rcu_spawn_gp_kthread() rename it
to rcu_spawn_kthreads()
* move nocb kthread launches on cpu hotplug to rcu_prepare_kthreads(), the name
is generic so thre is no need to rename ;)
* check whether CPU is a no-CB CPU before calling rcu_spawn_all_nocb_kthreads()
on that CPU
I've got the KVM rcutorture setup running and verified these changes.
Side note: we should probably change the trace comment for rcu_cpu_notify()
which says it is for Hotplug :)
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree.c| 8
kernel/rcu/tree_plugin.h | 19 ---
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 72e0b1f..2866464 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3451,7 +3451,6 @@ static int rcu_cpu_notify(struct notifier_block *self,
case CPU_UP_PREPARE_FROZEN:
rcu_prepare_cpu(cpu);
rcu_prepare_kthreads(cpu);
- rcu_spawn_all_nocb_kthreads(cpu);
break;
case CPU_ONLINE:
case CPU_DOWN_FAILED:
@@ -3499,9 +3498,10 @@ static int rcu_pm_notify(struct notifier_block *self,
}
/*
- * Spawn the kthreads that handle each RCU flavor's grace periods.
+ * Spawn the kthreads that handle each RCU flavor's grace periods
+ * and the no-CB and boost kthreads.
*/
-static int __init rcu_spawn_gp_kthread(void)
+static int __init rcu_spawn_kthreads(void)
{
unsigned long flags;
struct rcu_node *rnp;
@@ -3521,7 +3521,7 @@ static int __init rcu_spawn_gp_kthread(void)
rcu_spawn_boost_kthreads();
return 0;
}
-early_initcall(rcu_spawn_gp_kthread);
+early_initcall(rcu_spawn_kthreads);
/*
* This function is invoked towards the end of the scheduler's initialization
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index eaa32e4..42e113b 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1339,7 +1339,7 @@ static int rcu_spawn_one_boost_kthread(struct rcu_state
*rsp,
if (_preempt_state != rsp)
return 0;
- if (!rcu_scheduler_fully_active || rnp->qsmaskinit == 0)
+ if (rnp->qsmaskinit == 0)
return 0;
rsp->boost = 1;
@@ -1486,9 +1486,14 @@ static void rcu_prepare_kthreads(int cpu)
struct rcu_data *rdp = per_cpu_ptr(rcu_state_p->rda, cpu);
struct rcu_node *rnp = rdp->mynode;
+ if (!rcu_scheduler_fully_active)
+ return;
+
/* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
- if (rcu_scheduler_fully_active)
- (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
+ (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
+
+ if (rcu_is_nocb_cpu(cpu))
+ rcu_spawn_all_nocb_kthreads(cpu);
}
#else /* #ifdef CONFIG_RCU_BOOST */
@@ -2507,9 +2512,8 @@ static void rcu_spawn_all_nocb_kthreads(int cpu)
{
struct rcu_state *rsp;
- if (rcu_scheduler_fully_active)
- for_each_rcu_flavor(rsp)
- rcu_spawn_one_nocb_kthread(rsp, cpu);
+ for_each_rcu_flavor(rsp)
+ rcu_spawn_one_nocb_kthread(rsp, cpu);
}
/*
@@ -2523,7 +2527,8 @@ static void __init rcu_spawn_nocb_kthreads(void)
int cpu;
for_each_online_cpu(cpu)
- rcu_spawn_all_nocb_kthreads(cpu);
+ if (rcu_is_nocb_cpu(cpu))
+ rcu_spawn_all_nocb_kthreads(cpu);
}
/* How many follower CPU IDs per leader? Default of -1 for sqrt(nr_cpu_ids).
*/
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: Fix build failure
Hi Paul,
While running the kvm rcutorture test scripts, I encountered a build failure
caused by
Commit 918179699e4a ("rcu: Don't keep timekeeping CPU tick running for
non-nohz_full= CPUs")
This commit fixes the failure. This is on top of paul/rcu/dev.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 5591276..eaa32e4 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2795,7 +2795,7 @@ static void rcu_sysidle_exit(struct rcu_dynticks *rdtp,
int irq)
* CPU to keep a tick on our behalf. We assume that the timekeeping
* CPU is also a nohz_full= CPU.
*/
- if (!tick_nohz_full_cpu(cpu))
+ if (!tick_nohz_full_cpu(smp_processor_id()))
return;
/* Update system-idle state: We are clearly no longer fully idle! */
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: Fix build failure
Hi Paul, While running the kvm rcutorture test scripts, I encountered a build failure caused by Commit 918179699e4a (rcu: Don't keep timekeeping CPU tick running for non-nohz_full= CPUs) This commit fixes the failure. This is on top of paul/rcu/dev. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- kernel/rcu/tree_plugin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index 5591276..eaa32e4 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -2795,7 +2795,7 @@ static void rcu_sysidle_exit(struct rcu_dynticks *rdtp, int irq) * CPU to keep a tick on our behalf. We assume that the timekeeping * CPU is also a nohz_full= CPU. */ - if (!tick_nohz_full_cpu(cpu)) + if (!tick_nohz_full_cpu(smp_processor_id())) return; /* Update system-idle state: We are clearly no longer fully idle! */ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/1] rcu: Consolidate kthread launches
Hi Paul,
I looked at why my previous change removing checks for
rcu_scheduler_fully_active failed to boot.
The reason is that rcu_init() is called earlier than rest_init() from within
which the early_init() is being called. rcu_init() calls rcu_cpu_notify() which
in turn calls rcu_spawn_all_nocb_kthread().
This patch does the following:
* since we are launching other threads now from rcu_spawn_gp_kthread() rename it
to rcu_spawn_kthreads()
* move nocb kthread launches on cpu hotplug to rcu_prepare_kthreads(), the name
is generic so thre is no need to rename ;)
* check whether CPU is a no-CB CPU before calling rcu_spawn_all_nocb_kthreads()
on that CPU
I've got the KVM rcutorture setup running and verified these changes.
Side note: we should probably change the trace comment for rcu_cpu_notify()
which says it is for Hotplug :)
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree.c| 8
kernel/rcu/tree_plugin.h | 19 ---
2 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 72e0b1f..2866464 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3451,7 +3451,6 @@ static int rcu_cpu_notify(struct notifier_block *self,
case CPU_UP_PREPARE_FROZEN:
rcu_prepare_cpu(cpu);
rcu_prepare_kthreads(cpu);
- rcu_spawn_all_nocb_kthreads(cpu);
break;
case CPU_ONLINE:
case CPU_DOWN_FAILED:
@@ -3499,9 +3498,10 @@ static int rcu_pm_notify(struct notifier_block *self,
}
/*
- * Spawn the kthreads that handle each RCU flavor's grace periods.
+ * Spawn the kthreads that handle each RCU flavor's grace periods
+ * and the no-CB and boost kthreads.
*/
-static int __init rcu_spawn_gp_kthread(void)
+static int __init rcu_spawn_kthreads(void)
{
unsigned long flags;
struct rcu_node *rnp;
@@ -3521,7 +3521,7 @@ static int __init rcu_spawn_gp_kthread(void)
rcu_spawn_boost_kthreads();
return 0;
}
-early_initcall(rcu_spawn_gp_kthread);
+early_initcall(rcu_spawn_kthreads);
/*
* This function is invoked towards the end of the scheduler's initialization
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index eaa32e4..42e113b 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1339,7 +1339,7 @@ static int rcu_spawn_one_boost_kthread(struct rcu_state
*rsp,
if (rcu_preempt_state != rsp)
return 0;
- if (!rcu_scheduler_fully_active || rnp-qsmaskinit == 0)
+ if (rnp-qsmaskinit == 0)
return 0;
rsp-boost = 1;
@@ -1486,9 +1486,14 @@ static void rcu_prepare_kthreads(int cpu)
struct rcu_data *rdp = per_cpu_ptr(rcu_state_p-rda, cpu);
struct rcu_node *rnp = rdp-mynode;
+ if (!rcu_scheduler_fully_active)
+ return;
+
/* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
- if (rcu_scheduler_fully_active)
- (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
+ (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
+
+ if (rcu_is_nocb_cpu(cpu))
+ rcu_spawn_all_nocb_kthreads(cpu);
}
#else /* #ifdef CONFIG_RCU_BOOST */
@@ -2507,9 +2512,8 @@ static void rcu_spawn_all_nocb_kthreads(int cpu)
{
struct rcu_state *rsp;
- if (rcu_scheduler_fully_active)
- for_each_rcu_flavor(rsp)
- rcu_spawn_one_nocb_kthread(rsp, cpu);
+ for_each_rcu_flavor(rsp)
+ rcu_spawn_one_nocb_kthread(rsp, cpu);
}
/*
@@ -2523,7 +2527,8 @@ static void __init rcu_spawn_nocb_kthreads(void)
int cpu;
for_each_online_cpu(cpu)
- rcu_spawn_all_nocb_kthreads(cpu);
+ if (rcu_is_nocb_cpu(cpu))
+ rcu_spawn_all_nocb_kthreads(cpu);
}
/* How many follower CPU IDs per leader? Default of -1 for sqrt(nr_cpu_ids).
*/
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] rcu: Check for have_rcu_nocb_mask instead of rcu_nocb_mask
On 07/17/2014 07:49 PM, Paul E. McKenney wrote:
> On Thu, Jul 17, 2014 at 05:30:11PM -0400, Pranith Kumar wrote:
>> Hi Paul,
>>
>> This is something similar to what you found yesterday with tick_nohz_full
>> mask.
>>
>> --
>> Pranith
>>
>> If we configure a kernel with CONFIG_NOCB_CPU=y, CONFIG_RCU_NOCB_CPU_NONE=y
>> and
>> CONFIG_CPUMASK_OFFSTACK=n and do not pass in a rcu_nocb= boot parameter, the
>> cpumask rcu_nocb_mask can be garbage instead of NULL.
>>
>> Hence this commit replaces checks for rcu_nocb_mask == NULL with a check for
>> have_rcu_nocb_mask.
>>
>> Signed-off-by: Pranith Kumar
>
> Good catch! Could you please forward-port this to current rcu/dev?
> This now includes the fix you mentioned above, which generates conflicts.
>
> Could you please also fix the use in rcu_is_nocb_cpu()?
>
> And please see the comment below.
>
Please find the fixed patch.
--
Pranith
From: Pranith Kumar
Date: Thu, 17 Jul 2014 20:02:54 -0400
Subject: [PATCH 1/1] rcu: Feedback comments
If we configure a kernel with CONFIG_NOCB_CPU=y, CONFIG_RCU_NOCB_CPU_NONE=y and
CONFIG_CPUMASK_OFFSTACK=n and do not pass in a rcu_nocb= boot parameter, the
cpumask rcu_nocb_mask can be garbage instead of NULL.
Hence this commit replaces checks for rcu_nocb_mask == NULL with a check for
have_rcu_nocb_mask.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index ce8c331..8f987c1 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2053,9 +2053,10 @@ static void rcu_init_one_nocb(struct rcu_node *rnp)
/* Is the specified CPU a no-CBs CPU? */
bool rcu_is_nocb_cpu(int cpu)
{
- if (have_rcu_nocb_mask)
- return cpumask_test_cpu(cpu, rcu_nocb_mask);
- return false;
+ if (!have_rcu_nocb_mask)
+ return false;
+
+ return cpumask_test_cpu(cpu, rcu_nocb_mask);
}
#endif /* #ifndef CONFIG_RCU_NOCB_CPU_ALL */
@@ -2540,7 +2541,7 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
struct rcu_data *rdp_leader = NULL; /* Suppress misguided gcc warn. */
struct rcu_data *rdp_prev = NULL;
- if (rcu_nocb_mask == NULL)
+ if (!have_rcu_nocb_mask)
return;
#if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL)
if (tick_nohz_full_running)
@@ -2574,9 +2575,9 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
/* Prevent __call_rcu() from enqueuing callbacks on no-CBs CPUs */
static bool init_nocb_callback_list(struct rcu_data *rdp)
{
- if (rcu_nocb_mask == NULL ||
- !cpumask_test_cpu(rdp->cpu, rcu_nocb_mask))
+ if (!rcu_is_nocb_cpu(rdp->cpu))
return false;
+
rdp->nxttail[RCU_NEXT_TAIL] = NULL;
return true;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 11:19 AM, Christoph Lameter wrote:
> Regarding atomic_t in per cpu areas: I am uncomfortable especially
> because both locked and unlocked RMW write operations could be acting on
> values in the same cacheline. I am concerned that the unlocked operation
> could have an unpredictable result.
>
>
> f.e. the following per cpu data structure
>
> struct test {
> atomic_t a;
> int b;
> } onecacheline;
>
>
> Local cpu does
>
> this_cpu_inc(onecacheline.b);
>
> If this is racing with a remote cpus:
>
> atomic_inc(percpu(, cpu))
>
> then we have on x86 a increment operation with locked semantics racing
> with an unlocked one on the same cacheline.
>
OK, I will add this as a warning in the documentation. Thanks!
--
Pranith
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 11:26 AM, Christoph Lameter wrote: > On Thu, 17 Jul 2014, Pranith Kumar wrote: > >> I can mention that IPI is preferable. What is that you don't want mentioned? >> atomic_t? > > Definitely not as an example. atomic_t in per cpu areas is self > contradicting. The per cpu area is exclusively for that processor whereas > an atomic_t is supposed to be accessed from multiple processors. > >>> Remote percpu updates are extremely rare events. If the cpu is idle/asleep >>> then usually no updates are needed because no activity is occurring on >>> that cpu. >>> >> >> Yes, -usually- that is the case. But we are talking about the extreme rare >> event >> where we need to update some remote CPU`s per-cpu data without waking it up >> from >> sleep/idle. How do you suggest we handle this? I don't think suggesting not >> to >> use per-cpu areas because of this is a good idea, since we lose a lot of >> performance in the most common cases. > > If you modify a percpu area then that is usually done because that cpu > needs to take some action. An IPI is fine. > > Otherwise yes I would suggest not use a percpu area but a separate data > structure for synchronization. > Yes, I will add this information to the doc. Thanks! -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: Check for have_rcu_nocb_mask instead of rcu_nocb_mask
Hi Paul,
This is something similar to what you found yesterday with tick_nohz_full mask.
--
Pranith
If we configure a kernel with CONFIG_NOCB_CPU=y, CONFIG_RCU_NOCB_CPU_NONE=y and
CONFIG_CPUMASK_OFFSTACK=n and do not pass in a rcu_nocb= boot parameter, the
cpumask rcu_nocb_mask can be garbage instead of NULL.
Hence this commit replaces checks for rcu_nocb_mask == NULL with a check for
have_rcu_nocb_mask.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..15c00b5 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2542,7 +2542,7 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
struct rcu_data *rdp_leader = NULL; /* Suppress misguided gcc warn. */
struct rcu_data *rdp_prev = NULL;
- if (rcu_nocb_mask == NULL)
+ if (!have_rcu_nocb_mask)
return;
#ifdef CONFIG_NO_HZ_FULL
cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
@@ -2575,7 +2575,7 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
/* Prevent __call_rcu() from enqueuing callbacks on no-CBs CPUs */
static bool init_nocb_callback_list(struct rcu_data *rdp)
{
- if (rcu_nocb_mask == NULL ||
+ if (have_rcu_nocb_mask == false ||
!cpumask_test_cpu(rdp->cpu, rcu_nocb_mask))
return false;
rdp->nxttail[RCU_NEXT_TAIL] = NULL;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 10:55 AM, Christoph Lameter wrote: > On Thu, 17 Jul 2014, Pranith Kumar wrote: > >>> The RCU code has u... some issues with percpu usage and should >>> not be taken as a good example. If you look at the RCU code it looks >>> horrible with numerous barriers around remote percpu read/wrirte >>> accesses and one wonders if that code is actually ok. >> >> Well, it is running in all our kernels with not many reported issues, isn't >> it ;) >> And yes, that is one of the extra-ordinary situations where we use per-cpu >> data. >> Once you've extracted a pointer to the per-cpu area -and- ensure that >> concurrent >> accesses do not happen(or happen with enough guarantees using barriers), >> what is >> the case against remote accesses? I am asking from a correctness and a >> performance point of view, not style/aesthetics. > > Could be working but I do not want it to be mentioned in the > documentation given the problems it causes. IPI is preferable. I can mention that IPI is preferable. What is that you don't want mentioned? atomic_t? > >>>> If data needs to be modified from multiple cpus only very rarely, doesn't >>>> it >>>> make sense to use per-cpu areas? >>> >>> I would suggest that this should not occur. You can always "modify" remote >>> percpu areas by generating an IPI on that cpu to make that processor >>> update its own per cpu data. >>> >> >> The case against doing that is not to wake up CPUs which are in idle/sleep >> states. I think mentioning it here that remote accesses are strongly >> discouraged >> with a reasonable explanation of the implications should be enough. There >> might >> always be rare situations where remote accesses might be necessary. > > Remote percpu updates are extremely rare events. If the cpu is idle/asleep > then usually no updates are needed because no activity is occurring on > that cpu. > Yes, -usually- that is the case. But we are talking about the extreme rare event where we need to update some remote CPU`s per-cpu data without waking it up from sleep/idle. How do you suggest we handle this? I don't think suggesting not to use per-cpu areas because of this is a good idea, since we lose a lot of performance in the most common cases. Thoughts? -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 10:39 AM, Christoph Lameter wrote: > On Thu, 17 Jul 2014, Pranith Kumar wrote: > >>> The use of atomic_t implies a remote write operation to a percpu area. >>> >>> atomic_t needs to be avoided. If data needs to be modified from multiple >>> cpus then it usually does not belong into a percpu area. >>> >> >> Yes, I think I made it pretty clear that remote accesses need to be avoided >> unless absolutely necessary. But, there will be scenarios where mostly local >> data will need to be have remote accesses. In such scenarios, isn't using >> atomic_t better? FYI, that is how RCU code currently works. It uses atomic_t >> in >> per cpu areas to ensure atomicity for remote accesses. > > The RCU code has u... some issues with percpu usage and should > not be taken as a good example. If you look at the RCU code it looks > horrible with numerous barriers around remote percpu read/wrirte > accesses and one wonders if that code is actually ok. Well, it is running in all our kernels with not many reported issues, isn't it ;) And yes, that is one of the extra-ordinary situations where we use per-cpu data. Once you've extracted a pointer to the per-cpu area -and- ensure that concurrent accesses do not happen(or happen with enough guarantees using barriers), what is the case against remote accesses? I am asking from a correctness and a performance point of view, not style/aesthetics. > >> If data needs to be modified from multiple cpus only very rarely, doesn't it >> make sense to use per-cpu areas? > > I would suggest that this should not occur. You can always "modify" remote > percpu areas by generating an IPI on that cpu to make that processor > update its own per cpu data. > The case against doing that is not to wake up CPUs which are in idle/sleep states. I think mentioning it here that remote accesses are strongly discouraged with a reasonable explanation of the implications should be enough. There might always be rare situations where remote accesses might be necessary. -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 10:39 AM, Christoph Lameter wrote: On Thu, 17 Jul 2014, Pranith Kumar wrote: The use of atomic_t implies a remote write operation to a percpu area. atomic_t needs to be avoided. If data needs to be modified from multiple cpus then it usually does not belong into a percpu area. Yes, I think I made it pretty clear that remote accesses need to be avoided unless absolutely necessary. But, there will be scenarios where mostly local data will need to be have remote accesses. In such scenarios, isn't using atomic_t better? FYI, that is how RCU code currently works. It uses atomic_t in per cpu areas to ensure atomicity for remote accesses. The RCU code has u... some issues with percpu usage and should not be taken as a good example. If you look at the RCU code it looks horrible with numerous barriers around remote percpu read/wrirte accesses and one wonders if that code is actually ok. Well, it is running in all our kernels with not many reported issues, isn't it ;) And yes, that is one of the extra-ordinary situations where we use per-cpu data. Once you've extracted a pointer to the per-cpu area -and- ensure that concurrent accesses do not happen(or happen with enough guarantees using barriers), what is the case against remote accesses? I am asking from a correctness and a performance point of view, not style/aesthetics. If data needs to be modified from multiple cpus only very rarely, doesn't it make sense to use per-cpu areas? I would suggest that this should not occur. You can always modify remote percpu areas by generating an IPI on that cpu to make that processor update its own per cpu data. The case against doing that is not to wake up CPUs which are in idle/sleep states. I think mentioning it here that remote accesses are strongly discouraged with a reasonable explanation of the implications should be enough. There might always be rare situations where remote accesses might be necessary. -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 10:55 AM, Christoph Lameter wrote: On Thu, 17 Jul 2014, Pranith Kumar wrote: The RCU code has u... some issues with percpu usage and should not be taken as a good example. If you look at the RCU code it looks horrible with numerous barriers around remote percpu read/wrirte accesses and one wonders if that code is actually ok. Well, it is running in all our kernels with not many reported issues, isn't it ;) And yes, that is one of the extra-ordinary situations where we use per-cpu data. Once you've extracted a pointer to the per-cpu area -and- ensure that concurrent accesses do not happen(or happen with enough guarantees using barriers), what is the case against remote accesses? I am asking from a correctness and a performance point of view, not style/aesthetics. Could be working but I do not want it to be mentioned in the documentation given the problems it causes. IPI is preferable. I can mention that IPI is preferable. What is that you don't want mentioned? atomic_t? If data needs to be modified from multiple cpus only very rarely, doesn't it make sense to use per-cpu areas? I would suggest that this should not occur. You can always modify remote percpu areas by generating an IPI on that cpu to make that processor update its own per cpu data. The case against doing that is not to wake up CPUs which are in idle/sleep states. I think mentioning it here that remote accesses are strongly discouraged with a reasonable explanation of the implications should be enough. There might always be rare situations where remote accesses might be necessary. Remote percpu updates are extremely rare events. If the cpu is idle/asleep then usually no updates are needed because no activity is occurring on that cpu. Yes, -usually- that is the case. But we are talking about the extreme rare event where we need to update some remote CPU`s per-cpu data without waking it up from sleep/idle. How do you suggest we handle this? I don't think suggesting not to use per-cpu areas because of this is a good idea, since we lose a lot of performance in the most common cases. Thoughts? -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: Check for have_rcu_nocb_mask instead of rcu_nocb_mask
Hi Paul,
This is something similar to what you found yesterday with tick_nohz_full mask.
--
Pranith
If we configure a kernel with CONFIG_NOCB_CPU=y, CONFIG_RCU_NOCB_CPU_NONE=y and
CONFIG_CPUMASK_OFFSTACK=n and do not pass in a rcu_nocb= boot parameter, the
cpumask rcu_nocb_mask can be garbage instead of NULL.
Hence this commit replaces checks for rcu_nocb_mask == NULL with a check for
have_rcu_nocb_mask.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree_plugin.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..15c00b5 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2542,7 +2542,7 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
struct rcu_data *rdp_leader = NULL; /* Suppress misguided gcc warn. */
struct rcu_data *rdp_prev = NULL;
- if (rcu_nocb_mask == NULL)
+ if (!have_rcu_nocb_mask)
return;
#ifdef CONFIG_NO_HZ_FULL
cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
@@ -2575,7 +2575,7 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
/* Prevent __call_rcu() from enqueuing callbacks on no-CBs CPUs */
static bool init_nocb_callback_list(struct rcu_data *rdp)
{
- if (rcu_nocb_mask == NULL ||
+ if (have_rcu_nocb_mask == false ||
!cpumask_test_cpu(rdp-cpu, rcu_nocb_mask))
return false;
rdp-nxttail[RCU_NEXT_TAIL] = NULL;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 11:26 AM, Christoph Lameter wrote: On Thu, 17 Jul 2014, Pranith Kumar wrote: I can mention that IPI is preferable. What is that you don't want mentioned? atomic_t? Definitely not as an example. atomic_t in per cpu areas is self contradicting. The per cpu area is exclusively for that processor whereas an atomic_t is supposed to be accessed from multiple processors. Remote percpu updates are extremely rare events. If the cpu is idle/asleep then usually no updates are needed because no activity is occurring on that cpu. Yes, -usually- that is the case. But we are talking about the extreme rare event where we need to update some remote CPU`s per-cpu data without waking it up from sleep/idle. How do you suggest we handle this? I don't think suggesting not to use per-cpu areas because of this is a good idea, since we lose a lot of performance in the most common cases. If you modify a percpu area then that is usually done because that cpu needs to take some action. An IPI is fine. Otherwise yes I would suggest not use a percpu area but a separate data structure for synchronization. Yes, I will add this information to the doc. Thanks! -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
On 07/17/2014 11:19 AM, Christoph Lameter wrote:
Regarding atomic_t in per cpu areas: I am uncomfortable especially
because both locked and unlocked RMW write operations could be acting on
values in the same cacheline. I am concerned that the unlocked operation
could have an unpredictable result.
f.e. the following per cpu data structure
struct test {
atomic_t a;
int b;
} onecacheline;
Local cpu does
this_cpu_inc(onecacheline.b);
If this is racing with a remote cpus:
atomic_inc(percpu(a, cpu))
then we have on x86 a increment operation with locked semantics racing
with an unlocked one on the same cacheline.
OK, I will add this as a warning in the documentation. Thanks!
--
Pranith
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] rcu: Check for have_rcu_nocb_mask instead of rcu_nocb_mask
On 07/17/2014 07:49 PM, Paul E. McKenney wrote:
On Thu, Jul 17, 2014 at 05:30:11PM -0400, Pranith Kumar wrote:
Hi Paul,
This is something similar to what you found yesterday with tick_nohz_full
mask.
--
Pranith
If we configure a kernel with CONFIG_NOCB_CPU=y, CONFIG_RCU_NOCB_CPU_NONE=y
and
CONFIG_CPUMASK_OFFSTACK=n and do not pass in a rcu_nocb= boot parameter, the
cpumask rcu_nocb_mask can be garbage instead of NULL.
Hence this commit replaces checks for rcu_nocb_mask == NULL with a check for
have_rcu_nocb_mask.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
Good catch! Could you please forward-port this to current rcu/dev?
This now includes the fix you mentioned above, which generates conflicts.
Could you please also fix the use in rcu_is_nocb_cpu()?
And please see the comment below.
Please find the fixed patch.
--
Pranith
From: Pranith Kumar bobby.pr...@gmail.com
Date: Thu, 17 Jul 2014 20:02:54 -0400
Subject: [PATCH 1/1] rcu: Feedback comments
If we configure a kernel with CONFIG_NOCB_CPU=y, CONFIG_RCU_NOCB_CPU_NONE=y and
CONFIG_CPUMASK_OFFSTACK=n and do not pass in a rcu_nocb= boot parameter, the
cpumask rcu_nocb_mask can be garbage instead of NULL.
Hence this commit replaces checks for rcu_nocb_mask == NULL with a check for
have_rcu_nocb_mask.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree_plugin.h | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index ce8c331..8f987c1 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2053,9 +2053,10 @@ static void rcu_init_one_nocb(struct rcu_node *rnp)
/* Is the specified CPU a no-CBs CPU? */
bool rcu_is_nocb_cpu(int cpu)
{
- if (have_rcu_nocb_mask)
- return cpumask_test_cpu(cpu, rcu_nocb_mask);
- return false;
+ if (!have_rcu_nocb_mask)
+ return false;
+
+ return cpumask_test_cpu(cpu, rcu_nocb_mask);
}
#endif /* #ifndef CONFIG_RCU_NOCB_CPU_ALL */
@@ -2540,7 +2541,7 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
struct rcu_data *rdp_leader = NULL; /* Suppress misguided gcc warn. */
struct rcu_data *rdp_prev = NULL;
- if (rcu_nocb_mask == NULL)
+ if (!have_rcu_nocb_mask)
return;
#if defined(CONFIG_NO_HZ_FULL) !defined(CONFIG_NO_HZ_FULL_ALL)
if (tick_nohz_full_running)
@@ -2574,9 +2575,9 @@ static void __init rcu_organize_nocb_kthreads(struct
rcu_state *rsp)
/* Prevent __call_rcu() from enqueuing callbacks on no-CBs CPUs */
static bool init_nocb_callback_list(struct rcu_data *rdp)
{
- if (rcu_nocb_mask == NULL ||
- !cpumask_test_cpu(rdp-cpu, rcu_nocb_mask))
+ if (!rcu_is_nocb_cpu(rdp-cpu))
return false;
+
rdp-nxttail[RCU_NEXT_TAIL] = NULL;
return true;
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC tip/core/rcu 1/2] rcu: Rationalize kthread spawning
On 07/16/2014 10:57 PM, Sasha Levin wrote: > On 07/14/2014 06:06 AM, Paul E. McKenney wrote: >> From: "Paul E. McKenney" >> >> Currently, RCU spawns kthreads from several different early_initcall() >> functions. Although this has served RCU well for quite some time, >> as more kthreads are added a more deterministic approach is required. >> This commit therefore causes all of RCU's early-boot kthreads to be >> spawned from a single early_initcall() function. >> >> Signed-off-by: Paul E. McKenney > > Hey Paul, > > I've updated to today's -next and my VM started hanging on boot. Bisect > pointed out this patch. > > I don't have any further info right now, but can provide whatever you'll > find useful. Hi Sasha, Could you attach the config file which is failing for a start? Thanks, -- Pranith > > > Thanks, > Sasha > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 10:14 PM, Josh Triplett wrote:
> On Wed, Jul 16, 2014 at 09:01:52PM -0400, Pranith Kumar wrote:
>> Sure, please find an updated patch with Josh Triplett's sign-off added:
>
> It appears to have a reviewed-by from someone named "Joe Tripplett"
> instead. ;)
>
I apologize for fat-fingering this.
Since I've sent one too many emails in this thread already, I suppose on more
can not do much harm :)
This time with Reviewed-by Lai Jiangshan added and Reviewed-by Josh Triplett
corrected.
From: Pranith Kumar
Date: Mon, 14 Jul 2014 16:01:05 -0400
Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c
This commit removes a stale comment in rcu/tree.c which was left out when some
code was moved around previously in the commit
2036d94a7b61 ("rcu: Rework detection of use of RCU by offline CPUs")
For reference, the following updated comment exists a few lines below this which
means the same.
/* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */
Signed-off-by: Pranith Kumar
Reviewed-by: Josh Triplett
Reviewed-by: Lai Jiangshan
---
kernel/rcu/tree.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index a1abaa8..e67246e 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct
rcu_state *rsp)
/* Adjust any no-longer-needed kthreads. */
rcu_boost_kthread_setaffinity(rnp, -1);
- /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */
-
/* Exclude any attempts to start a new grace period. */
mutex_lock(>onoff_mutex);
raw_spin_lock_irqsave(>orphan_lock, flags);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On 07/16/2014 09:14 AM, Paul E. McKenney wrote: > On Mon, Jul 14, 2014 at 09:27:00AM -0400, Pranith Kumar wrote: >> On Sat, Jul 12, 2014 at 8:08 AM, Paul E. McKenney wrote: >>> >>> They ensure that any RCU read-side critical sections that took place before >>> the current (or previous) idle/userspace period on the remote CPU in >>> question are seen as having completed before the completion of the current >>> grace period. It also ensures that any RCU read-side critical sections >>> that extend beyond the end of the current grace period (thus starting >>> after the current (or previous) idle/userspace period) see any updates >>> that were carried out before the beginning of the current grace period. >>> >>> Of course, that is also the purpose of many of RCU's memory barriers, >>> so this probably doesn't help much. An alternative explanation is that >>> it ensures a coherent view of the ->dynticks counter, but I am quite >>> sure that this helps even less. >>> >>> So here is the problem we are having: >>> >>> The dyntick_save_progress_counter() and rcu_implicit_dynticks_qs() >>> functions are really bad places to start reading the RCU code. I would >>> say that starting there is like learning to swim by diving into the deep >>> end of a swimming pool, but that doesn't really capture it. Instead, >>> it is more like learning to swim by diving from the top of this waterfall: >>> >>> http://blog.pacificnorthwestphotography.com/wp-content/uploads/2011/03/54.jpg >>> >>> To understand these functions, you will first need to understand how >>> the rest of RCU works. These functions are tiny cogs in RCU's energy >>> efficiency optimization mechanism, which fits into the larger grace-period >>> detection mechanism. The purpose of the two atomic operations is to >>> preserve the memory-ordering guarantees called out in the docbook header >>> comments for call_rcu() and synchronize_rcu(), and I must confess that >>> it is not clear to me that you actually read these header comments. >>> Even so, these two functions interact with lots of other accesses to >>> implement these guarantees -- so again, it is really really difficult >>> to understand these two functions in isolation. >>> >>> Please see the end of this message for my suggested order of learning >>> the RCU code. A study plan, if you will. >> >> This guide helps a lot, thank you for the detailed study plan. I will >> make sure to go slow and steady. :) > > Best of everything with it! > Thanks a lot, Paul, for taking the time to help me understand. Hopefully one of these days, after reading the study list, I actually will :) -- Pranith. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 09:25 PM, Lai Jiangshan wrote:
> On 07/17/2014 09:01 AM, Pranith Kumar wrote:
>> On 07/16/2014 08:55 PM, Lai Jiangshan wrote:
>>> On 07/16/2014 09:29 PM, Pranith Kumar wrote:
>>>> On 07/16/2014 08:47 AM, Paul E. McKenney wrote:
>>>>> On Tue, Jul 15, 2014 at 06:57:59PM -0400, Pranith Kumar wrote:
>>>>>>
>>>>>> On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote:
>>>>>>> On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote:
>>>>>>>> This commit removes a stale comment in rcu/tree.c.
>>>>>>>> FYI, an updated comment exists a few lines below this.
>>>>>>>>
>>>>>>>> Signed-off-by: Pranith Kumar
>>>>>>> In general, when removing a stale comment, I'd suggest explaining why
>>>>>>> the comment is stale. Was code removed without removing the
>>>>>>> corresponding comment, or was code changed such that the comment no
>>>>>>> longer applies, or...?
>>>>>>
>>>>>> I guess it was left out when code was moved around previously. And I did
>>>>>> mention that an updated comment is there a few lines below.
>>>>>>
>>>>>> For reference this is the new comment which is below the old comment,
>>>>>> they mean the same :)
>>>>>>
>>>>>> /* Remove the outgoing CPU from the masks in the rcu_node hierarchy.
>>>>>> */
>>>>>
>>>>> Indeed that is the case.
>>>>>
>>>>> Please update the commit log with this explanation and resend.
>>>>>
>>>>> Thanx, Paul
>>>>>
>>>>
>>>> Please find the updated patch below.
>>>>
>>>> --
>>>> Pranith
>>>>
>>>> From: Pranith Kumar
>>>> Date: Mon, 14 Jul 2014 16:01:05 -0400
>>>> Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c
>>>>
>>>> This commit removes a stale comment in rcu/tree.c which was left out when
>>>> some
>>>> code was moved around previously.
>>>
>>> Which commit caused this leftover comment? Could you mention that commit in
>>> your changlog?
>>>
>>> 12BitsCmmtID ("commit title...")
>>>
>>
>> Sure, please find an updated patch with Josh Triplett's sign-off added:
>>
>> From: Pranith Kumar
>> Date: Mon, 14 Jul 2014 16:01:05 -0400
>> Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c
>>
>>
>> This commit removes a stale comment in rcu/tree.c which was left out in
>> commit 2036d94a7b61ca5032ce (rcu: Rework detection of use of RCU by offline
>> CPUs)
>
> I suggest you use the following syntax in future.
>
> 2036d94a7b61 ("rcu: Rework detection of use of RCU by offline CPUs")
>
OK. I will do that from now on. Thanks! :)
--
Pranith
>>
>> For reference, the following updated comment exists a few lines below this
>> which
>> means the same.
>>
>> /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */
>>
>> Signed-off-by: Pranith Kumar
>> Reviewed-by: Joe Tripplett
>
> Reviewed-by: Lai Jiangshan
>
>> ---
>> kernel/rcu/tree.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>> index a1abaa8..e67246e 100644
>> --- a/kernel/rcu/tree.c
>> +++ b/kernel/rcu/tree.c
>> @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct
>> rcu_state *rsp)
>> /* Adjust any no-longer-needed kthreads. */
>> rcu_boost_kthread_setaffinity(rnp, -1);
>>
>> -/* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */
>> -
>> /* Exclude any attempts to start a new grace period. */
>> mutex_lock(>onoff_mutex);
>> raw_spin_lock_irqsave(>orphan_lock, flags);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 08:55 PM, Lai Jiangshan wrote:
> On 07/16/2014 09:29 PM, Pranith Kumar wrote:
>> On 07/16/2014 08:47 AM, Paul E. McKenney wrote:
>>> On Tue, Jul 15, 2014 at 06:57:59PM -0400, Pranith Kumar wrote:
>>>>
>>>> On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote:
>>>>> On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote:
>>>>>> This commit removes a stale comment in rcu/tree.c.
>>>>>> FYI, an updated comment exists a few lines below this.
>>>>>>
>>>>>> Signed-off-by: Pranith Kumar
>>>>> In general, when removing a stale comment, I'd suggest explaining why
>>>>> the comment is stale. Was code removed without removing the
>>>>> corresponding comment, or was code changed such that the comment no
>>>>> longer applies, or...?
>>>>
>>>> I guess it was left out when code was moved around previously. And I did
>>>> mention that an updated comment is there a few lines below.
>>>>
>>>> For reference this is the new comment which is below the old comment, they
>>>> mean the same :)
>>>>
>>>> /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */
>>>
>>> Indeed that is the case.
>>>
>>> Please update the commit log with this explanation and resend.
>>>
>>> Thanx, Paul
>>>
>>
>> Please find the updated patch below.
>>
>> --
>> Pranith
>>
>> From: Pranith Kumar
>> Date: Mon, 14 Jul 2014 16:01:05 -0400
>> Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c
>>
>> This commit removes a stale comment in rcu/tree.c which was left out when
>> some
>> code was moved around previously.
>
> Which commit caused this leftover comment? Could you mention that commit in
> your changlog?
>
> 12BitsCmmtID ("commit title...")
>
Sure, please find an updated patch with Josh Triplett's sign-off added:
From: Pranith Kumar
Date: Mon, 14 Jul 2014 16:01:05 -0400
Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c
This commit removes a stale comment in rcu/tree.c which was left out in
commit 2036d94a7b61ca5032ce (rcu: Rework detection of use of RCU by offline
CPUs)
For reference, the following updated comment exists a few lines below this which
means the same.
/* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */
Signed-off-by: Pranith Kumar
Reviewed-by: Joe Tripplett
---
kernel/rcu/tree.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index a1abaa8..e67246e 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct
rcu_state *rsp)
/* Adjust any no-longer-needed kthreads. */
rcu_boost_kthread_setaffinity(rnp, -1);
- /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */
-
/* Exclude any attempts to start a new grace period. */
mutex_lock(>onoff_mutex);
raw_spin_lock_irqsave(>orphan_lock, flags);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/1] rcu: Allow user to ovveride RCU_NOCB_CPU_ALL at boot time
On 07/16/2014 08:26 PM, Paul E. McKenney wrote: > On Wed, Jul 16, 2014 at 06:38:08PM -0400, Pranith Kumar wrote: >> A kernel built with RCU_NOCB_CPU_ALL build time option will offload callbacks >> from all CPUs. The user cannot override this behavior without recompiling the >> kernel with the RCU_NOCB_CPU_ALL option turned off. >> >> This commit allows the user to override the build-time option by using the >> rcu_nocbs= boot time option without needing to recompile the kernel. >> >> Please note that this is how NO_HZ_FULL_ALL build time option works and this >> commit makes it work similar to that. >> >> Signed-off-by: Pranith Kumar Hi Paul, > I cannot accept this patch. For one thing, tick_nohz_init_all() looks > a bit on the unconditional side when CONFIG_NO_HZ_FULL_ALL=y. For > another thing, we really do not want to be handing the user a tool that > allows CPUs that are nohz_full to not be no-CBs CPUs. For another thing, I thought the latest patch does not allow that by ORing the nohz_full and rcu_nocbs mask. Doesn't it? All nohz_full CPUs will be nocb CPUS. We can mention this explicitly in the kernel-parameters.txt file. > if we add this and it turns out to be a bad idea, it will be difficult > to take it back -- someone somewhere will no doubt have scripted the > boot parameter. This option already exists in the kernel when RCU_NOCB_CPU_ALL is not set. The user can pass in rcu_nocbs= at boot time. I am not sure we are adding anything new with this patch. Most of the distro kernels set RCU_NOCB_CPU_ALL and rightly so, as it is the suggested and most appropriate option. This patch will make it easier for users who want to specify nocb CPUs for their needs, without having to recompile the kernel. > > Thanx, Paul > > -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
Add more details from a recent kernel newbies mailing list discussion here: http://www.spinics.net/lists/newbies/msg52747.html Also list the operations available and add details about safe accesses. v2: * updated with comments from Christoph Lameter Signed-off-by: Pranith Kumar CC: Christoph Lameter --- Documentation/this_cpu_ops.txt | 129 ++--- scripts/bin2c | Bin 0 -> 8706 bytes 2 files changed, 108 insertions(+), 21 deletions(-) create mode 100755 scripts/bin2c diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..b1faa9d 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -13,22 +13,51 @@ operating on the per cpu variable. This means there are no atomicity issues between the calculation of the offset and the operation on the data. Therefore it is not -necessary to disable preempt or interrupts to ensure that the +necessary to disable preemption or interrupts to ensure that the processor is not changed between the calculation of the address and the operation on the data. Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate -without the typical synchronization overhead but still provide some -sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +without the typical synchronization overhead, but still provide some +sort of relaxed atomicity guarantees. The x86, for example, can execute +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but synchronization is not necessary since we are dealing with per cpu data specific to the currently executing processor. Only the current processor should be accessing that variable and therefore there are no -concurrency issues with other processors in the system. +concurrency issues with other processors in the system. Please see the +section "Remote access to per-CPU data" if you need remote access. + +The main use of the this_cpu operations has been to optimize counter +operations. + +The following this_cpu() operations with implied preemption protection +are defined. These operations can be used without worrying about +preemption and interrupts. + + this_cpu_add() + this_cpu_read(pcp) + this_cpu_write(pcp, val) + this_cpu_add(pcp, val) + this_cpu_and(pcp, val) + this_cpu_or(pcp, val) + this_cpu_add_return(pcp, val) + this_cpu_xchg(pcp, nval) + this_cpu_cmpxchg(pcp, oval, nval) + this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) + this_cpu_sub(pcp, val) + this_cpu_inc(pcp) + this_cpu_dec(pcp) + this_cpu_sub_return(pcp, val) + this_cpu_inc_return(pcp) + this_cpu_dec_return(pcp) + + +Inner working of this_cpu operations + On x86 the fs: or the gs: segment registers contain the base of the per cpu area. It is then possible to simply use the segment override @@ -53,12 +82,11 @@ this_cpu_ops such sequence also required preempt disable/enable to prevent the kernel from moving the thread to a different processor while the calculation is performed. -The main use of the this_cpu operations has been to optimize counter -operations. +Consider the following this_cpu operation this_cpu_inc(x) -results in the following single instruction (no lock prefix!) +The above results in the following single instruction (no lock prefix!) inc gs:[x] @@ -100,11 +128,10 @@ Takes the offset of a per cpu variable ( !) and returns the address of the per cpu variable that belongs to the currently executing processor. this_cpu_ptr avoids multiple steps that the common get_cpu/put_cpu sequence requires. No processor number is -available. Instead the offset of the local per cpu area is simply +available. Instead, the offset of the local per cpu area is simply added to the percpu offset. - Per cpu variables and offsets - @@ -118,15 +145,16 @@ Therefore the use of x or outside of the context of per cpu operations is invalid and will generally be treated like a NULL pointer dereference. -In the context of per cpu operations + DEFINE_PER_CPU(int, x); - x is a per cpu variable. Most this_cpu operations take a cpu - variable. +In the context of per cpu operations the above implies that x is a per +cpu variable. Most this_cpu operations take a cpu variable. -is the *offset* a per cpu variable. this_cpu_ptr() takes - the offset of a per cpu variable which makes th
[RFC PATCH 1/1] rcu: Allow user to ovveride RCU_NOCB_CPU_ALL at boot time
A kernel built with RCU_NOCB_CPU_ALL build time option will offload callbacks
from all CPUs. The user cannot override this behavior without recompiling the
kernel with the RCU_NOCB_CPU_ALL option turned off.
This commit allows the user to override the build-time option by using the
rcu_nocbs= boot time option without needing to recompile the kernel.
Please note that this is how NO_HZ_FULL_ALL build time option works and this
commit makes it work similar to that.
Signed-off-by: Pranith Kumar
---
init/Kconfig | 14 +++---
kernel/rcu/tree_plugin.h | 8
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index 41066e4..7d363a4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -768,13 +768,13 @@ config RCU_NOCB_CPU_ALL
bool "All CPUs are build_forced no-CBs CPUs"
depends on RCU_NOCB_CPU
help
- This option forces all CPUs to be no-CBs CPUs. The rcu_nocbs=
- boot parameter will be ignored. All CPUs' RCU callbacks will
- be executed in the context of per-CPU rcuo kthreads created for
- this purpose. Assuming that the kthreads whose names start with
- "rcuo" are bound to "housekeeping" CPUs, this reduces OS jitter
- on the remaining CPUs, but might decrease memory locality during
- RCU-callback invocation, thus potentially degrading throughput.
+ If the user doesn't pass the rcu_nocbs= boot option, force all CPUs
+ to be no-CBs CPUs. All CPUs' RCU callbacks will be executed in the
+ context of per-CPU rcuo kthreads created for this purpose. Assuming
+ that the kthreads whose names start with "rcuo" are bound to
+ "housekeeping" CPUs, this reduces OS jitter on the remaining CPUs, but
+ might decrease memory locality during RCU-callback invocation, thus
+ potentially degrading throughput.
Select this if all CPUs need to be no-CBs CPUs for real-time
or energy-efficiency reasons.
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index a2113f6..b97a939 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -92,15 +92,15 @@ static void __init rcu_bootup_announce_oddness(void)
if (!have_rcu_nocb_mask) {
zalloc_cpumask_var(_nocb_mask, GFP_KERNEL);
have_rcu_nocb_mask = true;
+#ifdef CONFIG_RCU_NOCB_CPU_ALL
+ pr_info("\tOffload RCU callbacks from all CPUs\n");
+ cpumask_copy(rcu_nocb_mask, cpu_possible_mask);
+#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ALL */
}
#ifdef CONFIG_RCU_NOCB_CPU_ZERO
pr_info("\tOffload RCU callbacks from CPU 0\n");
cpumask_set_cpu(0, rcu_nocb_mask);
#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ZERO */
-#ifdef CONFIG_RCU_NOCB_CPU_ALL
- pr_info("\tOffload RCU callbacks from all CPUs\n");
- cpumask_copy(rcu_nocb_mask, cpu_possible_mask);
-#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ALL */
#endif /* #ifndef CONFIG_RCU_NOCB_CPU_NONE */
if (have_rcu_nocb_mask) {
if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL rcu/next] RCU commits for 3.17
On 07/16/2014 02:15 PM, Pranith Kumar wrote:
>
> On 07/16/2014 01:24 PM, Paul E. McKenney wrote:
>>>> And of course if you don't actually specify a nohz_full= mask, then
>>>> tick_nohz_full_mask can be NULL at RCU initialization time, and if it
>>>> is also true that CONFIG_NO_HZ_FULL_ALL=n, this condition can persist
>>>> forever.
>>>>
>>> Hi Paul,
>>>
>>> The other location where tick_nohz_full_mask is being allocated is in
>>> tick_nohz_init_all(), called from tick_nohz_init(). rcu_init() is called
>>> before
>>> tick_nohz_init() in init/main.c. CONFIG_NO_HZ_FULL_ALL for allocation of the
>>> mask does not take effect when rcu_init() runs.
>>>
>>> So if nohz_full command line arg is not specified, tick_nohz_full_mask will
>>> always be NULL when rcu_init() runs. So just checking for
>>> tick_nohz_full_mask
>>> is NULL should be enough I guess.
>> Yep, that is indeed one of the conditions called out in the commit log
>> below.
>>
>> Thanx, Paul
>>
>
> Sorry, I was not clear.
>
> nohz_full= boot time parameter overrides the CONFIG_NO_HZ_FULL_ALL build time
> parameter.
> The check for !CONFIG_NO_HZ_FULL_ALL will disable masking out the cpus on
> which NOHZ is
> not enabled using the nohz_full boot time parameter, and hence you will
> enable callback
> offloading on all CPUs as was previously being done.
>
> So, if I have CONFIG_NO_HZ_FULL_ALL enabled at build time but pass nohz_cpu=1
> at boot
> time, all the CPUs will have callback offloading enabled.
>
> I think the following looks like what is needed in our case:
>
The previous patch will fail if CONFIG_NO_HZ_FULL is not defined, this should
work.
Thoughts?
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 00dc411..83f3f58 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2479,10 +2479,10 @@ static void __init rcu_spawn_nocb_kthreads(struct
rcu_state *rsp)
if (rcu_nocb_mask == NULL)
return;
-#if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL)
+#if defined(CONFIG_NO_HZ_FULL)
if (tick_nohz_full_running)
cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
-#endif /* #if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL) */
+#endif /* #if defined(CONFIG_NO_HZ_FULL) */
if (ls == -1) {
ls = int_sqrt(nr_cpu_ids);
rcu_nocb_leader_stride = ls;
--
Pranith
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL rcu/next] RCU commits for 3.17
On 07/16/2014 01:24 PM, Paul E. McKenney wrote:
>>> And of course if you don't actually specify a nohz_full= mask, then
>>> tick_nohz_full_mask can be NULL at RCU initialization time, and if it
>>> is also true that CONFIG_NO_HZ_FULL_ALL=n, this condition can persist
>>> forever.
>>>
>> Hi Paul,
>>
>> The other location where tick_nohz_full_mask is being allocated is in
>> tick_nohz_init_all(), called from tick_nohz_init(). rcu_init() is called
>> before
>> tick_nohz_init() in init/main.c. CONFIG_NO_HZ_FULL_ALL for allocation of the
>> mask does not take effect when rcu_init() runs.
>>
>> So if nohz_full command line arg is not specified, tick_nohz_full_mask will
>> always be NULL when rcu_init() runs. So just checking for tick_nohz_full_mask
>> is NULL should be enough I guess.
> Yep, that is indeed one of the conditions called out in the commit log
> below.
>
> Thanx, Paul
>
Sorry, I was not clear.
nohz_full= boot time parameter overrides the CONFIG_NO_HZ_FULL_ALL build time
parameter.
The check for !CONFIG_NO_HZ_FULL_ALL will disable masking out the cpus on which
NOHZ is
not enabled using the nohz_full boot time parameter, and hence you will enable
callback
offloading on all CPUs as was previously being done.
So, if I have CONFIG_NO_HZ_FULL_ALL enabled at build time but pass nohz_cpu=1
at boot
time, all the CPUs will have callback offloading enabled.
I think the following looks like what is needed in our case:
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 00dc411..1123ed4 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2479,10 +2479,10 @@ static void __init rcu_spawn_nocb_kthreads(struct
rcu_state *rsp)
if (rcu_nocb_mask == NULL)
return;
-#if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL)
+
if (tick_nohz_full_running)
cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
-#endif /* #if defined(CONFIG_NO_HZ_FULL) && !defined(CONFIG_NO_HZ_FULL_ALL) */
+
if (ls == -1) {
ls = int_sqrt(nr_cpu_ids);
rcu_nocb_leader_stride = ls;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL rcu/next] RCU commits for 3.17
On 07/16/2014 11:26 AM, Paul E. McKenney wrote: > On Wed, Jul 16, 2014 at 07:17:07AM -0700, Paul E. McKenney wrote: >> On Wed, Jul 16, 2014 at 03:13:22PM +0200, Ingo Molnar wrote: >>> >>> * Ingo Molnar wrote: >>> * Paul E. McKenney wrote: > Hello, Ingo, > > The changes in this series include: > > 1.Update RCU documentation. These were posted to LKML at > https://lkml.org/lkml/2014/7/7/650. > > 2.Miscellaneous fixes. These were posted to LKML at > https://lkml.org/lkml/2014/7/7/678. > > 3.Maintainership changes. These were posted to LKML at > https://lkml.org/lkml/2014/7/7/713, with a couple of > additional at https://lkml.org/lkml/2014/7/3/812 and > https://lkml.org/lkml/2014/6/2/585. > > 4.Torture-test updates. These were posted to LKML at > https://lkml.org/lkml/2014/7/7/816. > > 5.Callback-offloading changes. These were posted to LKML at > https://lkml.org/lkml/2014/7/7/1007. > > All of these have been exposed to -next testing. >>> >>> JFYI, the attached x86 (rand!-) config crashes on early bootup: >>> >>> [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=2 >>> [0.00] [ cut here ] >>> [0.00] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/cpu/common.c:1439 >>> warn_pre_alternatives+0x1e/0x20() >>> [0.00] You're using static_cpu_has before alternatives have run! >>> [0.00] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.16.0-rc5+ #236363 >>> [0.00] Hardware name: System manufacturer System Product >>> Name/A8N-E, BIOS ASUS A8N-E ACPI BIOS Revision 1008 08/22/2005 >>> [0.00] 82787c60 81ff4ec0 >>> 82787ca8 >>> [0.00] 82787c98 810651ac 81011531 >>> 82787e48 >>> [0.00] 827cf880 >>> 82787cf8 >>> [0.00] Call Trace: >>> [0.00] [] dump_stack+0x4d/0x66 >>> [0.00] [] warn_slowpath_common+0x7a/0x93 >>> [0.00] [] ? warn_pre_alternatives+0x1e/0x20 >>> [0.00] [] warn_slowpath_fmt+0x4c/0x4e >>> [0.00] [] ? irq_return+0x7/0x7 >>> [0.00] [] warn_pre_alternatives+0x1e/0x20 >>> [0.00] [] __do_page_fault+0xc3/0x43f >>> [0.00] [] ? print_context_stack+0x6a/0xb6 >>> [0.00] [] ? dump_trace+0x27d/0x294 >>> [0.00] [] ? number.isra.1+0x127/0x22c >>> [0.00] [] ? print_time.part.5+0x58/0x5c >>> [0.00] [] ? sched_clock_cpu+0x11/0xb9 >>> [0.00] [] do_page_fault+0x1e/0x54 >>> [0.00] [] ? irq_return+0x7/0x7 >>> [0.00] [] page_fault+0x22/0x30 >>> [0.00] [] ? __bitmap_or+0x15/0x28 >>> [0.00] [] rcu_init_one+0x4c0/0x55d >>> [0.00] [] rcu_init+0x270/0x2da >>> [0.00] [] start_kernel+0x24f/0x4d2 >>> [0.00] [] ? set_init_arg+0x53/0x53 >>> [0.00] [] x86_64_start_reservations+0x2a/0x2c >>> [0.00] [] x86_64_start_kernel+0xf1/0xf4 >>> [0.00] ---[ end trace 4650963e41188009 ]--- >>> [0.00] BUG: unable to handle kernel NULL pointer dereference at >>> (null) >>> [0.00] IP: [] __bitmap_or+0x15/0x28 >>> [0.00] PGD 0 >>> [0.00] Oops: [#1] SMP DEBUG_PAGEALLOC >>> [0.00] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW >>> 3.16.0-rc5+ #236363 >>> [0.00] Hardware name: System manufacturer System Product >>> Name/A8N-E, BIOS ASUS A8N-E ACPI BIOS Revision 1008 08/22/2005 >>> [0.00] task: 827a3480 ti: 82784000 task.ti: >>> 82784000 >>> [0.00] RIP: 0010:[] [] >>> __bitmap_or+0x15/0x28 >>> [0.00] RSP: :82787ef8 EFLAGS: 00010002 >>> [0.00] RAX: RBX: RCX: >>> 0001 >>> [0.00] RDX: RSI: 88019800 RDI: >>> 88019800 >>> [0.00] RBP: 82787ef8 R08: R09: >>> >>> [0.00] R10: R11: R12: >>> 827cf880 >>> [0.00] R13: 0002 R14: 827cf880 R15: >>> 001cd000 >>> [0.00] FS: () GS:88003f80() >>> knlGS: >>> [0.00] CS: 0010 DS: ES: CR0: 8005003b >>> [0.00] CR2: CR3: 0279e000 CR4: >>> 06b0 >>> [0.00] Stack: >>> [0.00] 82787f50 82c876f0 83b7e7a0 >>> 0001 >>> [0.00] 0082 0002 >>> 82d37920 >>> [0.00] 88003ffbba40 82d3e890 >>> 82787f80 >>> [0.00] Call Trace: >>> [0.00] [] rcu_init_one+0x4c0/0x55d >>> [0.00] []
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 08:47 AM, Paul E. McKenney wrote: > On Tue, Jul 15, 2014 at 06:57:59PM -0400, Pranith Kumar wrote: >> >> On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote: >>> On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote: >>>> This commit removes a stale comment in rcu/tree.c. >>>> FYI, an updated comment exists a few lines below this. >>>> >>>> Signed-off-by: Pranith Kumar >>> In general, when removing a stale comment, I'd suggest explaining why >>> the comment is stale. Was code removed without removing the >>> corresponding comment, or was code changed such that the comment no >>> longer applies, or...? >> >> I guess it was left out when code was moved around previously. And I did >> mention that an updated comment is there a few lines below. >> >> For reference this is the new comment which is below the old comment, they >> mean the same :) >> >> /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ > > Indeed that is the case. > > Please update the commit log with this explanation and resend. > > Thanx, Paul > Please find the updated patch below. -- Pranith From: Pranith Kumar Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out when some code was moved around previously. For reference, the following updated comment exists a few lines below this which means the same. /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Signed-off-by: Pranith Kumar --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); - /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(>onoff_mutex); raw_spin_lock_irqsave(>orphan_lock, flags); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] rcu: Use rcu_num_nodes instead of NUM_RCU_NODES
On 07/16/2014 08:45 AM, Paul E. McKenney wrote:
> On Tue, Jul 15, 2014 at 06:31:49PM -0400, Pranith Kumar wrote:
>> NUM_RCU_NODES is set at build time and is usually a huge number. We
>> calculate the
>> actual number of rcu nodes necessary at boot time based on nr_cpu_ids in
>> rcu_init_geometry() and store it in rcu_num_nodes. We should use this
>> variable
>> instead of NUM_RCU_NODES.
>>
>> This commit changes all such uses of NUM_RCU_NODES to rcu_num_nodes.
>>
>> Signed-off-by: Pranith Kumar
>> ---
>> kernel/rcu/tree_plugin.h | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>> index cedb020..17ccb62 100644
>> --- a/kernel/rcu/tree_plugin.h
>> +++ b/kernel/rcu/tree_plugin.h
>> @@ -46,6 +46,8 @@ static bool __read_mostly rcu_nocb_poll;/* Offload
>> kthread are to poll. */
>> static char __initdata nocb_buf[NR_CPUS * 5];
>> #endif /* #ifdef CONFIG_RCU_NOCB_CPU */
>>
>> +extern int rcu_num_nodes;
>
> This should not be necessary given the existing declaration in
> kernel/rcu/tree.c way before the #include of kernel/rcu/tree_plugin.h.
>
> Or did you get a build failure without this? (And if you did get a build
> failure, I would be really curious how that happened!)
>
> So please send an updated patch or tell me how your build managed to fail.
>
Indeed. The extern was unnecessary. Please find an updated patch below.
--
Pranith
From: Pranith Kumar
Date: Wed, 16 Jul 2014 09:21:49 -0400
Subject: [PATCH 1/1] rcu: Use rcu_num_nodes instead of NUM_RCU_NODES
NUM_RCU_NODES is set at build time and is usually a huge number. We calculate
the
actual number of rcu nodes necessary at boot time based on nr_cpu_ids in
rcu_init_geometry() and store it in rcu_num_nodes. We should use this variable
instead of NUM_RCU_NODES.
This commit changes all such uses of NUM_RCU_NODES to rcu_num_nodes.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..b99055a 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -885,7 +885,7 @@ void synchronize_rcu_expedited(void)
/* Snapshot current state of ->blkd_tasks lists. */
rcu_for_each_leaf_node(rsp, rnp)
sync_rcu_preempt_exp_init(rsp, rnp);
- if (NUM_RCU_NODES > 1)
+ if (rcu_num_nodes > 1)
sync_rcu_preempt_exp_init(rsp, rcu_get_root(rsp));
put_online_cpus();
@@ -1475,7 +1475,7 @@ static void __init rcu_spawn_boost_kthreads(void)
BUG_ON(smpboot_register_percpu_thread(_cpu_thread_spec));
rnp = rcu_get_root(rcu_state_p);
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
- if (NUM_RCU_NODES > 1) {
+ if (rcu_num_nodes > 1) {
rcu_for_each_leaf_node(rcu_state_p, rnp)
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] rcu: Use rcu_num_nodes instead of NUM_RCU_NODES
On 07/16/2014 08:45 AM, Paul E. McKenney wrote:
On Tue, Jul 15, 2014 at 06:31:49PM -0400, Pranith Kumar wrote:
NUM_RCU_NODES is set at build time and is usually a huge number. We
calculate the
actual number of rcu nodes necessary at boot time based on nr_cpu_ids in
rcu_init_geometry() and store it in rcu_num_nodes. We should use this
variable
instead of NUM_RCU_NODES.
This commit changes all such uses of NUM_RCU_NODES to rcu_num_nodes.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree_plugin.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..17ccb62 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -46,6 +46,8 @@ static bool __read_mostly rcu_nocb_poll;/* Offload
kthread are to poll. */
static char __initdata nocb_buf[NR_CPUS * 5];
#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
+extern int rcu_num_nodes;
This should not be necessary given the existing declaration in
kernel/rcu/tree.c way before the #include of kernel/rcu/tree_plugin.h.
Or did you get a build failure without this? (And if you did get a build
failure, I would be really curious how that happened!)
So please send an updated patch or tell me how your build managed to fail.
Indeed. The extern was unnecessary. Please find an updated patch below.
--
Pranith
From: Pranith Kumar bobby.pr...@gmail.com
Date: Wed, 16 Jul 2014 09:21:49 -0400
Subject: [PATCH 1/1] rcu: Use rcu_num_nodes instead of NUM_RCU_NODES
NUM_RCU_NODES is set at build time and is usually a huge number. We calculate
the
actual number of rcu nodes necessary at boot time based on nr_cpu_ids in
rcu_init_geometry() and store it in rcu_num_nodes. We should use this variable
instead of NUM_RCU_NODES.
This commit changes all such uses of NUM_RCU_NODES to rcu_num_nodes.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree_plugin.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..b99055a 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -885,7 +885,7 @@ void synchronize_rcu_expedited(void)
/* Snapshot current state of -blkd_tasks lists. */
rcu_for_each_leaf_node(rsp, rnp)
sync_rcu_preempt_exp_init(rsp, rnp);
- if (NUM_RCU_NODES 1)
+ if (rcu_num_nodes 1)
sync_rcu_preempt_exp_init(rsp, rcu_get_root(rsp));
put_online_cpus();
@@ -1475,7 +1475,7 @@ static void __init rcu_spawn_boost_kthreads(void)
BUG_ON(smpboot_register_percpu_thread(rcu_cpu_thread_spec));
rnp = rcu_get_root(rcu_state_p);
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
- if (NUM_RCU_NODES 1) {
+ if (rcu_num_nodes 1) {
rcu_for_each_leaf_node(rcu_state_p, rnp)
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 08:47 AM, Paul E. McKenney wrote: On Tue, Jul 15, 2014 at 06:57:59PM -0400, Pranith Kumar wrote: On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote: On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote: This commit removes a stale comment in rcu/tree.c. FYI, an updated comment exists a few lines below this. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com In general, when removing a stale comment, I'd suggest explaining why the comment is stale. Was code removed without removing the corresponding comment, or was code changed such that the comment no longer applies, or...? I guess it was left out when code was moved around previously. And I did mention that an updated comment is there a few lines below. For reference this is the new comment which is below the old comment, they mean the same :) /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Indeed that is the case. Please update the commit log with this explanation and resend. Thanx, Paul Please find the updated patch below. -- Pranith From: Pranith Kumar bobby.pr...@gmail.com Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out when some code was moved around previously. For reference, the following updated comment exists a few lines below this which means the same. /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); - /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(rsp-onoff_mutex); raw_spin_lock_irqsave(rsp-orphan_lock, flags); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL rcu/next] RCU commits for 3.17
On 07/16/2014 11:26 AM, Paul E. McKenney wrote: On Wed, Jul 16, 2014 at 07:17:07AM -0700, Paul E. McKenney wrote: On Wed, Jul 16, 2014 at 03:13:22PM +0200, Ingo Molnar wrote: * Ingo Molnar mi...@kernel.org wrote: * Paul E. McKenney paul...@linux.vnet.ibm.com wrote: Hello, Ingo, The changes in this series include: 1.Update RCU documentation. These were posted to LKML at https://lkml.org/lkml/2014/7/7/650. 2.Miscellaneous fixes. These were posted to LKML at https://lkml.org/lkml/2014/7/7/678. 3.Maintainership changes. These were posted to LKML at https://lkml.org/lkml/2014/7/7/713, with a couple of additional at https://lkml.org/lkml/2014/7/3/812 and https://lkml.org/lkml/2014/6/2/585. 4.Torture-test updates. These were posted to LKML at https://lkml.org/lkml/2014/7/7/816. 5.Callback-offloading changes. These were posted to LKML at https://lkml.org/lkml/2014/7/7/1007. All of these have been exposed to -next testing. JFYI, the attached x86 (rand!-) config crashes on early bootup: [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=2 [0.00] [ cut here ] [0.00] WARNING: CPU: 0 PID: 0 at arch/x86/kernel/cpu/common.c:1439 warn_pre_alternatives+0x1e/0x20() [0.00] You're using static_cpu_has before alternatives have run! [0.00] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.16.0-rc5+ #236363 [0.00] Hardware name: System manufacturer System Product Name/A8N-E, BIOS ASUS A8N-E ACPI BIOS Revision 1008 08/22/2005 [0.00] 82787c60 81ff4ec0 82787ca8 [0.00] 82787c98 810651ac 81011531 82787e48 [0.00] 827cf880 82787cf8 [0.00] Call Trace: [0.00] [81ff4ec0] dump_stack+0x4d/0x66 [0.00] [810651ac] warn_slowpath_common+0x7a/0x93 [0.00] [81011531] ? warn_pre_alternatives+0x1e/0x20 [0.00] [81065239] warn_slowpath_fmt+0x4c/0x4e [0.00] [82005817] ? irq_return+0x7/0x7 [0.00] [81011531] warn_pre_alternatives+0x1e/0x20 [0.00] [81033d31] __do_page_fault+0xc3/0x43f [0.00] [81005347] ? print_context_stack+0x6a/0xb6 [0.00] [810045f1] ? dump_trace+0x27d/0x294 [0.00] [815b185b] ? number.isra.1+0x127/0x22c [0.00] [8109abe2] ? print_time.part.5+0x58/0x5c [0.00] [81086a9a] ? sched_clock_cpu+0x11/0xb9 [0.00] [810340ef] do_page_fault+0x1e/0x54 [0.00] [82005817] ? irq_return+0x7/0x7 [0.00] [82006872] page_fault+0x22/0x30 [0.00] [815b65fa] ? __bitmap_or+0x15/0x28 [0.00] [82c876f0] rcu_init_one+0x4c0/0x55d [0.00] [82c87b00] rcu_init+0x270/0x2da [0.00] [82c6ec82] start_kernel+0x24f/0x4d2 [0.00] [82c6e841] ? set_init_arg+0x53/0x53 [0.00] [82c6e453] x86_64_start_reservations+0x2a/0x2c [0.00] [82c6e546] x86_64_start_kernel+0xf1/0xf4 [0.00] ---[ end trace 4650963e41188009 ]--- [0.00] BUG: unable to handle kernel NULL pointer dereference at (null) [0.00] IP: [815b65fa] __bitmap_or+0x15/0x28 [0.00] PGD 0 [0.00] Oops: [#1] SMP DEBUG_PAGEALLOC [0.00] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW 3.16.0-rc5+ #236363 [0.00] Hardware name: System manufacturer System Product Name/A8N-E, BIOS ASUS A8N-E ACPI BIOS Revision 1008 08/22/2005 [0.00] task: 827a3480 ti: 82784000 task.ti: 82784000 [0.00] RIP: 0010:[815b65fa] [815b65fa] __bitmap_or+0x15/0x28 [0.00] RSP: :82787ef8 EFLAGS: 00010002 [0.00] RAX: RBX: RCX: 0001 [0.00] RDX: RSI: 88019800 RDI: 88019800 [0.00] RBP: 82787ef8 R08: R09: [0.00] R10: R11: R12: 827cf880 [0.00] R13: 0002 R14: 827cf880 R15: 001cd000 [0.00] FS: () GS:88003f80() knlGS: [0.00] CS: 0010 DS: ES: CR0: 8005003b [0.00] CR2: CR3: 0279e000 CR4: 06b0 [0.00] Stack: [0.00] 82787f50 82c876f0 83b7e7a0 0001 [0.00] 0082 0002 82d37920 [0.00] 88003ffbba40 82d3e890 82787f80 [0.00] Call Trace:
Re: [GIT PULL rcu/next] RCU commits for 3.17
On 07/16/2014 01:24 PM, Paul E. McKenney wrote:
And of course if you don't actually specify a nohz_full= mask, then
tick_nohz_full_mask can be NULL at RCU initialization time, and if it
is also true that CONFIG_NO_HZ_FULL_ALL=n, this condition can persist
forever.
Hi Paul,
The other location where tick_nohz_full_mask is being allocated is in
tick_nohz_init_all(), called from tick_nohz_init(). rcu_init() is called
before
tick_nohz_init() in init/main.c. CONFIG_NO_HZ_FULL_ALL for allocation of the
mask does not take effect when rcu_init() runs.
So if nohz_full command line arg is not specified, tick_nohz_full_mask will
always be NULL when rcu_init() runs. So just checking for tick_nohz_full_mask
is NULL should be enough I guess.
Yep, that is indeed one of the conditions called out in the commit log
below.
Thanx, Paul
Sorry, I was not clear.
nohz_full= boot time parameter overrides the CONFIG_NO_HZ_FULL_ALL build time
parameter.
The check for !CONFIG_NO_HZ_FULL_ALL will disable masking out the cpus on which
NOHZ is
not enabled using the nohz_full boot time parameter, and hence you will enable
callback
offloading on all CPUs as was previously being done.
So, if I have CONFIG_NO_HZ_FULL_ALL enabled at build time but pass nohz_cpu=1
at boot
time, all the CPUs will have callback offloading enabled.
I think the following looks like what is needed in our case:
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 00dc411..1123ed4 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2479,10 +2479,10 @@ static void __init rcu_spawn_nocb_kthreads(struct
rcu_state *rsp)
if (rcu_nocb_mask == NULL)
return;
-#if defined(CONFIG_NO_HZ_FULL) !defined(CONFIG_NO_HZ_FULL_ALL)
+
if (tick_nohz_full_running)
cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
-#endif /* #if defined(CONFIG_NO_HZ_FULL) !defined(CONFIG_NO_HZ_FULL_ALL) */
+
if (ls == -1) {
ls = int_sqrt(nr_cpu_ids);
rcu_nocb_leader_stride = ls;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL rcu/next] RCU commits for 3.17
On 07/16/2014 02:15 PM, Pranith Kumar wrote:
On 07/16/2014 01:24 PM, Paul E. McKenney wrote:
And of course if you don't actually specify a nohz_full= mask, then
tick_nohz_full_mask can be NULL at RCU initialization time, and if it
is also true that CONFIG_NO_HZ_FULL_ALL=n, this condition can persist
forever.
Hi Paul,
The other location where tick_nohz_full_mask is being allocated is in
tick_nohz_init_all(), called from tick_nohz_init(). rcu_init() is called
before
tick_nohz_init() in init/main.c. CONFIG_NO_HZ_FULL_ALL for allocation of the
mask does not take effect when rcu_init() runs.
So if nohz_full command line arg is not specified, tick_nohz_full_mask will
always be NULL when rcu_init() runs. So just checking for
tick_nohz_full_mask
is NULL should be enough I guess.
Yep, that is indeed one of the conditions called out in the commit log
below.
Thanx, Paul
Sorry, I was not clear.
nohz_full= boot time parameter overrides the CONFIG_NO_HZ_FULL_ALL build time
parameter.
The check for !CONFIG_NO_HZ_FULL_ALL will disable masking out the cpus on
which NOHZ is
not enabled using the nohz_full boot time parameter, and hence you will
enable callback
offloading on all CPUs as was previously being done.
So, if I have CONFIG_NO_HZ_FULL_ALL enabled at build time but pass nohz_cpu=1
at boot
time, all the CPUs will have callback offloading enabled.
I think the following looks like what is needed in our case:
The previous patch will fail if CONFIG_NO_HZ_FULL is not defined, this should
work.
Thoughts?
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 00dc411..83f3f58 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2479,10 +2479,10 @@ static void __init rcu_spawn_nocb_kthreads(struct
rcu_state *rsp)
if (rcu_nocb_mask == NULL)
return;
-#if defined(CONFIG_NO_HZ_FULL) !defined(CONFIG_NO_HZ_FULL_ALL)
+#if defined(CONFIG_NO_HZ_FULL)
if (tick_nohz_full_running)
cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
-#endif /* #if defined(CONFIG_NO_HZ_FULL) !defined(CONFIG_NO_HZ_FULL_ALL) */
+#endif /* #if defined(CONFIG_NO_HZ_FULL) */
if (ls == -1) {
ls = int_sqrt(nr_cpu_ids);
rcu_nocb_leader_stride = ls;
--
Pranith
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/1] rcu: Allow user to ovveride RCU_NOCB_CPU_ALL at boot time
A kernel built with RCU_NOCB_CPU_ALL build time option will offload callbacks
from all CPUs. The user cannot override this behavior without recompiling the
kernel with the RCU_NOCB_CPU_ALL option turned off.
This commit allows the user to override the build-time option by using the
rcu_nocbs= boot time option without needing to recompile the kernel.
Please note that this is how NO_HZ_FULL_ALL build time option works and this
commit makes it work similar to that.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
init/Kconfig | 14 +++---
kernel/rcu/tree_plugin.h | 8
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index 41066e4..7d363a4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -768,13 +768,13 @@ config RCU_NOCB_CPU_ALL
bool All CPUs are build_forced no-CBs CPUs
depends on RCU_NOCB_CPU
help
- This option forces all CPUs to be no-CBs CPUs. The rcu_nocbs=
- boot parameter will be ignored. All CPUs' RCU callbacks will
- be executed in the context of per-CPU rcuo kthreads created for
- this purpose. Assuming that the kthreads whose names start with
- rcuo are bound to housekeeping CPUs, this reduces OS jitter
- on the remaining CPUs, but might decrease memory locality during
- RCU-callback invocation, thus potentially degrading throughput.
+ If the user doesn't pass the rcu_nocbs= boot option, force all CPUs
+ to be no-CBs CPUs. All CPUs' RCU callbacks will be executed in the
+ context of per-CPU rcuo kthreads created for this purpose. Assuming
+ that the kthreads whose names start with rcuo are bound to
+ housekeeping CPUs, this reduces OS jitter on the remaining CPUs, but
+ might decrease memory locality during RCU-callback invocation, thus
+ potentially degrading throughput.
Select this if all CPUs need to be no-CBs CPUs for real-time
or energy-efficiency reasons.
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index a2113f6..b97a939 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -92,15 +92,15 @@ static void __init rcu_bootup_announce_oddness(void)
if (!have_rcu_nocb_mask) {
zalloc_cpumask_var(rcu_nocb_mask, GFP_KERNEL);
have_rcu_nocb_mask = true;
+#ifdef CONFIG_RCU_NOCB_CPU_ALL
+ pr_info(\tOffload RCU callbacks from all CPUs\n);
+ cpumask_copy(rcu_nocb_mask, cpu_possible_mask);
+#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ALL */
}
#ifdef CONFIG_RCU_NOCB_CPU_ZERO
pr_info(\tOffload RCU callbacks from CPU 0\n);
cpumask_set_cpu(0, rcu_nocb_mask);
#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ZERO */
-#ifdef CONFIG_RCU_NOCB_CPU_ALL
- pr_info(\tOffload RCU callbacks from all CPUs\n);
- cpumask_copy(rcu_nocb_mask, cpu_possible_mask);
-#endif /* #ifdef CONFIG_RCU_NOCB_CPU_ALL */
#endif /* #ifndef CONFIG_RCU_NOCB_CPU_NONE */
if (have_rcu_nocb_mask) {
if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] doc: Add remote CPU access details and others to this_cpu_ops.txt
Add more details from a recent kernel newbies mailing list discussion here: http://www.spinics.net/lists/newbies/msg52747.html Also list the operations available and add details about safe accesses. v2: * updated with comments from Christoph Lameter Signed-off-by: Pranith Kumar bobby.pr...@gmail.com CC: Christoph Lameter c...@linux.com --- Documentation/this_cpu_ops.txt | 129 ++--- scripts/bin2c | Bin 0 - 8706 bytes 2 files changed, 108 insertions(+), 21 deletions(-) create mode 100755 scripts/bin2c diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..b1faa9d 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -13,22 +13,51 @@ operating on the per cpu variable. This means there are no atomicity issues between the calculation of the offset and the operation on the data. Therefore it is not -necessary to disable preempt or interrupts to ensure that the +necessary to disable preemption or interrupts to ensure that the processor is not changed between the calculation of the address and the operation on the data. Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate -without the typical synchronization overhead but still provide some -sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +without the typical synchronization overhead, but still provide some +sort of relaxed atomicity guarantees. The x86, for example, can execute +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but synchronization is not necessary since we are dealing with per cpu data specific to the currently executing processor. Only the current processor should be accessing that variable and therefore there are no -concurrency issues with other processors in the system. +concurrency issues with other processors in the system. Please see the +section Remote access to per-CPU data if you need remote access. + +The main use of the this_cpu operations has been to optimize counter +operations. + +The following this_cpu() operations with implied preemption protection +are defined. These operations can be used without worrying about +preemption and interrupts. + + this_cpu_add() + this_cpu_read(pcp) + this_cpu_write(pcp, val) + this_cpu_add(pcp, val) + this_cpu_and(pcp, val) + this_cpu_or(pcp, val) + this_cpu_add_return(pcp, val) + this_cpu_xchg(pcp, nval) + this_cpu_cmpxchg(pcp, oval, nval) + this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) + this_cpu_sub(pcp, val) + this_cpu_inc(pcp) + this_cpu_dec(pcp) + this_cpu_sub_return(pcp, val) + this_cpu_inc_return(pcp) + this_cpu_dec_return(pcp) + + +Inner working of this_cpu operations + On x86 the fs: or the gs: segment registers contain the base of the per cpu area. It is then possible to simply use the segment override @@ -53,12 +82,11 @@ this_cpu_ops such sequence also required preempt disable/enable to prevent the kernel from moving the thread to a different processor while the calculation is performed. -The main use of the this_cpu operations has been to optimize counter -operations. +Consider the following this_cpu operation this_cpu_inc(x) -results in the following single instruction (no lock prefix!) +The above results in the following single instruction (no lock prefix!) inc gs:[x] @@ -100,11 +128,10 @@ Takes the offset of a per cpu variable (x !) and returns the address of the per cpu variable that belongs to the currently executing processor. this_cpu_ptr avoids multiple steps that the common get_cpu/put_cpu sequence requires. No processor number is -available. Instead the offset of the local per cpu area is simply +available. Instead, the offset of the local per cpu area is simply added to the percpu offset. - Per cpu variables and offsets - @@ -118,15 +145,16 @@ Therefore the use of x or x outside of the context of per cpu operations is invalid and will generally be treated like a NULL pointer dereference. -In the context of per cpu operations + DEFINE_PER_CPU(int, x); - x is a per cpu variable. Most this_cpu operations take a cpu - variable. +In the context of per cpu operations the above implies that x is a per +cpu variable. Most this_cpu operations take a cpu variable. - x is the *offset* a per cpu variable. this_cpu_ptr() takes - the offset of a per cpu variable
Re: [RFC PATCH 1/1] rcu: Allow user to ovveride RCU_NOCB_CPU_ALL at boot time
On 07/16/2014 08:26 PM, Paul E. McKenney wrote: On Wed, Jul 16, 2014 at 06:38:08PM -0400, Pranith Kumar wrote: A kernel built with RCU_NOCB_CPU_ALL build time option will offload callbacks from all CPUs. The user cannot override this behavior without recompiling the kernel with the RCU_NOCB_CPU_ALL option turned off. This commit allows the user to override the build-time option by using the rcu_nocbs= boot time option without needing to recompile the kernel. Please note that this is how NO_HZ_FULL_ALL build time option works and this commit makes it work similar to that. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com Hi Paul, I cannot accept this patch. For one thing, tick_nohz_init_all() looks a bit on the unconditional side when CONFIG_NO_HZ_FULL_ALL=y. For another thing, we really do not want to be handing the user a tool that allows CPUs that are nohz_full to not be no-CBs CPUs. For another thing, I thought the latest patch does not allow that by ORing the nohz_full and rcu_nocbs mask. Doesn't it? All nohz_full CPUs will be nocb CPUS. We can mention this explicitly in the kernel-parameters.txt file. if we add this and it turns out to be a bad idea, it will be difficult to take it back -- someone somewhere will no doubt have scripted the boot parameter. This option already exists in the kernel when RCU_NOCB_CPU_ALL is not set. The user can pass in rcu_nocbs= at boot time. I am not sure we are adding anything new with this patch. Most of the distro kernels set RCU_NOCB_CPU_ALL and rightly so, as it is the suggested and most appropriate option. This patch will make it easier for users who want to specify nocb CPUs for their needs, without having to recompile the kernel. Thanx, Paul -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 08:55 PM, Lai Jiangshan wrote: On 07/16/2014 09:29 PM, Pranith Kumar wrote: On 07/16/2014 08:47 AM, Paul E. McKenney wrote: On Tue, Jul 15, 2014 at 06:57:59PM -0400, Pranith Kumar wrote: On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote: On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote: This commit removes a stale comment in rcu/tree.c. FYI, an updated comment exists a few lines below this. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com In general, when removing a stale comment, I'd suggest explaining why the comment is stale. Was code removed without removing the corresponding comment, or was code changed such that the comment no longer applies, or...? I guess it was left out when code was moved around previously. And I did mention that an updated comment is there a few lines below. For reference this is the new comment which is below the old comment, they mean the same :) /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Indeed that is the case. Please update the commit log with this explanation and resend. Thanx, Paul Please find the updated patch below. -- Pranith From: Pranith Kumar bobby.pr...@gmail.com Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out when some code was moved around previously. Which commit caused this leftover comment? Could you mention that commit in your changlog? 12BitsCmmtID (commit title...) Sure, please find an updated patch with Josh Triplett's sign-off added: From: Pranith Kumar bobby.pr...@gmail.com Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out in commit 2036d94a7b61ca5032ce (rcu: Rework detection of use of RCU by offline CPUs) For reference, the following updated comment exists a few lines below this which means the same. /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Signed-off-by: Pranith Kumar bobby.pr...@gmail.com Reviewed-by: Joe Tripplett j...@joshtriplett.org --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); - /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(rsp-onoff_mutex); raw_spin_lock_irqsave(rsp-orphan_lock, flags); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 09:25 PM, Lai Jiangshan wrote: On 07/17/2014 09:01 AM, Pranith Kumar wrote: On 07/16/2014 08:55 PM, Lai Jiangshan wrote: On 07/16/2014 09:29 PM, Pranith Kumar wrote: On 07/16/2014 08:47 AM, Paul E. McKenney wrote: On Tue, Jul 15, 2014 at 06:57:59PM -0400, Pranith Kumar wrote: On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote: On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote: This commit removes a stale comment in rcu/tree.c. FYI, an updated comment exists a few lines below this. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com In general, when removing a stale comment, I'd suggest explaining why the comment is stale. Was code removed without removing the corresponding comment, or was code changed such that the comment no longer applies, or...? I guess it was left out when code was moved around previously. And I did mention that an updated comment is there a few lines below. For reference this is the new comment which is below the old comment, they mean the same :) /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Indeed that is the case. Please update the commit log with this explanation and resend. Thanx, Paul Please find the updated patch below. -- Pranith From: Pranith Kumar bobby.pr...@gmail.com Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out when some code was moved around previously. Which commit caused this leftover comment? Could you mention that commit in your changlog? 12BitsCmmtID (commit title...) Sure, please find an updated patch with Josh Triplett's sign-off added: From: Pranith Kumar bobby.pr...@gmail.com Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out in commit 2036d94a7b61ca5032ce (rcu: Rework detection of use of RCU by offline CPUs) I suggest you use the following syntax in future. 2036d94a7b61 (rcu: Rework detection of use of RCU by offline CPUs) OK. I will do that from now on. Thanks! :) -- Pranith For reference, the following updated comment exists a few lines below this which means the same. /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Signed-off-by: Pranith Kumar bobby.pr...@gmail.com Reviewed-by: Joe Tripplett j...@joshtriplett.org Reviewed-by: Lai Jiangshan la...@cn.fujitsu.com --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); -/* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(rsp-onoff_mutex); raw_spin_lock_irqsave(rsp-orphan_lock, flags); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On 07/16/2014 09:14 AM, Paul E. McKenney wrote: On Mon, Jul 14, 2014 at 09:27:00AM -0400, Pranith Kumar wrote: On Sat, Jul 12, 2014 at 8:08 AM, Paul E. McKenney wrote: They ensure that any RCU read-side critical sections that took place before the current (or previous) idle/userspace period on the remote CPU in question are seen as having completed before the completion of the current grace period. It also ensures that any RCU read-side critical sections that extend beyond the end of the current grace period (thus starting after the current (or previous) idle/userspace period) see any updates that were carried out before the beginning of the current grace period. Of course, that is also the purpose of many of RCU's memory barriers, so this probably doesn't help much. An alternative explanation is that it ensures a coherent view of the -dynticks counter, but I am quite sure that this helps even less. So here is the problem we are having: The dyntick_save_progress_counter() and rcu_implicit_dynticks_qs() functions are really bad places to start reading the RCU code. I would say that starting there is like learning to swim by diving into the deep end of a swimming pool, but that doesn't really capture it. Instead, it is more like learning to swim by diving from the top of this waterfall: http://blog.pacificnorthwestphotography.com/wp-content/uploads/2011/03/54.jpg To understand these functions, you will first need to understand how the rest of RCU works. These functions are tiny cogs in RCU's energy efficiency optimization mechanism, which fits into the larger grace-period detection mechanism. The purpose of the two atomic operations is to preserve the memory-ordering guarantees called out in the docbook header comments for call_rcu() and synchronize_rcu(), and I must confess that it is not clear to me that you actually read these header comments. Even so, these two functions interact with lots of other accesses to implement these guarantees -- so again, it is really really difficult to understand these two functions in isolation. Please see the end of this message for my suggested order of learning the RCU code. A study plan, if you will. This guide helps a lot, thank you for the detailed study plan. I will make sure to go slow and steady. :) Best of everything with it! Thanks a lot, Paul, for taking the time to help me understand. Hopefully one of these days, after reading the study list, I actually will :) -- Pranith. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/16/2014 10:14 PM, Josh Triplett wrote: On Wed, Jul 16, 2014 at 09:01:52PM -0400, Pranith Kumar wrote: Sure, please find an updated patch with Josh Triplett's sign-off added: It appears to have a reviewed-by from someone named Joe Tripplett instead. ;) I apologize for fat-fingering this. Since I've sent one too many emails in this thread already, I suppose on more can not do much harm :) This time with Reviewed-by Lai Jiangshan added and Reviewed-by Josh Triplett corrected. From: Pranith Kumar bobby.pr...@gmail.com Date: Mon, 14 Jul 2014 16:01:05 -0400 Subject: [PATCH 2/3] rcu: Remove stale comment in tree.c This commit removes a stale comment in rcu/tree.c which was left out when some code was moved around previously in the commit 2036d94a7b61 (rcu: Rework detection of use of RCU by offline CPUs) For reference, the following updated comment exists a few lines below this which means the same. /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ Signed-off-by: Pranith Kumar bobby.pr...@gmail.com Reviewed-by: Josh Triplett j...@joshtriplett.org Reviewed-by: Lai Jiangshan la...@cn.fujitsu.com --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); - /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(rsp-onoff_mutex); raw_spin_lock_irqsave(rsp-orphan_lock, flags); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC tip/core/rcu 1/2] rcu: Rationalize kthread spawning
On 07/16/2014 10:57 PM, Sasha Levin wrote: On 07/14/2014 06:06 AM, Paul E. McKenney wrote: From: Paul E. McKenney paul...@linux.vnet.ibm.com Currently, RCU spawns kthreads from several different early_initcall() functions. Although this has served RCU well for quite some time, as more kthreads are added a more deterministic approach is required. This commit therefore causes all of RCU's early-boot kthreads to be spawned from a single early_initcall() function. Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com Hey Paul, I've updated to today's -next and my VM started hanging on boot. Bisect pointed out this patch. I don't have any further info right now, but can provide whatever you'll find useful. Hi Sasha, Could you attach the config file which is failing for a start? Thanks, -- Pranith Thanks, Sasha -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] scripts/get_maintainer: increase threshold for --follow to reduce time
On Wed, Jul 16, 2014 at 12:08 AM, Joe Perches wrote: > On Tue, 2014-07-15 at 22:23 -0400, Pranith Kumar wrote: >> ping? >> >> On Sat, Jul 12, 2014 at 3:25 PM, Pranith Kumar wrote: >> > get_maintainer tries to follow files with a matching threshold of default >> > 50%. >> > This is not really necessary as we do not change a file and move it in the >> > same >> > commit usually. Increasing the threshold to 90% should be sufficient. > While I understand the goal, I wonder if this should > be in a .gitconfig, not in get_maintainers at all. > > (git doesn't seem to have a config entry for score though) > > If not, the % should probably be cmd-line configurable. > > The problem is with the default which is at 50% now. That means that a commit needs to have changed a file by more than 50% -and- moved it in the same commit. This is highly discouraged and unlikely to happen in our case. Even if we have this a cmd-line configuration, setting the default to 90% really makes sense, no? -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: Add remote CPU access details to this_cpu_ops.txt
Add more details from a recent kernel newbies mailing list discussion here: http://www.spinics.net/lists/newbies/msg52747.html Signed-off-by: Pranith Kumar CC: Christoph Lameter --- Documentation/this_cpu_ops.txt | 107 + 1 file changed, 87 insertions(+), 20 deletions(-) diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..2381602 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -13,22 +13,53 @@ operating on the per cpu variable. This means there are no atomicity issues between the calculation of the offset and the operation on the data. Therefore it is not -necessary to disable preempt or interrupts to ensure that the +necessary to disable preemption or interrupts to ensure that the processor is not changed between the calculation of the address and the operation on the data. Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate -without the typical synchronization overhead but still provide some -sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +without the typical synchronization overhead, but still provide some +sort of relaxed atomicity guarantees. The x86, for example, can execute +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but synchronization is not necessary since we are dealing with per cpu data specific to the currently executing processor. Only the current processor should be accessing that variable and therefore there are no -concurrency issues with other processors in the system. +concurrency issues with other processors in the system. Please see the +section "Remote access to per-CPU data" if you need remote access. + +The main use of the this_cpu operations has been to optimize counter +operations. + +The following this_cpu() operations with implied preemption protection +are defined. These operations can be used without worrying about +preemption. Note that interrupts may still occur while an operation is +in progress and if the interrupt too modifies the variable, then RMW +actions may not be atomic. + + this_cpu_add() + this_cpu_read(pcp) + this_cpu_write(pcp, val) + this_cpu_add(pcp, val) + this_cpu_and(pcp, val) + this_cpu_or(pcp, val) + this_cpu_add_return(pcp, val) + this_cpu_xchg(pcp, nval) + this_cpu_cmpxchg(pcp, oval, nval) + this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) + this_cpu_sub(pcp, val) + this_cpu_inc(pcp) + this_cpu_dec(pcp) + this_cpu_sub_return(pcp, val) + this_cpu_inc_return(pcp) + this_cpu_dec_return(pcp) + + +Inner working of this_cpu operations + On x86 the fs: or the gs: segment registers contain the base of the per cpu area. It is then possible to simply use the segment override @@ -53,12 +84,11 @@ this_cpu_ops such sequence also required preempt disable/enable to prevent the kernel from moving the thread to a different processor while the calculation is performed. -The main use of the this_cpu operations has been to optimize counter -operations. +Consider the following this_cpu operation this_cpu_inc(x) -results in the following single instruction (no lock prefix!) +The above results in the following single instruction (no lock prefix!) inc gs:[x] @@ -100,11 +130,10 @@ Takes the offset of a per cpu variable ( !) and returns the address of the per cpu variable that belongs to the currently executing processor. this_cpu_ptr avoids multiple steps that the common get_cpu/put_cpu sequence requires. No processor number is -available. Instead the offset of the local per cpu area is simply +available. Instead, the offset of the local per cpu area is simply added to the percpu offset. - Per cpu variables and offsets - @@ -118,15 +147,15 @@ Therefore the use of x or outside of the context of per cpu operations is invalid and will generally be treated like a NULL pointer dereference. -In the context of per cpu operations + DEFINE_PER_CPU(int, x); - x is a per cpu variable. Most this_cpu operations take a cpu - variable. +In the context of per cpu operations the above implies that x is a per +cpu variable. Most this_cpu operations take a cpu variable. -is the *offset* a per cpu variable. this_cpu_ptr() takes - the offset of a per cpu variable which makes this look a bit - strange. + int __percpu *p = + and hence p is the *offset* a per cpu variable. this_cpu_ptr() takes +the offset of a per cpu variable which makes this look a b
Re: [PATCH 1/1] scripts/get_maintainer: increase threshold for --follow to reduce time
ping?
On Sat, Jul 12, 2014 at 3:25 PM, Pranith Kumar wrote:
> get_maintainer tries to follow files with a matching threshold of default 50%.
> This is not really necessary as we do not change a file and move it in the
> same
> commit usually. Increasing the threshold to 90% should be sufficient.
>
> I encountered this on a recent commit where I was touching 12 files. Time take
> dropped from 24 sec to 12 sec after this change. It can be reduced further by
> increasing the threshold, but I think 90% is sufficient.
>
> Signed-off-by: Pranith Kumar
> ---
> scripts/get_maintainer.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index d701627..bc4a69e 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -98,7 +98,7 @@ my %VCS_cmds_git = (
> "execute_cmd" => \_execute_cmd,
> "available" => '(which("git") ne "") && (-e ".git")',
> "find_signers_cmd" =>
> - "git log --no-color --follow --since=\$email_git_since " .
> + "git log --no-color --follow -M90 --since=\$email_git_since " .
> '--numstat --no-merges ' .
> '--format="GitCommit: %H%n' .
> 'GitAuthor: %an <%ae>%n' .
> --
> 1.9.1
>
--
Pranith
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: Remove redundant checks for rcu_scheduler_fully_active
rcu_scheduler_fully_active is set to true early in the boot process.
rcu_prepare_kthreads() is called in two locations. Once after setting the above
flag and the other while hotplugging a CPU from rcu_cpu_notify(). CPU hotplug is
enabled much later by which time the above flag is already set. Hence checking
for this flag is redundant in this function.
The checks in rcu_spawn_one_boost_kthread() and rcu_spawn_all_nocb_kthreads()
are similarly redundant.
This commit removes the redundant checks for this flag from the above
locations.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 17ccb62..a2113f6 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1341,7 +1341,7 @@ static int rcu_spawn_one_boost_kthread(struct rcu_state
*rsp,
if (_preempt_state != rsp)
return 0;
- if (!rcu_scheduler_fully_active || rnp->qsmaskinit == 0)
+ if (rnp->qsmaskinit == 0)
return 0;
rsp->boost = 1;
@@ -1489,8 +1489,7 @@ static void rcu_prepare_kthreads(int cpu)
struct rcu_node *rnp = rdp->mynode;
/* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
- if (rcu_scheduler_fully_active)
- (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
+ (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
}
#else /* #ifdef CONFIG_RCU_BOOST */
@@ -2509,9 +2508,8 @@ static void rcu_spawn_all_nocb_kthreads(int cpu)
{
struct rcu_state *rsp;
- if (rcu_scheduler_fully_active)
- for_each_rcu_flavor(rsp)
- rcu_spawn_one_nocb_kthread(rsp, cpu);
+ for_each_rcu_flavor(rsp)
+ rcu_spawn_one_nocb_kthread(rsp, cpu);
}
/*
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote: > On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote: >> This commit removes a stale comment in rcu/tree.c. >> FYI, an updated comment exists a few lines below this. >> >> Signed-off-by: Pranith Kumar > In general, when removing a stale comment, I'd suggest explaining why > the comment is stale. Was code removed without removing the > corresponding comment, or was code changed such that the comment no > longer applies, or...? I guess it was left out when code was moved around previously. And I did mention that an updated comment is there a few lines below. For reference this is the new comment which is below the old comment, they mean the same :) /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] rcu: Use rcu_num_nodes instead of NUM_RCU_NODES
NUM_RCU_NODES is set at build time and is usually a huge number. We calculate
the
actual number of rcu nodes necessary at boot time based on nr_cpu_ids in
rcu_init_geometry() and store it in rcu_num_nodes. We should use this variable
instead of NUM_RCU_NODES.
This commit changes all such uses of NUM_RCU_NODES to rcu_num_nodes.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..17ccb62 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -46,6 +46,8 @@ static bool __read_mostly rcu_nocb_poll;/* Offload
kthread are to poll. */
static char __initdata nocb_buf[NR_CPUS * 5];
#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
+extern int rcu_num_nodes;
+
/*
* Check the RCU kernel configuration parameters and print informative
* messages about anything out of the ordinary. If you like #ifdef, you
@@ -885,7 +887,7 @@ void synchronize_rcu_expedited(void)
/* Snapshot current state of ->blkd_tasks lists. */
rcu_for_each_leaf_node(rsp, rnp)
sync_rcu_preempt_exp_init(rsp, rnp);
- if (NUM_RCU_NODES > 1)
+ if (rcu_num_nodes > 1)
sync_rcu_preempt_exp_init(rsp, rcu_get_root(rsp));
put_online_cpus();
@@ -1475,7 +1477,7 @@ static void __init rcu_spawn_boost_kthreads(void)
BUG_ON(smpboot_register_percpu_thread(_cpu_thread_spec));
rnp = rcu_get_root(rcu_state_p);
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
- if (NUM_RCU_NODES > 1) {
+ if (rcu_num_nodes > 1) {
rcu_for_each_leaf_node(rcu_state_p, rnp)
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] rcu: Remove stale comment in tree.c
This commit removes a stale comment in rcu/tree.c. FYI, an updated comment exists a few lines below this. Signed-off-by: Pranith Kumar --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); - /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(>onoff_mutex); raw_spin_lock_irqsave(>orphan_lock, flags); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] rcu: tiny.c: Update reference to tree.c
This commit updates the references to rcutree.c which is now rcu/tree.c
Signed-off-by: Pranith Kumar
---
kernel/rcu/tiny.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index d9efcc1..6bd785c 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -51,7 +51,7 @@ static long long rcu_dynticks_nesting =
DYNTICK_TASK_EXIT_IDLE;
#include "tiny_plugin.h"
-/* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcutree.c.
*/
+/* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcu/tree.c.
*/
static void rcu_idle_enter_common(long long newval)
{
if (newval) {
@@ -114,7 +114,7 @@ void rcu_irq_exit(void)
}
EXPORT_SYMBOL_GPL(rcu_irq_exit);
-/* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcutree.c.
*/
+/* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcu/tree.c.
*/
static void rcu_idle_exit_common(long long oldval)
{
if (oldval) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] rcu: Remove stale comment in tree.c
This commit removes a stale comment in rcu/tree.c. FYI, an updated comment exists a few lines below this. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- kernel/rcu/tree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index a1abaa8..e67246e 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2211,8 +2211,6 @@ static void rcu_cleanup_dead_cpu(int cpu, struct rcu_state *rsp) /* Adjust any no-longer-needed kthreads. */ rcu_boost_kthread_setaffinity(rnp, -1); - /* Remove the dead CPU from the bitmasks in the rcu_node hierarchy. */ - /* Exclude any attempts to start a new grace period. */ mutex_lock(rsp-onoff_mutex); raw_spin_lock_irqsave(rsp-orphan_lock, flags); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] rcu: tiny.c: Update reference to tree.c
This commit updates the references to rcutree.c which is now rcu/tree.c
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tiny.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index d9efcc1..6bd785c 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -51,7 +51,7 @@ static long long rcu_dynticks_nesting =
DYNTICK_TASK_EXIT_IDLE;
#include tiny_plugin.h
-/* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcutree.c.
*/
+/* Common code for rcu_idle_enter() and rcu_irq_exit(), see kernel/rcu/tree.c.
*/
static void rcu_idle_enter_common(long long newval)
{
if (newval) {
@@ -114,7 +114,7 @@ void rcu_irq_exit(void)
}
EXPORT_SYMBOL_GPL(rcu_irq_exit);
-/* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcutree.c.
*/
+/* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcu/tree.c.
*/
static void rcu_idle_exit_common(long long oldval)
{
if (oldval) {
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] rcu: Use rcu_num_nodes instead of NUM_RCU_NODES
NUM_RCU_NODES is set at build time and is usually a huge number. We calculate
the
actual number of rcu nodes necessary at boot time based on nr_cpu_ids in
rcu_init_geometry() and store it in rcu_num_nodes. We should use this variable
instead of NUM_RCU_NODES.
This commit changes all such uses of NUM_RCU_NODES to rcu_num_nodes.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree_plugin.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index cedb020..17ccb62 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -46,6 +46,8 @@ static bool __read_mostly rcu_nocb_poll;/* Offload
kthread are to poll. */
static char __initdata nocb_buf[NR_CPUS * 5];
#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
+extern int rcu_num_nodes;
+
/*
* Check the RCU kernel configuration parameters and print informative
* messages about anything out of the ordinary. If you like #ifdef, you
@@ -885,7 +887,7 @@ void synchronize_rcu_expedited(void)
/* Snapshot current state of -blkd_tasks lists. */
rcu_for_each_leaf_node(rsp, rnp)
sync_rcu_preempt_exp_init(rsp, rnp);
- if (NUM_RCU_NODES 1)
+ if (rcu_num_nodes 1)
sync_rcu_preempt_exp_init(rsp, rcu_get_root(rsp));
put_online_cpus();
@@ -1475,7 +1477,7 @@ static void __init rcu_spawn_boost_kthreads(void)
BUG_ON(smpboot_register_percpu_thread(rcu_cpu_thread_spec));
rnp = rcu_get_root(rcu_state_p);
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
- if (NUM_RCU_NODES 1) {
+ if (rcu_num_nodes 1) {
rcu_for_each_leaf_node(rcu_state_p, rnp)
(void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] rcu: Remove stale comment in tree.c
On 07/15/2014 06:53 PM, j...@joshtriplett.org wrote: On Tue, Jul 15, 2014 at 06:31:48PM -0400, Pranith Kumar wrote: This commit removes a stale comment in rcu/tree.c. FYI, an updated comment exists a few lines below this. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com In general, when removing a stale comment, I'd suggest explaining why the comment is stale. Was code removed without removing the corresponding comment, or was code changed such that the comment no longer applies, or...? I guess it was left out when code was moved around previously. And I did mention that an updated comment is there a few lines below. For reference this is the new comment which is below the old comment, they mean the same :) /* Remove the outgoing CPU from the masks in the rcu_node hierarchy. */ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: Remove redundant checks for rcu_scheduler_fully_active
rcu_scheduler_fully_active is set to true early in the boot process.
rcu_prepare_kthreads() is called in two locations. Once after setting the above
flag and the other while hotplugging a CPU from rcu_cpu_notify(). CPU hotplug is
enabled much later by which time the above flag is already set. Hence checking
for this flag is redundant in this function.
The checks in rcu_spawn_one_boost_kthread() and rcu_spawn_all_nocb_kthreads()
are similarly redundant.
This commit removes the redundant checks for this flag from the above
locations.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
kernel/rcu/tree_plugin.h | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 17ccb62..a2113f6 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1341,7 +1341,7 @@ static int rcu_spawn_one_boost_kthread(struct rcu_state
*rsp,
if (rcu_preempt_state != rsp)
return 0;
- if (!rcu_scheduler_fully_active || rnp-qsmaskinit == 0)
+ if (rnp-qsmaskinit == 0)
return 0;
rsp-boost = 1;
@@ -1489,8 +1489,7 @@ static void rcu_prepare_kthreads(int cpu)
struct rcu_node *rnp = rdp-mynode;
/* Fire up the incoming CPU's kthread and leaf rcu_node kthread. */
- if (rcu_scheduler_fully_active)
- (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
+ (void)rcu_spawn_one_boost_kthread(rcu_state_p, rnp);
}
#else /* #ifdef CONFIG_RCU_BOOST */
@@ -2509,9 +2508,8 @@ static void rcu_spawn_all_nocb_kthreads(int cpu)
{
struct rcu_state *rsp;
- if (rcu_scheduler_fully_active)
- for_each_rcu_flavor(rsp)
- rcu_spawn_one_nocb_kthread(rsp, cpu);
+ for_each_rcu_flavor(rsp)
+ rcu_spawn_one_nocb_kthread(rsp, cpu);
}
/*
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] scripts/get_maintainer: increase threshold for --follow to reduce time
ping? On Sat, Jul 12, 2014 at 3:25 PM, Pranith Kumar bobby.pr...@gmail.com wrote: get_maintainer tries to follow files with a matching threshold of default 50%. This is not really necessary as we do not change a file and move it in the same commit usually. Increasing the threshold to 90% should be sufficient. I encountered this on a recent commit where I was touching 12 files. Time take dropped from 24 sec to 12 sec after this change. It can be reduced further by increasing the threshold, but I think 90% is sufficient. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- scripts/get_maintainer.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index d701627..bc4a69e 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -98,7 +98,7 @@ my %VCS_cmds_git = ( execute_cmd = \git_execute_cmd, available = '(which(git) ne ) (-e .git)', find_signers_cmd = - git log --no-color --follow --since=\$email_git_since . + git log --no-color --follow -M90 --since=\$email_git_since . '--numstat --no-merges ' . '--format=GitCommit: %H%n' . 'GitAuthor: %an %ae%n' . -- 1.9.1 -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: Add remote CPU access details to this_cpu_ops.txt
Add more details from a recent kernel newbies mailing list discussion here: http://www.spinics.net/lists/newbies/msg52747.html Signed-off-by: Pranith Kumar bobby.pr...@gmail.com CC: Christoph Lameter c...@linux.com --- Documentation/this_cpu_ops.txt | 107 + 1 file changed, 87 insertions(+), 20 deletions(-) diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..2381602 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -13,22 +13,53 @@ operating on the per cpu variable. This means there are no atomicity issues between the calculation of the offset and the operation on the data. Therefore it is not -necessary to disable preempt or interrupts to ensure that the +necessary to disable preemption or interrupts to ensure that the processor is not changed between the calculation of the address and the operation on the data. Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate -without the typical synchronization overhead but still provide some -sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +without the typical synchronization overhead, but still provide some +sort of relaxed atomicity guarantees. The x86, for example, can execute +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but synchronization is not necessary since we are dealing with per cpu data specific to the currently executing processor. Only the current processor should be accessing that variable and therefore there are no -concurrency issues with other processors in the system. +concurrency issues with other processors in the system. Please see the +section Remote access to per-CPU data if you need remote access. + +The main use of the this_cpu operations has been to optimize counter +operations. + +The following this_cpu() operations with implied preemption protection +are defined. These operations can be used without worrying about +preemption. Note that interrupts may still occur while an operation is +in progress and if the interrupt too modifies the variable, then RMW +actions may not be atomic. + + this_cpu_add() + this_cpu_read(pcp) + this_cpu_write(pcp, val) + this_cpu_add(pcp, val) + this_cpu_and(pcp, val) + this_cpu_or(pcp, val) + this_cpu_add_return(pcp, val) + this_cpu_xchg(pcp, nval) + this_cpu_cmpxchg(pcp, oval, nval) + this_cpu_cmpxchg_double(pcp1, pcp2, oval1, oval2, nval1, nval2) + this_cpu_sub(pcp, val) + this_cpu_inc(pcp) + this_cpu_dec(pcp) + this_cpu_sub_return(pcp, val) + this_cpu_inc_return(pcp) + this_cpu_dec_return(pcp) + + +Inner working of this_cpu operations + On x86 the fs: or the gs: segment registers contain the base of the per cpu area. It is then possible to simply use the segment override @@ -53,12 +84,11 @@ this_cpu_ops such sequence also required preempt disable/enable to prevent the kernel from moving the thread to a different processor while the calculation is performed. -The main use of the this_cpu operations has been to optimize counter -operations. +Consider the following this_cpu operation this_cpu_inc(x) -results in the following single instruction (no lock prefix!) +The above results in the following single instruction (no lock prefix!) inc gs:[x] @@ -100,11 +130,10 @@ Takes the offset of a per cpu variable (x !) and returns the address of the per cpu variable that belongs to the currently executing processor. this_cpu_ptr avoids multiple steps that the common get_cpu/put_cpu sequence requires. No processor number is -available. Instead the offset of the local per cpu area is simply +available. Instead, the offset of the local per cpu area is simply added to the percpu offset. - Per cpu variables and offsets - @@ -118,15 +147,15 @@ Therefore the use of x or x outside of the context of per cpu operations is invalid and will generally be treated like a NULL pointer dereference. -In the context of per cpu operations + DEFINE_PER_CPU(int, x); - x is a per cpu variable. Most this_cpu operations take a cpu - variable. +In the context of per cpu operations the above implies that x is a per +cpu variable. Most this_cpu operations take a cpu variable. - x is the *offset* a per cpu variable. this_cpu_ptr() takes - the offset of a per cpu variable which makes this look a bit - strange. + int __percpu *p = x; +x and hence p is the *offset* a per cpu variable. this_cpu_ptr() takes +the offset of a per cpu variable which makes
Re: [PATCH 1/1] scripts/get_maintainer: increase threshold for --follow to reduce time
On Wed, Jul 16, 2014 at 12:08 AM, Joe Perches j...@perches.com wrote: On Tue, 2014-07-15 at 22:23 -0400, Pranith Kumar wrote: ping? On Sat, Jul 12, 2014 at 3:25 PM, Pranith Kumar bobby.pr...@gmail.com wrote: get_maintainer tries to follow files with a matching threshold of default 50%. This is not really necessary as we do not change a file and move it in the same commit usually. Increasing the threshold to 90% should be sufficient. snip While I understand the goal, I wonder if this should be in a .gitconfig, not in get_maintainers at all. (git doesn't seem to have a config entry for score though) If not, the % should probably be cmd-line configurable. The problem is with the default which is at 50% now. That means that a commit needs to have changed a file by more than 50% -and- moved it in the same commit. This is highly discouraged and unlikely to happen in our case. Even if we have this a cmd-line configuration, setting the default to 90% really makes sense, no? -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: cpu-hotplug.txt: Minor grammar fix
fix minor grammar in sentence Signed-off-by: Pranith Kumar --- Documentation/cpu-hotplug.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt index a0b005d..2ec86f6 100644 --- a/Documentation/cpu-hotplug.txt +++ b/Documentation/cpu-hotplug.txt @@ -383,7 +383,7 @@ A: The following are what is required for CPU hotplug infrastructure to work dead routine is called to be sure positively. Q: I need to ensure that a particular cpu is not removed when there is some - work specific to this cpu is in progress. + work specific to this cpu in progress. A: There are two ways. If your code can be run in interrupt context, use smp_call_function_single(), otherwise use work_on_cpu(). Note that work_on_cpu() is slow, and can fail due to out of memory: -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC tip/core/rcu 2/2] rcu: Create rcuo kthreads only for onlined CPUs
On 07/14/2014 06:06 AM, Paul E. McKenney wrote:
> From: "Paul E. McKenney"
>
> RCU currently uses for_each_possible_cpu() to spawn rcuo kthreads,
> which can result in more rcuo kthreads than one would expect, for
> example, derRichard reported 64 CPUs worth of rcuo kthreads on an
> 8-CPU image. This commit therefore creates rcuo kthreads only for
> those CPUs that actually come online.
>
> Reported-by: derRichard
> Signed-off-by: Paul E. McKenney
> ---
> kernel/rcu/tree.c| 4 ++-
> kernel/rcu/tree.h| 4 ++-
> kernel/rcu/tree_plugin.h | 93
> ++--
> 3 files changed, 88 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index eecdbe20a08e..a1abaa8abd49 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3444,6 +3444,7 @@ static int rcu_cpu_notify(struct notifier_block *self,
> case CPU_UP_PREPARE_FROZEN:
> rcu_prepare_cpu(cpu);
> rcu_prepare_kthreads(cpu);
> + rcu_spawn_all_nocb_kthreads(cpu);
> break;
> case CPU_ONLINE:
> case CPU_DOWN_FAILED:
> @@ -3508,8 +3509,8 @@ static int __init rcu_spawn_gp_kthread(void)
> raw_spin_lock_irqsave(>lock, flags);
> rsp->gp_kthread = t;
> raw_spin_unlock_irqrestore(>lock, flags);
> - rcu_spawn_nocb_kthreads(rsp);
> }
> + rcu_spawn_nocb_kthreads();
> rcu_spawn_boost_kthreads();
> return 0;
> }
> @@ -3643,6 +3644,7 @@ static void __init rcu_init_one(struct rcu_state *rsp,
> rcu_boot_init_percpu_data(i, rsp);
> }
> list_add(>flavors, _struct_flavors);
> + rcu_organize_nocb_kthreads(rsp);
> }
>
> /*
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index bd7b63da9a8c..f703ea8b7836 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -593,7 +593,9 @@ static bool rcu_nocb_adopt_orphan_cbs(struct rcu_state
> *rsp,
> static bool rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp);
> static void do_nocb_deferred_wakeup(struct rcu_data *rdp);
> static void rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp);
> -static void rcu_spawn_nocb_kthreads(struct rcu_state *rsp);
> +static void rcu_spawn_all_nocb_kthreads(int cpu);
> +static void __init rcu_spawn_nocb_kthreads(void);
> +static void __init rcu_organize_nocb_kthreads(struct rcu_state *rsp);
> static void __maybe_unused rcu_kick_nohz_cpu(int cpu);
> static bool init_nocb_callback_list(struct rcu_data *rdp);
> static void rcu_sysidle_enter(struct rcu_dynticks *rdtp, int irq);
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index f5314cc20750..cedb020f1f90 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -2455,15 +2455,85 @@ static void __init
> rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
> rdp->nocb_follower_tail = >nocb_follower_head;
> }
>
> +/*
> + * If the specified CPU is a no-CBs CPU that does not already have its
> + * rcuo kthread for the specified RCU flavor, spawn it. If the CPUs are
> + * brought online out of order, this can require require re-organizing
double require, can remove one
> + * the leader-follower relationships.
> + */
> +static void rcu_spawn_one_nocb_kthread(struct rcu_state *rsp, int cpu)
> +{
> + struct rcu_data *rdp;
> + struct rcu_data *rdp_last;
> + struct rcu_data *rdp_old_leader;
> + struct rcu_data *rdp_spawn = per_cpu_ptr(rsp->rda, cpu);
> + struct task_struct *t;
> +
> + /*
> + * If this isn't a no-CBs CPU or if it already has an rcuo kthread,
> + * then nothing to do.
> + */
> + if (!rcu_is_nocb_cpu(cpu) || rdp_spawn->nocb_kthread)
> + return;
> +
> + /* If we didn't spawn the leader first, reorganize! */
> + rdp_old_leader = rdp_spawn->nocb_leader;
> + if (rdp_old_leader != rdp_spawn && !rdp_old_leader->nocb_kthread) {
> + rdp_last = NULL;
> + rdp = rdp_old_leader;
> + do {
> + rdp->nocb_leader = rdp_spawn;
> + if (rdp_last && rdp != rdp_spawn)
> + rdp_last->nocb_next_follower = rdp;
> + rdp_last = rdp;
> + rdp = rdp->nocb_next_follower;
> + rdp_last->nocb_next_follower = NULL;
> + } while (rdp);
> + rdp_spawn->nocb_next_follower = rdp_old_leader;
> + }
> +
> + /* Spawn the kthread for this CPU and RCU flavor. */
> + t = kthread_run(rcu_nocb_kthread, rdp_spawn,
> + "rcuo%c/%d", rsp->abbr, cpu);
> + BUG_ON(IS_ERR(t));
> + ACCESS_ONCE(rdp_spawn->nocb_kthread) = t;
> +}
> +
> +/*
> + * If the specified CPU is a no-CBs CPU that does not already have its
> + * rcuo kthreads, spawn them.
> + */
> +static void rcu_spawn_all_nocb_kthreads(int cpu)
> +{
> + struct rcu_state *rsp;
> +
> + if
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Sat, Jul 12, 2014 at 8:08 AM, Paul E. McKenney wrote: > > They ensure that any RCU read-side critical sections that took place before > the current (or previous) idle/userspace period on the remote CPU in > question are seen as having completed before the completion of the current > grace period. It also ensures that any RCU read-side critical sections > that extend beyond the end of the current grace period (thus starting > after the current (or previous) idle/userspace period) see any updates > that were carried out before the beginning of the current grace period. > > Of course, that is also the purpose of many of RCU's memory barriers, > so this probably doesn't help much. An alternative explanation is that > it ensures a coherent view of the ->dynticks counter, but I am quite > sure that this helps even less. > > So here is the problem we are having: > > The dyntick_save_progress_counter() and rcu_implicit_dynticks_qs() > functions are really bad places to start reading the RCU code. I would > say that starting there is like learning to swim by diving into the deep > end of a swimming pool, but that doesn't really capture it. Instead, > it is more like learning to swim by diving from the top of this waterfall: > > http://blog.pacificnorthwestphotography.com/wp-content/uploads/2011/03/54.jpg > > To understand these functions, you will first need to understand how > the rest of RCU works. These functions are tiny cogs in RCU's energy > efficiency optimization mechanism, which fits into the larger grace-period > detection mechanism. The purpose of the two atomic operations is to > preserve the memory-ordering guarantees called out in the docbook header > comments for call_rcu() and synchronize_rcu(), and I must confess that > it is not clear to me that you actually read these header comments. > Even so, these two functions interact with lots of other accesses to > implement these guarantees -- so again, it is really really difficult > to understand these two functions in isolation. > > Please see the end of this message for my suggested order of learning > the RCU code. A study plan, if you will. > This guide helps a lot, thank you for the detailed study plan. I will make sure to go slow and steady. :) I believe my question was about a local issue, let me try to explain. My question stems from my understanding of why barriers are needed: (i) to prevent compiler re-ordering of memory accesses (ii) to ensure a partial ordering of memory accesses (global visibility) Barriers are also costly and hence must be used sparingly, if at all. I understand the need to use a barrier before/after an update to a shared variable. And using a barrier before a read access to a shared variable makes sense, as it ensures that we order this read with a previous write from the same CPU, if any. The question here is this: why do we need a barrier after a read access to a shared variable? The only reason I could think of is reason (i) above, but looking at the code, we can see that the read cannot really float around. Did I miss something or is there something fundamentally wrong with my thinking? Please note that all updates to dynticks are already strongly ordered as the updates are wrapped with barriers both before and after. So _reading_ the dynticks variable on other CPUs should be consistent i.e., an update should be immediately visible. -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] rcutorture: fixes for printing message buffer
On 07/11/2014 04:37 PM, Joe Perches wrote:
> On Fri, 2014-07-11 at 16:30 -0400, Pranith Kumar wrote:
>> Use snprintf() instead of sprintf() for writing to the message buffer.
>> Also use vmalloc() for the allocation of the message buffer. Since
>> pr_alert() is
>> limited to print LOG_LINE_MAX characters at a time, we print the buffer in a
>> loop one line at a time.
>>
>> I tested this using the parse-torture.sh script as follows:
>
> Did you see the patch I sent you?
> https://lkml.org/lkml/2014/6/20/604
>
> It doesn't need a vmalloc.
> What is wrong with that approach?
>
>
Hi Paul,
Please find an alternative patch as suggested by Joe.
>From 0a3050d5ac910cb5f488e0a0dfe44cde06af1259 Mon Sep 17 00:00:00 2001
From: Pranith Kumar
Date: Mon, 14 Jul 2014 09:16:15 -0400
Subject: [PATCH 1/1] rcu: Use pr_alert/pr_cont for printing logs
User pr_alert/pr_cont for printing the logs from rcutorture module directly
instead of writing it to a buffer and then printing it. This allows us from not
having to allocate such buffers. Also remove a resulting empty function.
I tested this using the parse-torture.sh script as follows:
$ dmesg | grep torture > log.txt
$ bash parse-torture.sh log.txt test
$
There were no warnings which means that parsing went fine.
Signed-off-by: Joe Percesh
Signed-off-by: Pranith Kumar
---
include/linux/torture.h | 2 +-
kernel/rcu/rcutorture.c | 127 +---
kernel/torture.c| 16 +++---
3 files changed, 64 insertions(+), 81 deletions(-)
diff --git a/include/linux/torture.h b/include/linux/torture.h
index 5ca58fc..fec46f8 100644
--- a/include/linux/torture.h
+++ b/include/linux/torture.h
@@ -51,7 +51,7 @@
/* Definitions for online/offline exerciser. */
int torture_onoff_init(long ooholdoff, long oointerval);
-char *torture_onoff_stats(char *page);
+void torture_onoff_stats(void);
bool torture_onoff_failures(void);
/* Low-rider random number generator. */
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 5ec0452..3e35f1b 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -242,7 +242,7 @@ struct rcu_torture_ops {
void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
void (*cb_barrier)(void);
void (*fqs)(void);
- void (*stats)(char *page);
+ void (*stats)(void);
int irq_capable;
int can_boost;
const char *name;
@@ -525,21 +525,21 @@ static void srcu_torture_barrier(void)
srcu_barrier(_ctl);
}
-static void srcu_torture_stats(char *page)
+static void srcu_torture_stats(void)
{
int cpu;
int idx = srcu_ctl.completed & 0x1;
- page += sprintf(page, "%s%s per-CPU(idx=%d):",
- torture_type, TORTURE_FLAG, idx);
+ pr_alert("%s%s per-CPU(idx=%d):",
+torture_type, TORTURE_FLAG, idx);
for_each_possible_cpu(cpu) {
long c0, c1;
c0 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)->c[!idx];
c1 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)->c[idx];
- page += sprintf(page, " %d(%ld,%ld)", cpu, c0, c1);
+ pr_cont(" %d(%ld,%ld)", cpu, c0, c1);
}
- sprintf(page, "\n");
+ pr_cont("\n");
}
static void srcu_torture_synchronize_expedited(void)
@@ -1031,10 +1031,15 @@ rcu_torture_reader(void *arg)
}
/*
- * Create an RCU-torture statistics message in the specified buffer.
+ * Print torture statistics. Caller must ensure that there is only
+ * one call to this function at a given time!!! This is normally
+ * accomplished by relying on the module system to only have one copy
+ * of the module loaded, and then by giving the rcu_torture_stats
+ * kthread full control (or the init/cleanup functions when rcu_torture_stats
+ * thread is not running).
*/
static void
-rcu_torture_printk(char *page)
+rcu_torture_stats_print(void)
{
int cpu;
int i;
@@ -1052,55 +1057,60 @@ rcu_torture_printk(char *page)
if (pipesummary[i] != 0)
break;
}
- page += sprintf(page, "%s%s ", torture_type, TORTURE_FLAG);
- page += sprintf(page,
- "rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ",
- rcu_torture_current,
- rcu_torture_current_version,
- list_empty(_torture_freelist),
- atomic_read(_rcu_torture_alloc),
- atomic_read(_rcu_torture_alloc_fail),
- atomic_read(_rcu_torture_free));
- page += sprintf(page, "rtmbe: %d rtbke: %ld rtbre: %ld ",
- atomic_read(_rcu_torture_mberror),
- n_rcu_torture_boost
Re: [PATCH 1/1] rcutorture: fixes for printing message buffer
On 07/11/2014 04:37 PM, Joe Perches wrote:
On Fri, 2014-07-11 at 16:30 -0400, Pranith Kumar wrote:
Use snprintf() instead of sprintf() for writing to the message buffer.
Also use vmalloc() for the allocation of the message buffer. Since
pr_alert() is
limited to print LOG_LINE_MAX characters at a time, we print the buffer in a
loop one line at a time.
I tested this using the parse-torture.sh script as follows:
Did you see the patch I sent you?
https://lkml.org/lkml/2014/6/20/604
It doesn't need a vmalloc.
What is wrong with that approach?
Hi Paul,
Please find an alternative patch as suggested by Joe.
From 0a3050d5ac910cb5f488e0a0dfe44cde06af1259 Mon Sep 17 00:00:00 2001
From: Pranith Kumar bobby.pr...@gmail.com
Date: Mon, 14 Jul 2014 09:16:15 -0400
Subject: [PATCH 1/1] rcu: Use pr_alert/pr_cont for printing logs
User pr_alert/pr_cont for printing the logs from rcutorture module directly
instead of writing it to a buffer and then printing it. This allows us from not
having to allocate such buffers. Also remove a resulting empty function.
I tested this using the parse-torture.sh script as follows:
$ dmesg | grep torture log.txt
$ bash parse-torture.sh log.txt test
$
There were no warnings which means that parsing went fine.
Signed-off-by: Joe Percesh j...@perches.com
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
---
include/linux/torture.h | 2 +-
kernel/rcu/rcutorture.c | 127 +---
kernel/torture.c| 16 +++---
3 files changed, 64 insertions(+), 81 deletions(-)
diff --git a/include/linux/torture.h b/include/linux/torture.h
index 5ca58fc..fec46f8 100644
--- a/include/linux/torture.h
+++ b/include/linux/torture.h
@@ -51,7 +51,7 @@
/* Definitions for online/offline exerciser. */
int torture_onoff_init(long ooholdoff, long oointerval);
-char *torture_onoff_stats(char *page);
+void torture_onoff_stats(void);
bool torture_onoff_failures(void);
/* Low-rider random number generator. */
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 5ec0452..3e35f1b 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -242,7 +242,7 @@ struct rcu_torture_ops {
void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
void (*cb_barrier)(void);
void (*fqs)(void);
- void (*stats)(char *page);
+ void (*stats)(void);
int irq_capable;
int can_boost;
const char *name;
@@ -525,21 +525,21 @@ static void srcu_torture_barrier(void)
srcu_barrier(srcu_ctl);
}
-static void srcu_torture_stats(char *page)
+static void srcu_torture_stats(void)
{
int cpu;
int idx = srcu_ctl.completed 0x1;
- page += sprintf(page, %s%s per-CPU(idx=%d):,
- torture_type, TORTURE_FLAG, idx);
+ pr_alert(%s%s per-CPU(idx=%d):,
+torture_type, TORTURE_FLAG, idx);
for_each_possible_cpu(cpu) {
long c0, c1;
c0 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)-c[!idx];
c1 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)-c[idx];
- page += sprintf(page, %d(%ld,%ld), cpu, c0, c1);
+ pr_cont( %d(%ld,%ld), cpu, c0, c1);
}
- sprintf(page, \n);
+ pr_cont(\n);
}
static void srcu_torture_synchronize_expedited(void)
@@ -1031,10 +1031,15 @@ rcu_torture_reader(void *arg)
}
/*
- * Create an RCU-torture statistics message in the specified buffer.
+ * Print torture statistics. Caller must ensure that there is only
+ * one call to this function at a given time!!! This is normally
+ * accomplished by relying on the module system to only have one copy
+ * of the module loaded, and then by giving the rcu_torture_stats
+ * kthread full control (or the init/cleanup functions when rcu_torture_stats
+ * thread is not running).
*/
static void
-rcu_torture_printk(char *page)
+rcu_torture_stats_print(void)
{
int cpu;
int i;
@@ -1052,55 +1057,60 @@ rcu_torture_printk(char *page)
if (pipesummary[i] != 0)
break;
}
- page += sprintf(page, %s%s , torture_type, TORTURE_FLAG);
- page += sprintf(page,
- 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));
- page += sprintf(page, rtmbe: %d rtbke: %ld rtbre: %ld ,
- atomic_read(n_rcu_torture_mberror),
- n_rcu_torture_boost_ktrerror,
- n_rcu_torture_boost_rterror);
- page += sprintf(page, rtbf: %ld rtb: %ld nt: %ld ,
- n_rcu_torture_boost_failure
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Sat, Jul 12, 2014 at 8:08 AM, Paul E. McKenney wrote: They ensure that any RCU read-side critical sections that took place before the current (or previous) idle/userspace period on the remote CPU in question are seen as having completed before the completion of the current grace period. It also ensures that any RCU read-side critical sections that extend beyond the end of the current grace period (thus starting after the current (or previous) idle/userspace period) see any updates that were carried out before the beginning of the current grace period. Of course, that is also the purpose of many of RCU's memory barriers, so this probably doesn't help much. An alternative explanation is that it ensures a coherent view of the -dynticks counter, but I am quite sure that this helps even less. So here is the problem we are having: The dyntick_save_progress_counter() and rcu_implicit_dynticks_qs() functions are really bad places to start reading the RCU code. I would say that starting there is like learning to swim by diving into the deep end of a swimming pool, but that doesn't really capture it. Instead, it is more like learning to swim by diving from the top of this waterfall: http://blog.pacificnorthwestphotography.com/wp-content/uploads/2011/03/54.jpg To understand these functions, you will first need to understand how the rest of RCU works. These functions are tiny cogs in RCU's energy efficiency optimization mechanism, which fits into the larger grace-period detection mechanism. The purpose of the two atomic operations is to preserve the memory-ordering guarantees called out in the docbook header comments for call_rcu() and synchronize_rcu(), and I must confess that it is not clear to me that you actually read these header comments. Even so, these two functions interact with lots of other accesses to implement these guarantees -- so again, it is really really difficult to understand these two functions in isolation. Please see the end of this message for my suggested order of learning the RCU code. A study plan, if you will. This guide helps a lot, thank you for the detailed study plan. I will make sure to go slow and steady. :) I believe my question was about a local issue, let me try to explain. My question stems from my understanding of why barriers are needed: (i) to prevent compiler re-ordering of memory accesses (ii) to ensure a partial ordering of memory accesses (global visibility) Barriers are also costly and hence must be used sparingly, if at all. I understand the need to use a barrier before/after an update to a shared variable. And using a barrier before a read access to a shared variable makes sense, as it ensures that we order this read with a previous write from the same CPU, if any. The question here is this: why do we need a barrier after a read access to a shared variable? The only reason I could think of is reason (i) above, but looking at the code, we can see that the read cannot really float around. Did I miss something or is there something fundamentally wrong with my thinking? Please note that all updates to dynticks are already strongly ordered as the updates are wrapped with barriers both before and after. So _reading_ the dynticks variable on other CPUs should be consistent i.e., an update should be immediately visible. -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC tip/core/rcu 2/2] rcu: Create rcuo kthreads only for onlined CPUs
On 07/14/2014 06:06 AM, Paul E. McKenney wrote:
From: Paul E. McKenney paul...@linux.vnet.ibm.com
RCU currently uses for_each_possible_cpu() to spawn rcuo kthreads,
which can result in more rcuo kthreads than one would expect, for
example, derRichard reported 64 CPUs worth of rcuo kthreads on an
8-CPU image. This commit therefore creates rcuo kthreads only for
those CPUs that actually come online.
Reported-by: derRichard
Signed-off-by: Paul E. McKenney paul...@linux.vnet.ibm.com
---
kernel/rcu/tree.c| 4 ++-
kernel/rcu/tree.h| 4 ++-
kernel/rcu/tree_plugin.h | 93
++--
3 files changed, 88 insertions(+), 13 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index eecdbe20a08e..a1abaa8abd49 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3444,6 +3444,7 @@ static int rcu_cpu_notify(struct notifier_block *self,
case CPU_UP_PREPARE_FROZEN:
rcu_prepare_cpu(cpu);
rcu_prepare_kthreads(cpu);
+ rcu_spawn_all_nocb_kthreads(cpu);
break;
case CPU_ONLINE:
case CPU_DOWN_FAILED:
@@ -3508,8 +3509,8 @@ static int __init rcu_spawn_gp_kthread(void)
raw_spin_lock_irqsave(rnp-lock, flags);
rsp-gp_kthread = t;
raw_spin_unlock_irqrestore(rnp-lock, flags);
- rcu_spawn_nocb_kthreads(rsp);
}
+ rcu_spawn_nocb_kthreads();
rcu_spawn_boost_kthreads();
return 0;
}
@@ -3643,6 +3644,7 @@ static void __init rcu_init_one(struct rcu_state *rsp,
rcu_boot_init_percpu_data(i, rsp);
}
list_add(rsp-flavors, rcu_struct_flavors);
+ rcu_organize_nocb_kthreads(rsp);
}
/*
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index bd7b63da9a8c..f703ea8b7836 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -593,7 +593,9 @@ static bool rcu_nocb_adopt_orphan_cbs(struct rcu_state
*rsp,
static bool rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp);
static void do_nocb_deferred_wakeup(struct rcu_data *rdp);
static void rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp);
-static void rcu_spawn_nocb_kthreads(struct rcu_state *rsp);
+static void rcu_spawn_all_nocb_kthreads(int cpu);
+static void __init rcu_spawn_nocb_kthreads(void);
+static void __init rcu_organize_nocb_kthreads(struct rcu_state *rsp);
static void __maybe_unused rcu_kick_nohz_cpu(int cpu);
static bool init_nocb_callback_list(struct rcu_data *rdp);
static void rcu_sysidle_enter(struct rcu_dynticks *rdtp, int irq);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index f5314cc20750..cedb020f1f90 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2455,15 +2455,85 @@ static void __init
rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
rdp-nocb_follower_tail = rdp-nocb_follower_head;
}
+/*
+ * If the specified CPU is a no-CBs CPU that does not already have its
+ * rcuo kthread for the specified RCU flavor, spawn it. If the CPUs are
+ * brought online out of order, this can require require re-organizing
double require, can remove one
+ * the leader-follower relationships.
+ */
+static void rcu_spawn_one_nocb_kthread(struct rcu_state *rsp, int cpu)
+{
+ struct rcu_data *rdp;
+ struct rcu_data *rdp_last;
+ struct rcu_data *rdp_old_leader;
+ struct rcu_data *rdp_spawn = per_cpu_ptr(rsp-rda, cpu);
+ struct task_struct *t;
+
+ /*
+ * If this isn't a no-CBs CPU or if it already has an rcuo kthread,
+ * then nothing to do.
+ */
+ if (!rcu_is_nocb_cpu(cpu) || rdp_spawn-nocb_kthread)
+ return;
+
+ /* If we didn't spawn the leader first, reorganize! */
+ rdp_old_leader = rdp_spawn-nocb_leader;
+ if (rdp_old_leader != rdp_spawn !rdp_old_leader-nocb_kthread) {
+ rdp_last = NULL;
+ rdp = rdp_old_leader;
+ do {
+ rdp-nocb_leader = rdp_spawn;
+ if (rdp_last rdp != rdp_spawn)
+ rdp_last-nocb_next_follower = rdp;
+ rdp_last = rdp;
+ rdp = rdp-nocb_next_follower;
+ rdp_last-nocb_next_follower = NULL;
+ } while (rdp);
+ rdp_spawn-nocb_next_follower = rdp_old_leader;
+ }
+
+ /* Spawn the kthread for this CPU and RCU flavor. */
+ t = kthread_run(rcu_nocb_kthread, rdp_spawn,
+ rcuo%c/%d, rsp-abbr, cpu);
+ BUG_ON(IS_ERR(t));
+ ACCESS_ONCE(rdp_spawn-nocb_kthread) = t;
+}
+
+/*
+ * If the specified CPU is a no-CBs CPU that does not already have its
+ * rcuo kthreads, spawn them.
+ */
+static void rcu_spawn_all_nocb_kthreads(int cpu)
+{
+ struct rcu_state *rsp;
+
+ if (rcu_scheduler_fully_active)
+ for_each_rcu_flavor(rsp)
+
[PATCH 1/1] doc: cpu-hotplug.txt: Minor grammar fix
fix minor grammar in sentence Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- Documentation/cpu-hotplug.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt index a0b005d..2ec86f6 100644 --- a/Documentation/cpu-hotplug.txt +++ b/Documentation/cpu-hotplug.txt @@ -383,7 +383,7 @@ A: The following are what is required for CPU hotplug infrastructure to work dead routine is called to be sure positively. Q: I need to ensure that a particular cpu is not removed when there is some - work specific to this cpu is in progress. + work specific to this cpu in progress. A: There are two ways. If your code can be run in interrupt context, use smp_call_function_single(), otherwise use work_on_cpu(). Note that work_on_cpu() is slow, and can fail due to out of memory: -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: fix docbook generation
kernel/timer.c and kernel/hrtimer.c have moved to kernel/time directory. This causes docbook generation to fail. The following commit fixes it by updating the references. Signed-off-by: Pranith Kumar --- Documentation/DocBook/device-drivers.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index dd3f278..f213058 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -54,7 +54,7 @@ !Ikernel/sched/cpupri.c !Ikernel/sched/fair.c !Iinclude/linux/completion.h -!Ekernel/timer.c +!Ekernel/time/timer.c Wait queues and Wake events !Iinclude/linux/wait.h @@ -63,7 +63,7 @@ High-resolution timers !Iinclude/linux/ktime.h !Iinclude/linux/hrtimer.h -!Ekernel/hrtimer.c +!Ekernel/time/hrtimer.c Workqueues and Kevents !Ekernel/workqueue.c -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] scripts/get_maintainer: increase threshold for --follow to reduce time
get_maintainer tries to follow files with a matching threshold of default 50%.
This is not really necessary as we do not change a file and move it in the same
commit usually. Increasing the threshold to 90% should be sufficient.
I encountered this on a recent commit where I was touching 12 files. Time take
dropped from 24 sec to 12 sec after this change. It can be reduced further by
increasing the threshold, but I think 90% is sufficient.
Signed-off-by: Pranith Kumar
---
scripts/get_maintainer.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index d701627..bc4a69e 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -98,7 +98,7 @@ my %VCS_cmds_git = (
"execute_cmd" => \_execute_cmd,
"available" => '(which("git") ne "") && (-e ".git")',
"find_signers_cmd" =>
- "git log --no-color --follow --since=\$email_git_since " .
+ "git log --no-color --follow -M90 --since=\$email_git_since " .
'--numstat --no-merges ' .
'--format="GitCommit: %H%n' .
'GitAuthor: %an <%ae>%n' .
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] scripts/get_maintainer: increase threshold for --follow to reduce time
get_maintainer tries to follow files with a matching threshold of default 50%. This is not really necessary as we do not change a file and move it in the same commit usually. Increasing the threshold to 90% should be sufficient. I encountered this on a recent commit where I was touching 12 files. Time take dropped from 24 sec to 12 sec after this change. It can be reduced further by increasing the threshold, but I think 90% is sufficient. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- scripts/get_maintainer.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index d701627..bc4a69e 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -98,7 +98,7 @@ my %VCS_cmds_git = ( execute_cmd = \git_execute_cmd, available = '(which(git) ne ) (-e .git)', find_signers_cmd = - git log --no-color --follow --since=\$email_git_since . + git log --no-color --follow -M90 --since=\$email_git_since . '--numstat --no-merges ' . '--format=GitCommit: %H%n' . 'GitAuthor: %an %ae%n' . -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: fix docbook generation
kernel/timer.c and kernel/hrtimer.c have moved to kernel/time directory. This causes docbook generation to fail. The following commit fixes it by updating the references. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- Documentation/DocBook/device-drivers.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/device-drivers.tmpl b/Documentation/DocBook/device-drivers.tmpl index dd3f278..f213058 100644 --- a/Documentation/DocBook/device-drivers.tmpl +++ b/Documentation/DocBook/device-drivers.tmpl @@ -54,7 +54,7 @@ !Ikernel/sched/cpupri.c !Ikernel/sched/fair.c !Iinclude/linux/completion.h -!Ekernel/timer.c +!Ekernel/time/timer.c /sect1 sect1titleWait queues and Wake events/title !Iinclude/linux/wait.h @@ -63,7 +63,7 @@ sect1titleHigh-resolution timers/title !Iinclude/linux/ktime.h !Iinclude/linux/hrtimer.h -!Ekernel/hrtimer.c +!Ekernel/time/hrtimer.c /sect1 sect1titleWorkqueues and Kevents/title !Ekernel/workqueue.c -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcutorture: set executable bit and drop bash from Usage
This patch is on top of the previous changes. Set the executable bit on test scripts config2frag.sh and kvm.sh Since #!/bin/bash is set in all the scripts, drop it from the usage line as the scripts can be invoked directly. Signed-off-by: Pranith Kumar --- tools/testing/selftests/rcutorture/bin/config2frag.sh | 2 +- tools/testing/selftests/rcutorture/bin/configcheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/configinit.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-build.sh| 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm.sh | 2 +- tools/testing/selftests/rcutorture/bin/parse-build.sh | 3 +-- tools/testing/selftests/rcutorture/bin/parse-console.sh| 3 +-- tools/testing/selftests/rcutorture/bin/parse-torture.sh| 3 +-- 12 files changed, 12 insertions(+), 15 deletions(-) mode change 100644 => 100755 tools/testing/selftests/rcutorture/bin/config2frag.sh mode change 100644 => 100755 tools/testing/selftests/rcutorture/bin/kvm.sh diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh b/tools/testing/selftests/rcutorture/bin/config2frag.sh old mode 100644 new mode 100755 index 4e394ef..56f51ae --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh +++ b/tools/testing/selftests/rcutorture/bin/config2frag.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Usage: bash config2frag.sh < .config > configfrag +# Usage: config2frag.sh < .config > configfrag # # Converts the "# CONFIG_XXX is not set" to "CONFIG_XXX=n" so that the # resulting file becomes a legitimate Kconfig fragment. diff --git a/tools/testing/selftests/rcutorture/bin/configcheck.sh b/tools/testing/selftests/rcutorture/bin/configcheck.sh index 6173ed5..eee31e2 100755 --- a/tools/testing/selftests/rcutorture/bin/configcheck.sh +++ b/tools/testing/selftests/rcutorture/bin/configcheck.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Usage: bash configcheck.sh .config .config-template +# Usage: configcheck.sh .config .config-template # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh b/tools/testing/selftests/rcutorture/bin/configinit.sh index d8f7418..15f1a17 100755 --- a/tools/testing/selftests/rcutorture/bin/configinit.sh +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh @@ -1,6 +1,6 @@ #!/bin/bash # -# bash configinit.sh config-spec-file [ build output dir ] +# Usage: configinit.sh config-spec-file [ build output dir ] # # Create a .config file from the spec file. Run from the kernel source tree. # Exits with 0 if all went well, with 1 if all went well but the config diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh b/tools/testing/selftests/rcutorture/bin/kvm-build.sh index e4bfb91..00cb0db 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh @@ -2,7 +2,7 @@ # # Build a kvm-ready Linux kernel from the tree in the current directory. # -# Usage: bash kvm-build.sh config-template build-dir more-configs +# Usage: kvm-build.sh config-template build-dir more-configs # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh index 30cbb63..43f7640 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for locktorture progress. # -# Usage: bash kvm-recheck-lock.sh resdir +# Usage: kvm-recheck-lock.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 6e94a5e..d6cc07f 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for rcutorture progress. # -# Usage: bash kvm-recheck-rcu.sh resdir +# Usage: kvm-recheck-rcu.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh index 3482b
Re: [PATCH 1/1] rcutorture: use bash shell for all the test scripts
On Fri, Jul 11, 2014 at 7:23 PM, wrote: > On Fri, Jul 11, 2014 at 05:31:27PM -0400, Pranith Kumar wrote: >> Some of the scripts encode a default /bin/sh shell. On systems which use >> dash as >> default shell, these scripts fail as they are bash scripts. I encountered >> this >> while testing the sprintf() changes on a Debian system where dash is the >> default >> shell. >> >> This commit changes all such uses to use bash explicitly. > > Good catch. > > Since these all have #! lines, can you please set the executable bit on > any that don't have it set (looks like just config2frag.sh and kvm.sh), > and then change the Usage lines to drop the shell entirely and just > invoke the program directly? OK. I will send that as a separate patch. > >> Signed-off-by: Pranith Kumar > > With the above changed (perhaps in a separate patch): > Reviewed-by: Josh Triplett > >> tools/testing/selftests/rcutorture/bin/config2frag.sh | 4 ++-- >> tools/testing/selftests/rcutorture/bin/configcheck.sh | 4 ++-- >> tools/testing/selftests/rcutorture/bin/configinit.sh | 4 ++-- >> tools/testing/selftests/rcutorture/bin/kvm-build.sh| 2 +- >> tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- >> tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- >> tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- >> tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- >> tools/testing/selftests/rcutorture/bin/kvm.sh | 2 +- >> tools/testing/selftests/rcutorture/bin/parse-build.sh | 4 ++-- >> tools/testing/selftests/rcutorture/bin/parse-console.sh| 4 ++-- >> tools/testing/selftests/rcutorture/bin/parse-torture.sh| 4 ++-- >> 12 files changed, 18 insertions(+), 18 deletions(-) >> >> diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh >> b/tools/testing/selftests/rcutorture/bin/config2frag.sh >> index 9f9ffcd..4e394ef 100644 >> --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh >> +++ b/tools/testing/selftests/rcutorture/bin/config2frag.sh >> @@ -1,5 +1,5 @@ >> -#!/bin/sh >> -# Usage: sh config2frag.sh < .config > configfrag >> +#!/bin/bash >> +# Usage: bash config2frag.sh < .config > configfrag >> # >> # Converts the "# CONFIG_XXX is not set" to "CONFIG_XXX=n" so that the >> # resulting file becomes a legitimate Kconfig fragment. >> diff --git a/tools/testing/selftests/rcutorture/bin/configcheck.sh >> b/tools/testing/selftests/rcutorture/bin/configcheck.sh >> index d686537..6173ed5 100755 >> --- a/tools/testing/selftests/rcutorture/bin/configcheck.sh >> +++ b/tools/testing/selftests/rcutorture/bin/configcheck.sh >> @@ -1,5 +1,5 @@ >> -#!/bin/sh >> -# Usage: sh configcheck.sh .config .config-template >> +#!/bin/bash >> +# Usage: bash configcheck.sh .config .config-template >> # >> # This program is free software; you can redistribute it and/or modify >> # it under the terms of the GNU General Public License as published by >> diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh >> b/tools/testing/selftests/rcutorture/bin/configinit.sh >> index 9c3f3d3..d8f7418 100755 >> --- a/tools/testing/selftests/rcutorture/bin/configinit.sh >> +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh >> @@ -1,6 +1,6 @@ >> -#!/bin/sh >> +#!/bin/bash >> # >> -# sh configinit.sh config-spec-file [ build output dir ] >> +# bash configinit.sh config-spec-file [ build output dir ] >> # >> # Create a .config file from the spec file. Run from the kernel source >> tree. >> # Exits with 0 if all went well, with 1 if all went well but the config >> diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh >> b/tools/testing/selftests/rcutorture/bin/kvm-build.sh >> index 7c1e56b..e4bfb91 100755 >> --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh >> +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh >> @@ -2,7 +2,7 @@ >> # >> # Build a kvm-ready Linux kernel from the tree in the current directory. >> # >> -# Usage: sh kvm-build.sh config-template build-dir more-configs >> +# Usage: bash kvm-build.sh config-template build-dir more-configs >> # >> # This program is free software; you can redistribute it and/or modify >> # it under the terms of the GNU General Public License as published by >> diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh >> b/tools/testing/selftests/rcutorture/bin/kvm-recheck-
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Fri, Jul 11, 2014 at 5:43 AM, Paul E. McKenney wrote: > On Thu, Jul 10, 2014 at 09:17:33PM -0400, Pranith Kumar wrote: >> On Wed, Jul 9, 2014 at 3:56 PM, Paul E. McKenney >> wrote: >> >> > OK, so ->dynticks_snap is accessed by only one task, namely the >> > corresponding RCU grace-period kthread. So it can be accessed without >> > any atomic instructions or memory barriers, since all accesses to it are >> > single-threaded. On the other hand, ->dynticks is written by one CPU >> > and potentially accessed from any CPU. Therefore, accesses to it must >> > take concurrency into account. Especially given that any confusion can >> > fool RCU into thinking that a CPU is idle when it really is not, which >> > could result in too-short grace periods, which could in turn result in >> > random memory corruption. >> >> Yes, I missed reading the call-chain for accessing dynticks_snap. It >> does not need any synchronization/barriers. >> >> Here since we are reading ->dynticks, doesn't having one barrier >> before reading make sense? like >> >> smp_mb(); >> dynticks_snap = atomic_read(...->dynticks); >> >> instead of having two barriers with atomic_add_return()? i.e., >> why is the second barrier necessary? > > I suggest looking at the docbook comment headers for call_rcu() and > synchronize_rcu() and thinking about the memory-barrier guarantees. > Those guarantees are quite strong, and so if you remove any one of a > large number of memory barriers (either explicit or, as in this case, > implicit in some other operation), you will break RCU. > > Now, there might well be some redundant memory barriers somewhere in > RCU, but the second memory barrier implied by this atomic_add_return() > most certainly is not one of them. One reason I could think of having barriers on both sides here is to disable the read to float around. But in these two particular cases that does not seem to be necessary. Could you please clarify why a barrier after this atomic read is required? Almost all the barriers are commented about why they are necessary. Would be good to have that here too. >> >> Sorry to ask you about such an old change. But I am not able to see >> why we need atomic_t for dynticks here since per-cpu operations are >> guaranteed to be atomic. > > Per-CPU operations are guaranteed to be atomic? When one CPU is accessing > another CPU's per-CPU variable, as is the case here? Can't say that I > believe you. ;-) this_cpu_ops() are guaranteed to be atomic when operating on local per-cpu variables. When we are operating on other CPU's per-cpu variables directly this does not hold. dynticks here is a per-cpu variable. I don't understand why one CPU needs to access another CPU's dynticks variable. > >> It gets twisted pretty fast trying to understand the RCU code. No >> wonder people say that rcu is scary black magic :) > > Well, let's just say that this isn't one of the parts of the RCU code > that should be randomly hacked. Lots and lots of ways to get it wrong, > and very few ways to get it right. And most of the ways of getting it > right are too slow or too non-scalable to be useful. I am definitely not trying to hack randomly. Reading the code is very educating. I tried looking up why this was being done and it was not clear from the code and history. I was thinking of getting hold of you on IRC, but was not sure if that is such a good idea. I'll ask questions instead of sending RFCs from now on. > > Speaking of portions of RCU that are more susceptible to local-knowledge > hacking, how are things going on that rcutorture printing fixup? > > Thanx, Paul > It must have reached your inbox by now :) -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcutorture: use bash shell for all the test scripts
Some of the scripts encode a default /bin/sh shell. On systems which use dash as default shell, these scripts fail as they are bash scripts. I encountered this while testing the sprintf() changes on a Debian system where dash is the default shell. This commit changes all such uses to use bash explicitly. Signed-off-by: Pranith Kumar --- tools/testing/selftests/rcutorture/bin/config2frag.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/configcheck.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/configinit.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/kvm-build.sh| 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm.sh | 2 +- tools/testing/selftests/rcutorture/bin/parse-build.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/parse-console.sh| 4 ++-- tools/testing/selftests/rcutorture/bin/parse-torture.sh| 4 ++-- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh b/tools/testing/selftests/rcutorture/bin/config2frag.sh index 9f9ffcd..4e394ef 100644 --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh +++ b/tools/testing/selftests/rcutorture/bin/config2frag.sh @@ -1,5 +1,5 @@ -#!/bin/sh -# Usage: sh config2frag.sh < .config > configfrag +#!/bin/bash +# Usage: bash config2frag.sh < .config > configfrag # # Converts the "# CONFIG_XXX is not set" to "CONFIG_XXX=n" so that the # resulting file becomes a legitimate Kconfig fragment. diff --git a/tools/testing/selftests/rcutorture/bin/configcheck.sh b/tools/testing/selftests/rcutorture/bin/configcheck.sh index d686537..6173ed5 100755 --- a/tools/testing/selftests/rcutorture/bin/configcheck.sh +++ b/tools/testing/selftests/rcutorture/bin/configcheck.sh @@ -1,5 +1,5 @@ -#!/bin/sh -# Usage: sh configcheck.sh .config .config-template +#!/bin/bash +# Usage: bash configcheck.sh .config .config-template # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh b/tools/testing/selftests/rcutorture/bin/configinit.sh index 9c3f3d3..d8f7418 100755 --- a/tools/testing/selftests/rcutorture/bin/configinit.sh +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh @@ -1,6 +1,6 @@ -#!/bin/sh +#!/bin/bash # -# sh configinit.sh config-spec-file [ build output dir ] +# bash configinit.sh config-spec-file [ build output dir ] # # Create a .config file from the spec file. Run from the kernel source tree. # Exits with 0 if all went well, with 1 if all went well but the config diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh b/tools/testing/selftests/rcutorture/bin/kvm-build.sh index 7c1e56b..e4bfb91 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh @@ -2,7 +2,7 @@ # # Build a kvm-ready Linux kernel from the tree in the current directory. # -# Usage: sh kvm-build.sh config-template build-dir more-configs +# Usage: bash kvm-build.sh config-template build-dir more-configs # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh index 7f1ff1a..30cbb63 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for locktorture progress. # -# Usage: sh kvm-recheck-lock.sh resdir +# Usage: bash kvm-recheck-lock.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 307c4b9..6e94a5e 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for rcutorture progress. # -# Usage: sh kvm-recheck-rcu.sh resdir +# Usage: bash kvm-recheck-rcu.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh index 3f6c9b7..3482b3f 100755 --- a/tools/testing/selftests/rcutortur
Re: [PATCH 1/1] rcutorture: fixes for printing message buffer
On Fri, Jul 11, 2014 at 4:37 PM, Joe Perches wrote: > On Fri, 2014-07-11 at 16:30 -0400, Pranith Kumar wrote: >> Use snprintf() instead of sprintf() for writing to the message buffer. >> Also use vmalloc() for the allocation of the message buffer. Since >> pr_alert() is >> limited to print LOG_LINE_MAX characters at a time, we print the buffer in a >> loop one line at a time. >> >> I tested this using the parse-torture.sh script as follows: > > Did you see the patch I sent you? > https://lkml.org/lkml/2014/6/20/604 > > It doesn't need a vmalloc. > What is wrong with that approach? > > I was trying to stay as close to the original code as possible by using a buffer instead of using pr_alert/pr_cont throughout the code. I see nothing wrong with what you proposed, but I like having one pr_alert instead of lots sprinkled. Also avoiding vmalloc/vfree using your approach seems like a win. I will let Paul decide. -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcutorture: fixes for printing message buffer
Use snprintf() instead of sprintf() for writing to the message buffer.
Also use vmalloc() for the allocation of the message buffer. Since pr_alert() is
limited to print LOG_LINE_MAX characters at a time, we print the buffer in a
loop one line at a time.
I tested this using the parse-torture.sh script as follows:
$ dmesg | grep torture > log.txt
$ bash parse-torture.sh log.txt test
$
There were no warnings which means that parsing went fine.
Signed-off-by: Pranith Kumar
cc: Joe Perches
Link: https://lkml.org/lkml/2014/6/18/604
---
include/linux/torture.h | 2 +-
kernel/rcu/rcutorture.c | 95 +++--
kernel/torture.c| 7 ++--
3 files changed, 66 insertions(+), 38 deletions(-)
diff --git a/include/linux/torture.h b/include/linux/torture.h
index 5ca58fc..1e47ec7 100644
--- a/include/linux/torture.h
+++ b/include/linux/torture.h
@@ -51,7 +51,7 @@
/* Definitions for online/offline exerciser. */
int torture_onoff_init(long ooholdoff, long oointerval);
-char *torture_onoff_stats(char *page);
+int torture_onoff_stats(char *page, int size);
bool torture_onoff_failures(void);
/* Low-rider random number generator. */
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 5ec0452..ab2cfbd 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -242,7 +242,7 @@ struct rcu_torture_ops {
void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
void (*cb_barrier)(void);
void (*fqs)(void);
- void (*stats)(char *page);
+ int (*stats)(char *page, int size);
int irq_capable;
int can_boost;
const char *name;
@@ -525,21 +525,23 @@ static void srcu_torture_barrier(void)
srcu_barrier(_ctl);
}
-static void srcu_torture_stats(char *page)
+static int srcu_torture_stats(char *page, int size)
{
- int cpu;
+ int cpu, used;
int idx = srcu_ctl.completed & 0x1;
- page += sprintf(page, "%s%s per-CPU(idx=%d):",
+ used = snprintf(page, size, "%s%s per-CPU(idx=%d):",
torture_type, TORTURE_FLAG, idx);
for_each_possible_cpu(cpu) {
long c0, c1;
c0 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)->c[!idx];
c1 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)->c[idx];
- page += sprintf(page, " %d(%ld,%ld)", cpu, c0, c1);
+ used += snprintf(page + used, size - used,
+ " %d(%ld,%ld)", cpu, c0, c1);
}
- sprintf(page, "\n");
+ used += snprintf(page + used, size - used, "\n");
+ return used;
}
static void srcu_torture_synchronize_expedited(void)
@@ -1033,10 +1035,10 @@ rcu_torture_reader(void *arg)
/*
* Create an RCU-torture statistics message in the specified buffer.
*/
-static void
-rcu_torture_printk(char *page)
+static int
+rcu_torture_printk(char *page, int size)
{
- int cpu;
+ int cpu, used = 0;
int i;
long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
long batchsummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
@@ -1052,8 +1054,9 @@ rcu_torture_printk(char *page)
if (pipesummary[i] != 0)
break;
}
- page += sprintf(page, "%s%s ", torture_type, TORTURE_FLAG);
- page += sprintf(page,
+ used += snprintf(page + used, size - used,
+ "%s%s ", torture_type, TORTURE_FLAG);
+ used += snprintf(page + used, size - used,
"rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ",
rcu_torture_current,
rcu_torture_current_version,
@@ -1061,46 +1064,54 @@ rcu_torture_printk(char *page)
atomic_read(_rcu_torture_alloc),
atomic_read(_rcu_torture_alloc_fail),
atomic_read(_rcu_torture_free));
- page += sprintf(page, "rtmbe: %d rtbke: %ld rtbre: %ld ",
+ used += snprintf(page + used, size - used,
+ "rtmbe: %d rtbke: %ld rtbre: %ld ",
atomic_read(_rcu_torture_mberror),
n_rcu_torture_boost_ktrerror,
n_rcu_torture_boost_rterror);
- page += sprintf(page, "rtbf: %ld rtb: %ld nt: %ld ",
+ used += snprintf(page + used, size - used,
+ "rtbf: %ld rtb: %ld nt: %ld ",
n_rcu_torture_boost_failure,
n_rcu_torture_boosts,
n_rcu_torture_timers);
- page = torture_onoff_stats(page);
- page += sprintf(page, "barrier: %ld/%ld:%ld",
+ used += torture_onoff_stats(page + used, size - used);
+ used += snprintf(page + used, size - used,
+ "barrier: %ld/%ld:
[PATCH 1/1] rcutorture: fixes for printing message buffer
Use snprintf() instead of sprintf() for writing to the message buffer.
Also use vmalloc() for the allocation of the message buffer. Since pr_alert() is
limited to print LOG_LINE_MAX characters at a time, we print the buffer in a
loop one line at a time.
I tested this using the parse-torture.sh script as follows:
$ dmesg | grep torture log.txt
$ bash parse-torture.sh log.txt test
$
There were no warnings which means that parsing went fine.
Signed-off-by: Pranith Kumar bobby.pr...@gmail.com
cc: Joe Perches j...@perches.com
Link: https://lkml.org/lkml/2014/6/18/604
---
include/linux/torture.h | 2 +-
kernel/rcu/rcutorture.c | 95 +++--
kernel/torture.c| 7 ++--
3 files changed, 66 insertions(+), 38 deletions(-)
diff --git a/include/linux/torture.h b/include/linux/torture.h
index 5ca58fc..1e47ec7 100644
--- a/include/linux/torture.h
+++ b/include/linux/torture.h
@@ -51,7 +51,7 @@
/* Definitions for online/offline exerciser. */
int torture_onoff_init(long ooholdoff, long oointerval);
-char *torture_onoff_stats(char *page);
+int torture_onoff_stats(char *page, int size);
bool torture_onoff_failures(void);
/* Low-rider random number generator. */
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 5ec0452..ab2cfbd 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -242,7 +242,7 @@ struct rcu_torture_ops {
void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
void (*cb_barrier)(void);
void (*fqs)(void);
- void (*stats)(char *page);
+ int (*stats)(char *page, int size);
int irq_capable;
int can_boost;
const char *name;
@@ -525,21 +525,23 @@ static void srcu_torture_barrier(void)
srcu_barrier(srcu_ctl);
}
-static void srcu_torture_stats(char *page)
+static int srcu_torture_stats(char *page, int size)
{
- int cpu;
+ int cpu, used;
int idx = srcu_ctl.completed 0x1;
- page += sprintf(page, %s%s per-CPU(idx=%d):,
+ used = snprintf(page, size, %s%s per-CPU(idx=%d):,
torture_type, TORTURE_FLAG, idx);
for_each_possible_cpu(cpu) {
long c0, c1;
c0 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)-c[!idx];
c1 = (long)per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)-c[idx];
- page += sprintf(page, %d(%ld,%ld), cpu, c0, c1);
+ used += snprintf(page + used, size - used,
+%d(%ld,%ld), cpu, c0, c1);
}
- sprintf(page, \n);
+ used += snprintf(page + used, size - used, \n);
+ return used;
}
static void srcu_torture_synchronize_expedited(void)
@@ -1033,10 +1035,10 @@ rcu_torture_reader(void *arg)
/*
* Create an RCU-torture statistics message in the specified buffer.
*/
-static void
-rcu_torture_printk(char *page)
+static int
+rcu_torture_printk(char *page, int size)
{
- int cpu;
+ int cpu, used = 0;
int i;
long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
long batchsummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
@@ -1052,8 +1054,9 @@ rcu_torture_printk(char *page)
if (pipesummary[i] != 0)
break;
}
- page += sprintf(page, %s%s , torture_type, TORTURE_FLAG);
- page += sprintf(page,
+ used += snprintf(page + used, size - used,
+ %s%s , torture_type, TORTURE_FLAG);
+ used += snprintf(page + used, size - used,
rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ,
rcu_torture_current,
rcu_torture_current_version,
@@ -1061,46 +1064,54 @@ rcu_torture_printk(char *page)
atomic_read(n_rcu_torture_alloc),
atomic_read(n_rcu_torture_alloc_fail),
atomic_read(n_rcu_torture_free));
- page += sprintf(page, rtmbe: %d rtbke: %ld rtbre: %ld ,
+ used += snprintf(page + used, size - used,
+ rtmbe: %d rtbke: %ld rtbre: %ld ,
atomic_read(n_rcu_torture_mberror),
n_rcu_torture_boost_ktrerror,
n_rcu_torture_boost_rterror);
- page += sprintf(page, rtbf: %ld rtb: %ld nt: %ld ,
+ used += snprintf(page + used, size - used,
+ rtbf: %ld rtb: %ld nt: %ld ,
n_rcu_torture_boost_failure,
n_rcu_torture_boosts,
n_rcu_torture_timers);
- page = torture_onoff_stats(page);
- page += sprintf(page, barrier: %ld/%ld:%ld,
+ used += torture_onoff_stats(page + used, size - used);
+ used += snprintf(page + used, size - used,
+ barrier: %ld/%ld:%ld,
n_barrier_successes,
n_barrier_attempts,
n_rcu_torture_barrier_error
Re: [PATCH 1/1] rcutorture: fixes for printing message buffer
On Fri, Jul 11, 2014 at 4:37 PM, Joe Perches j...@perches.com wrote: On Fri, 2014-07-11 at 16:30 -0400, Pranith Kumar wrote: Use snprintf() instead of sprintf() for writing to the message buffer. Also use vmalloc() for the allocation of the message buffer. Since pr_alert() is limited to print LOG_LINE_MAX characters at a time, we print the buffer in a loop one line at a time. I tested this using the parse-torture.sh script as follows: Did you see the patch I sent you? https://lkml.org/lkml/2014/6/20/604 It doesn't need a vmalloc. What is wrong with that approach? I was trying to stay as close to the original code as possible by using a buffer instead of using pr_alert/pr_cont throughout the code. I see nothing wrong with what you proposed, but I like having one pr_alert instead of lots sprinkled. Also avoiding vmalloc/vfree using your approach seems like a win. I will let Paul decide. -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcutorture: use bash shell for all the test scripts
Some of the scripts encode a default /bin/sh shell. On systems which use dash as default shell, these scripts fail as they are bash scripts. I encountered this while testing the sprintf() changes on a Debian system where dash is the default shell. This commit changes all such uses to use bash explicitly. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- tools/testing/selftests/rcutorture/bin/config2frag.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/configcheck.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/configinit.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/kvm-build.sh| 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm.sh | 2 +- tools/testing/selftests/rcutorture/bin/parse-build.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/parse-console.sh| 4 ++-- tools/testing/selftests/rcutorture/bin/parse-torture.sh| 4 ++-- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh b/tools/testing/selftests/rcutorture/bin/config2frag.sh index 9f9ffcd..4e394ef 100644 --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh +++ b/tools/testing/selftests/rcutorture/bin/config2frag.sh @@ -1,5 +1,5 @@ -#!/bin/sh -# Usage: sh config2frag.sh .config configfrag +#!/bin/bash +# Usage: bash config2frag.sh .config configfrag # # Converts the # CONFIG_XXX is not set to CONFIG_XXX=n so that the # resulting file becomes a legitimate Kconfig fragment. diff --git a/tools/testing/selftests/rcutorture/bin/configcheck.sh b/tools/testing/selftests/rcutorture/bin/configcheck.sh index d686537..6173ed5 100755 --- a/tools/testing/selftests/rcutorture/bin/configcheck.sh +++ b/tools/testing/selftests/rcutorture/bin/configcheck.sh @@ -1,5 +1,5 @@ -#!/bin/sh -# Usage: sh configcheck.sh .config .config-template +#!/bin/bash +# Usage: bash configcheck.sh .config .config-template # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh b/tools/testing/selftests/rcutorture/bin/configinit.sh index 9c3f3d3..d8f7418 100755 --- a/tools/testing/selftests/rcutorture/bin/configinit.sh +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh @@ -1,6 +1,6 @@ -#!/bin/sh +#!/bin/bash # -# sh configinit.sh config-spec-file [ build output dir ] +# bash configinit.sh config-spec-file [ build output dir ] # # Create a .config file from the spec file. Run from the kernel source tree. # Exits with 0 if all went well, with 1 if all went well but the config diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh b/tools/testing/selftests/rcutorture/bin/kvm-build.sh index 7c1e56b..e4bfb91 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh @@ -2,7 +2,7 @@ # # Build a kvm-ready Linux kernel from the tree in the current directory. # -# Usage: sh kvm-build.sh config-template build-dir more-configs +# Usage: bash kvm-build.sh config-template build-dir more-configs # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh index 7f1ff1a..30cbb63 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for locktorture progress. # -# Usage: sh kvm-recheck-lock.sh resdir +# Usage: bash kvm-recheck-lock.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 307c4b9..6e94a5e 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for rcutorture progress. # -# Usage: sh kvm-recheck-rcu.sh resdir +# Usage: bash kvm-recheck-rcu.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh index 3f6c9b7..3482b3f 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Fri, Jul 11, 2014 at 5:43 AM, Paul E. McKenney wrote: On Thu, Jul 10, 2014 at 09:17:33PM -0400, Pranith Kumar wrote: On Wed, Jul 9, 2014 at 3:56 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: snip OK, so -dynticks_snap is accessed by only one task, namely the corresponding RCU grace-period kthread. So it can be accessed without any atomic instructions or memory barriers, since all accesses to it are single-threaded. On the other hand, -dynticks is written by one CPU and potentially accessed from any CPU. Therefore, accesses to it must take concurrency into account. Especially given that any confusion can fool RCU into thinking that a CPU is idle when it really is not, which could result in too-short grace periods, which could in turn result in random memory corruption. Yes, I missed reading the call-chain for accessing dynticks_snap. It does not need any synchronization/barriers. Here since we are reading -dynticks, doesn't having one barrier before reading make sense? like smp_mb(); dynticks_snap = atomic_read(...-dynticks); instead of having two barriers with atomic_add_return()? i.e., why is the second barrier necessary? I suggest looking at the docbook comment headers for call_rcu() and synchronize_rcu() and thinking about the memory-barrier guarantees. Those guarantees are quite strong, and so if you remove any one of a large number of memory barriers (either explicit or, as in this case, implicit in some other operation), you will break RCU. Now, there might well be some redundant memory barriers somewhere in RCU, but the second memory barrier implied by this atomic_add_return() most certainly is not one of them. One reason I could think of having barriers on both sides here is to disable the read to float around. But in these two particular cases that does not seem to be necessary. Could you please clarify why a barrier after this atomic read is required? Almost all the barriers are commented about why they are necessary. Would be good to have that here too. snip Sorry to ask you about such an old change. But I am not able to see why we need atomic_t for dynticks here since per-cpu operations are guaranteed to be atomic. Per-CPU operations are guaranteed to be atomic? When one CPU is accessing another CPU's per-CPU variable, as is the case here? Can't say that I believe you. ;-) this_cpu_ops() are guaranteed to be atomic when operating on local per-cpu variables. When we are operating on other CPU's per-cpu variables directly this does not hold. dynticks here is a per-cpu variable. I don't understand why one CPU needs to access another CPU's dynticks variable. It gets twisted pretty fast trying to understand the RCU code. No wonder people say that rcu is scary black magic :) Well, let's just say that this isn't one of the parts of the RCU code that should be randomly hacked. Lots and lots of ways to get it wrong, and very few ways to get it right. And most of the ways of getting it right are too slow or too non-scalable to be useful. I am definitely not trying to hack randomly. Reading the code is very educating. I tried looking up why this was being done and it was not clear from the code and history. I was thinking of getting hold of you on IRC, but was not sure if that is such a good idea. I'll ask questions instead of sending RFCs from now on. Speaking of portions of RCU that are more susceptible to local-knowledge hacking, how are things going on that rcutorture printing fixup? Thanx, Paul It must have reached your inbox by now :) -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] rcutorture: use bash shell for all the test scripts
On Fri, Jul 11, 2014 at 7:23 PM, j...@joshtriplett.org wrote: On Fri, Jul 11, 2014 at 05:31:27PM -0400, Pranith Kumar wrote: Some of the scripts encode a default /bin/sh shell. On systems which use dash as default shell, these scripts fail as they are bash scripts. I encountered this while testing the sprintf() changes on a Debian system where dash is the default shell. This commit changes all such uses to use bash explicitly. Good catch. Since these all have #! lines, can you please set the executable bit on any that don't have it set (looks like just config2frag.sh and kvm.sh), and then change the Usage lines to drop the shell entirely and just invoke the program directly? OK. I will send that as a separate patch. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com With the above changed (perhaps in a separate patch): Reviewed-by: Josh Triplett j...@joshtriplett.org tools/testing/selftests/rcutorture/bin/config2frag.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/configcheck.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/configinit.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/kvm-build.sh| 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm.sh | 2 +- tools/testing/selftests/rcutorture/bin/parse-build.sh | 4 ++-- tools/testing/selftests/rcutorture/bin/parse-console.sh| 4 ++-- tools/testing/selftests/rcutorture/bin/parse-torture.sh| 4 ++-- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh b/tools/testing/selftests/rcutorture/bin/config2frag.sh index 9f9ffcd..4e394ef 100644 --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh +++ b/tools/testing/selftests/rcutorture/bin/config2frag.sh @@ -1,5 +1,5 @@ -#!/bin/sh -# Usage: sh config2frag.sh .config configfrag +#!/bin/bash +# Usage: bash config2frag.sh .config configfrag # # Converts the # CONFIG_XXX is not set to CONFIG_XXX=n so that the # resulting file becomes a legitimate Kconfig fragment. diff --git a/tools/testing/selftests/rcutorture/bin/configcheck.sh b/tools/testing/selftests/rcutorture/bin/configcheck.sh index d686537..6173ed5 100755 --- a/tools/testing/selftests/rcutorture/bin/configcheck.sh +++ b/tools/testing/selftests/rcutorture/bin/configcheck.sh @@ -1,5 +1,5 @@ -#!/bin/sh -# Usage: sh configcheck.sh .config .config-template +#!/bin/bash +# Usage: bash configcheck.sh .config .config-template # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh b/tools/testing/selftests/rcutorture/bin/configinit.sh index 9c3f3d3..d8f7418 100755 --- a/tools/testing/selftests/rcutorture/bin/configinit.sh +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh @@ -1,6 +1,6 @@ -#!/bin/sh +#!/bin/bash # -# sh configinit.sh config-spec-file [ build output dir ] +# bash configinit.sh config-spec-file [ build output dir ] # # Create a .config file from the spec file. Run from the kernel source tree. # Exits with 0 if all went well, with 1 if all went well but the config diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh b/tools/testing/selftests/rcutorture/bin/kvm-build.sh index 7c1e56b..e4bfb91 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh @@ -2,7 +2,7 @@ # # Build a kvm-ready Linux kernel from the tree in the current directory. # -# Usage: sh kvm-build.sh config-template build-dir more-configs +# Usage: bash kvm-build.sh config-template build-dir more-configs # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh index 7f1ff1a..30cbb63 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for locktorture progress. # -# Usage: sh kvm-recheck-lock.sh resdir +# Usage: bash kvm-recheck-lock.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 307c4b9..6e94a5e 100755 --- a/tools/testing
[PATCH 1/1] rcutorture: set executable bit and drop bash from Usage
This patch is on top of the previous changes. Set the executable bit on test scripts config2frag.sh and kvm.sh Since #!/bin/bash is set in all the scripts, drop it from the usage line as the scripts can be invoked directly. Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- tools/testing/selftests/rcutorture/bin/config2frag.sh | 2 +- tools/testing/selftests/rcutorture/bin/configcheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/configinit.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-build.sh| 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-recheck.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh | 2 +- tools/testing/selftests/rcutorture/bin/kvm.sh | 2 +- tools/testing/selftests/rcutorture/bin/parse-build.sh | 3 +-- tools/testing/selftests/rcutorture/bin/parse-console.sh| 3 +-- tools/testing/selftests/rcutorture/bin/parse-torture.sh| 3 +-- 12 files changed, 12 insertions(+), 15 deletions(-) mode change 100644 = 100755 tools/testing/selftests/rcutorture/bin/config2frag.sh mode change 100644 = 100755 tools/testing/selftests/rcutorture/bin/kvm.sh diff --git a/tools/testing/selftests/rcutorture/bin/config2frag.sh b/tools/testing/selftests/rcutorture/bin/config2frag.sh old mode 100644 new mode 100755 index 4e394ef..56f51ae --- a/tools/testing/selftests/rcutorture/bin/config2frag.sh +++ b/tools/testing/selftests/rcutorture/bin/config2frag.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Usage: bash config2frag.sh .config configfrag +# Usage: config2frag.sh .config configfrag # # Converts the # CONFIG_XXX is not set to CONFIG_XXX=n so that the # resulting file becomes a legitimate Kconfig fragment. diff --git a/tools/testing/selftests/rcutorture/bin/configcheck.sh b/tools/testing/selftests/rcutorture/bin/configcheck.sh index 6173ed5..eee31e2 100755 --- a/tools/testing/selftests/rcutorture/bin/configcheck.sh +++ b/tools/testing/selftests/rcutorture/bin/configcheck.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Usage: bash configcheck.sh .config .config-template +# Usage: configcheck.sh .config .config-template # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/configinit.sh b/tools/testing/selftests/rcutorture/bin/configinit.sh index d8f7418..15f1a17 100755 --- a/tools/testing/selftests/rcutorture/bin/configinit.sh +++ b/tools/testing/selftests/rcutorture/bin/configinit.sh @@ -1,6 +1,6 @@ #!/bin/bash # -# bash configinit.sh config-spec-file [ build output dir ] +# Usage: configinit.sh config-spec-file [ build output dir ] # # Create a .config file from the spec file. Run from the kernel source tree. # Exits with 0 if all went well, with 1 if all went well but the config diff --git a/tools/testing/selftests/rcutorture/bin/kvm-build.sh b/tools/testing/selftests/rcutorture/bin/kvm-build.sh index e4bfb91..00cb0db 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-build.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-build.sh @@ -2,7 +2,7 @@ # # Build a kvm-ready Linux kernel from the tree in the current directory. # -# Usage: bash kvm-build.sh config-template build-dir more-configs +# Usage: kvm-build.sh config-template build-dir more-configs # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh index 30cbb63..43f7640 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for locktorture progress. # -# Usage: bash kvm-recheck-lock.sh resdir +# Usage: kvm-recheck-lock.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh index 6e94a5e..d6cc07f 100755 --- a/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh +++ b/tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh @@ -2,7 +2,7 @@ # # Analyze a given results directory for rcutorture progress. # -# Usage: bash kvm-recheck-rcu.sh resdir +# Usage: kvm-recheck-rcu.sh resdir # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by diff --git a/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh b/tools/testing/selftests/rcutorture/bin/kvm-recheck.sh index 3482b3f..4f5b20f 100755 --- a/tools
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Wed, Jul 9, 2014 at 3:56 PM, Paul E. McKenney wrote: > OK, so ->dynticks_snap is accessed by only one task, namely the > corresponding RCU grace-period kthread. So it can be accessed without > any atomic instructions or memory barriers, since all accesses to it are > single-threaded. On the other hand, ->dynticks is written by one CPU > and potentially accessed from any CPU. Therefore, accesses to it must > take concurrency into account. Especially given that any confusion can > fool RCU into thinking that a CPU is idle when it really is not, which > could result in too-short grace periods, which could in turn result in > random memory corruption. > Yes, I missed reading the call-chain for accessing dynticks_snap. It does not need any synchronization/barriers. Here since we are reading ->dynticks, doesn't having one barrier before reading make sense? like smp_mb(); dynticks_snap = atomic_read(...->dynticks); instead of having two barriers with atomic_add_return()? i.e., why is the second barrier necessary? On a related note, I see that dynticks is a per-cpu variable _and_ atomic. Is there such a need for that? Digging into history I see that the change to an atomic variable was made in the commit 23b5c8fa01b723c70a which says: In addition, the old dyntick-idle synchronization depended on the fact that grace periods were many milliseconds in duration, so that it could be assumed that no dyntick-idle CPU could reorder a memory reference across an entire grace period. Unfortunately for this design, the addition of expedited grace periods breaks this assumption, which has the unfortunate side-effect of requiring atomic operations in the functions that track dyntick-idle state for RCU. Sorry to ask you about such an old change. But I am not able to see why we need atomic_t for dynticks here since per-cpu operations are guaranteed to be atomic. It gets twisted pretty fast trying to understand the RCU code. No wonder people say that rcu is scary black magic :) -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: fix acronym for read-modify-write
fix acronym for read-modify-write from RMV to RMW Signed-off-by: Pranith Kumar --- Documentation/this_cpu_ops.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..9e44fdf 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -21,7 +21,7 @@ Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate without the typical synchronization overhead but still provide some sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] doc: fix acronym for read-modify-write
fix acronym for read-modify-write from RMV to RMW Signed-off-by: Pranith Kumar bobby.pr...@gmail.com --- Documentation/this_cpu_ops.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/this_cpu_ops.txt b/Documentation/this_cpu_ops.txt index 1a4ce7e..9e44fdf 100644 --- a/Documentation/this_cpu_ops.txt +++ b/Documentation/this_cpu_ops.txt @@ -21,7 +21,7 @@ Read-modify-write operations are of particular interest. Frequently processors have special lower latency instructions that can operate without the typical synchronization overhead but still provide some sort of relaxed atomicity guarantee. The x86 for example can execute -RMV (Read Modify Write) instructions like inc/dec/cmpxchg without the +RMW (Read Modify Write) instructions like inc/dec/cmpxchg without the lock prefix and the associated latency penalty. Access to the variable without the lock prefix is not synchronized but -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Wed, Jul 9, 2014 at 3:56 PM, Paul E. McKenney paul...@linux.vnet.ibm.com wrote: snip OK, so -dynticks_snap is accessed by only one task, namely the corresponding RCU grace-period kthread. So it can be accessed without any atomic instructions or memory barriers, since all accesses to it are single-threaded. On the other hand, -dynticks is written by one CPU and potentially accessed from any CPU. Therefore, accesses to it must take concurrency into account. Especially given that any confusion can fool RCU into thinking that a CPU is idle when it really is not, which could result in too-short grace periods, which could in turn result in random memory corruption. Yes, I missed reading the call-chain for accessing dynticks_snap. It does not need any synchronization/barriers. Here since we are reading -dynticks, doesn't having one barrier before reading make sense? like smp_mb(); dynticks_snap = atomic_read(...-dynticks); instead of having two barriers with atomic_add_return()? i.e., why is the second barrier necessary? On a related note, I see that dynticks is a per-cpu variable _and_ atomic. Is there such a need for that? Digging into history I see that the change to an atomic variable was made in the commit 23b5c8fa01b723c70a which says: quote In addition, the old dyntick-idle synchronization depended on the fact that grace periods were many milliseconds in duration, so that it could be assumed that no dyntick-idle CPU could reorder a memory reference across an entire grace period. Unfortunately for this design, the addition of expedited grace periods breaks this assumption, which has the unfortunate side-effect of requiring atomic operations in the functions that track dyntick-idle state for RCU. /quote Sorry to ask you about such an old change. But I am not able to see why we need atomic_t for dynticks here since per-cpu operations are guaranteed to be atomic. It gets twisted pretty fast trying to understand the RCU code. No wonder people say that rcu is scary black magic :) -- Pranith -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] rcu: use true/false instead of 1/0 for a bool type
One more bool type change which I found while reading the code.
Use true/false instead of 0/1 for a bool type
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 1b70cb6..bc7b6f1 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1668,7 +1668,7 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int
fqs_state_in)
if (fqs_state == RCU_SAVE_DYNTICK) {
/* Collect dyntick-idle snapshots. */
if (is_sysidle_rcu_state(rsp)) {
- isidle = 1;
+ isidle = true;
maxj = jiffies - ULONG_MAX / 4;
}
force_qs_rnp(rsp, dyntick_save_progress_counter,
@@ -1677,7 +1677,7 @@ static int rcu_gp_fqs(struct rcu_state *rsp, int
fqs_state_in)
fqs_state = RCU_FORCE_QS;
} else {
/* Handle dyntick-idle and offline CPUs. */
- isidle = 0;
+ isidle = false;
force_qs_rnp(rsp, rcu_implicit_dynticks_qs, , );
}
/* Clear flag to prevent immediate re-entry. */
@@ -2449,7 +2449,7 @@ static void force_qs_rnp(struct rcu_state *rsp,
for (; cpu <= rnp->grphi; cpu++, bit <<= 1) {
if ((rnp->qsmask & bit) != 0) {
if ((rnp->qsmaskinit & bit) != 0)
- *isidle = 0;
+ *isidle = false;
if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
mask |= bit;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
On Tue, Jul 8, 2014 at 8:07 PM, Paul E. McKenney
wrote:
> On Tue, Jul 08, 2014 at 06:55:45PM -0400, Pranith Kumar wrote:
>> atomic_add_return() invalidates the cache line in other processors where-as
>> atomic_read does not. I don't see why we would need invalidation in this
>> case.
>> If indeed it was need a comment would be helpful for readers. Otherwise
>> doesn't
>> using atomic_read() make more sense here? RFC!
>>
>> replace atomic_add_return(0, v) with atomic_read(v) as the latter is better.
>>
>> Signed-off-by: Pranith Kumar
>
> This will break RCU -- the full memory barriers implied both before
> and after atomic_add_return() are needed in order for RCU to be able to
> avoid death due to memory reordering.
>
> That said, I have considered replacing the atomic_add_return() with:
>
> smp_mb();
> ... = atomic_read(...);
> smp_mb();
>
> However, this is a very ticklish change, and would need serious thought
> and even more serious testing.
>
Thank you for looking at the RFC. I tried understanding the code
deeper and found that the ordering which is needed here is actually on
dynticks_snap.
The ordering currently (by way of atomic_add_return) is on
rdp->dynticks->dynticks which I think is not right.
Looking at the history of the code led me to rev. 23b5c8fa01b723 which
makes me think that the above statement is true. I think providing
ordering guarantees on dynticks_snap should be enough.
I have added an updated patch below. However, it is really hard(and
error prone) to convince oneself that this is right. So I will not
pursue this further if you think this is wrong. You definitely know
better than me :)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 1b70cb6..bbccd0a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -891,7 +891,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
static int dyntick_save_progress_counter(struct rcu_data *rdp,
bool *isidle, unsigned long *maxj)
{
- rdp->dynticks_snap = atomic_add_return(0, >dynticks->dynticks);
+ smp_store_release(>dynticks_snap,
atomic_read(>dynticks->dynticks));
rcu_sysidle_check_cpu(rdp, isidle, maxj);
if ((rdp->dynticks_snap & 0x1) == 0) {
trace_rcu_fqs(rdp->rsp->name, rdp->gpnum, rdp->cpu, TPS("dti"));
@@ -920,8 +920,8 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
int *rcrmp;
unsigned int snap;
- curr = (unsigned int)atomic_add_return(0, >dynticks->dynticks);
- snap = (unsigned int)rdp->dynticks_snap;
+ curr = (unsigned int)atomic_read(>dynticks->dynticks);
+ snap = (unsigned int)smp_load_acquire(>dynticks_snap);
/*
* If the CPU passed through or entered a dynticks idle phase with
--
Pranith
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH 1/1] rcu: use atomic_read(v) instead of atomic_add_return(0, v)
atomic_add_return() invalidates the cache line in other processors where-as
atomic_read does not. I don't see why we would need invalidation in this case.
If indeed it was need a comment would be helpful for readers. Otherwise doesn't
using atomic_read() make more sense here? RFC!
replace atomic_add_return(0, v) with atomic_read(v) as the latter is better.
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index dac6d20..a4a8f5f 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -891,7 +891,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
static int dyntick_save_progress_counter(struct rcu_data *rdp,
bool *isidle, unsigned long *maxj)
{
- rdp->dynticks_snap = atomic_add_return(0, >dynticks->dynticks);
+ rdp->dynticks_snap = atomic_read(>dynticks->dynticks);
rcu_sysidle_check_cpu(rdp, isidle, maxj);
if ((rdp->dynticks_snap & 0x1) == 0) {
trace_rcu_fqs(rdp->rsp->name, rdp->gpnum, rdp->cpu, TPS("dti"));
@@ -920,7 +920,7 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp,
int *rcrmp;
unsigned int snap;
- curr = (unsigned int)atomic_add_return(0, >dynticks->dynticks);
+ curr = (unsigned int)atomic_read(>dynticks->dynticks);
snap = (unsigned int)rdp->dynticks_snap;
/*
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 5/5] kernel/rcu/rcutorture.c:185 fix a sparse warning
On Tue, Jul 8, 2014 at 6:35 PM, Paul E. McKenney wrote: > On Wed, Jun 11, 2014 at 02:47:52PM -0700, j...@joshtriplett.org wrote: >> On Wed, Jun 11, 2014 at 04:39:43PM -0400, Pranith Kumar wrote: >> > fix the following sparse warning >> > >> > kernel/rcu/rcutorture.c:185:1: warning: symbol 'boost_mutex' was not >> > declared. Should it be static? >> > >> > by marking boost_mutex as a static mutex >> > >> > Signed-off-by: Pranith Kumar >> >> Please preserve the comment alignment (by deleting a tab). With that >> fixed: >> Reviewed-by: Josh Triplett > > Queued for 3.18. > > But Pranith, next time Josh gives you a review comment, could you please > respond with the appropriate update? > I was away from my work desktop for the past few days. I actually sent a fixed patch 10 min ago, but in a new series. Sorry for missing this, I will drop an update promptly next time. -- Pranith -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/9] rcu: trivial sparse and return type fixes for rcu
This patch series contains trivial fixes for sparse warnings and using bool types where appropriate instead of 0/1. I can probably merge the return type changes into one commit if you think that is appropriate. Pranith Kumar (9): rcu: fix sparse warning about rcu_batches_completed_preempt( ) being non-static rcu: use bool type for return value in rcu_is_watching() rcu: Fix a sparse warning about boost_mutex being non-static rcu: return bool type in rcu_lockdep_current_cpu_online() rcu: return bool type for rcu_try_advance_all_cbs() rcu: use true/false for return in __call_rcu_nocb() rcu: use true/false for return in rcu_nocb_adopt_orphan_cbs() rcu: use false for return in __call_rcu_nocb() rcu: use bool return type in rcu_nocb_adopt_orphan_cbs() include/linux/rcupdate.h | 2 +- kernel/rcu/rcutorture.c | 2 +- kernel/rcu/tree.c| 2 +- kernel/rcu/tree_plugin.h | 16 4 files changed, 11 insertions(+), 11 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/9] rcu: use bool type for return value in rcu_is_watching()
use a bool type for return in rcu_is_watching()
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index dac6d20..dff3d5b 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -819,7 +819,7 @@ bool notrace __rcu_is_watching(void)
*/
bool notrace rcu_is_watching(void)
{
- int ret;
+ bool ret;
preempt_disable();
ret = __rcu_is_watching();
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/9] rcu: Fix a sparse warning about boost_mutex being non-static
fix a sparse warning about boost_mutex being non-static by marking it as static Signed-off-by: Pranith Kumar --- kernel/rcu/rcutorture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c index 7fa34f8..5ec0452 100644 --- a/kernel/rcu/rcutorture.c +++ b/kernel/rcu/rcutorture.c @@ -182,7 +182,7 @@ static u64 notrace rcu_trace_clock_local(void) #endif /* #else #ifdef CONFIG_RCU_TRACE */ static unsigned long boost_starttime; /* jiffies of next boost test start. */ -DEFINE_MUTEX(boost_mutex); /* protect setting boost_starttime */ +static DEFINE_MUTEX(boost_mutex); /* protect setting boost_starttime */ /* and boost task create/destroy. */ static atomic_t barrier_cbs_count; /* Barrier callbacks registered. */ static bool barrier_phase; /* Test phase. */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/9] rcu: fix sparse warning about rcu_batches_completed_preempt() being non-static
fix sparse warning about rcu_batches_completed_preempt() being non-static by
marking it as static
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 637a8a9..1a4ab26 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -134,7 +134,7 @@ static void __init rcu_bootup_announce(void)
* Return the number of RCU-preempt batches processed thus far
* for debug and statistics.
*/
-long rcu_batches_completed_preempt(void)
+static long rcu_batches_completed_preempt(void)
{
return rcu_preempt_state.completed;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/9] rcu: return bool type for rcu_try_advance_all_cbs()
return a bool type instead of 0 in rcu_try_advance_all_cbs()
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 1a4ab26..6c1feee 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1625,7 +1625,7 @@ static bool __maybe_unused rcu_try_advance_all_cbs(void)
/* Exit early if we advanced recently. */
if (jiffies == rdtp->last_advance_all)
- return 0;
+ return false;
rdtp->last_advance_all = jiffies;
for_each_rcu_flavor(rsp) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 8/9] rcu: return false instead of 0 in rcu_nocb_adopt_orphan_cbs()
return false instead of 0 in rcu_nocb_adopt_orphan_cbs() as this has bool as
return type
Signed-off-by: Pranith Kumar
---
kernel/rcu/tree_plugin.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 5c21f4b..9a4dc07 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2389,7 +2389,7 @@ static bool __maybe_unused
rcu_nocb_adopt_orphan_cbs(struct rcu_state *rsp,
struct rcu_data *rdp,
unsigned long flags)
{
- return 0;
+ return false;
}
static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
