Jules Irenge (5):
rcu/rcutorture: replace 0 with false
rcu: replace 1 with true
rcu: replace + with |
x86/ftrace: Add annotations for ftrace_arch_code_modify_prepare() and
ftrace_arch_code_modify_post_process()
sfc: add missing annotation for efx_ef10_try_update_nic_stats_vf()
arch/x86/kernel/ftrace.c | 2 ++
drivers/net/ethernet/sfc/ef10.c | 1 +
kernel/rcu/rcutorture.c | 2 +-
kernel/rcu/tree_plugin.h | 22 +++++++++++-----------
kernel/rcu/update.c | 2 +-
5 files changed, 16 insertions(+), 13 deletions(-)
--
2.18.2
Sparse reports warnings
warning: context imbalance in ftrace_arch_code_modify_prepare()
- wrong count at exit
warning: context imbalance in ftrace_arch_code_modify_post_process()
- wrong count at exit
The root cause is that even if
the annotations on the function are correct,
mutex do not support annotation
This makes Sparse to complain.
To fix this,
__acquire(&text_mutex) and
__release(&text_mutex) annotations are added
inside ftrace_arch_code_modify_prepare()
and ftrace_arch_code_modify_post_process()
respectively.
Signed-off-by: Jules Irenge <[email protected]>
---
arch/x86/kernel/ftrace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 37a0aeaf89e7..737c07ab2e07 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -46,6 +46,7 @@ int ftrace_arch_code_modify_prepare(void)
* ftrace has it set to "read/write".
*/
mutex_lock(&text_mutex);
+ __acquire(&text_mutex);
ftrace_poke_late = 1;
return 0;
}
@@ -61,6 +62,7 @@ int ftrace_arch_code_modify_post_process(void)
text_poke_finish();
ftrace_poke_late = 0;
mutex_unlock(&text_mutex);
+ __release(&text_mutex);
return 0;
}
--
2.18.2
Coccinelle reports a warning
WARNING: Assignment of 0/1 to bool variable
The root cause is the variable lastphase is of bool type is initialised with integer 0
Replacing 0 with false fixes the issue.
Signed-off-by: Jules Irenge <[email protected]>
---
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 5453bd557f43..a082bc402f9b 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -2065,7 +2065,7 @@ static void rcu_torture_barrier1cb(void *rcu_void)
static int rcu_torture_barrier_cbs(void *arg)
{
long myid = (long)arg;
- bool lastphase = 0;
+ bool lastphase = false;
bool newphase;
struct rcu_head rcu;
--
2.18.2
Coccinelle reports warnings at rcu_preempt_ctxt_queue()
WARNING: sum of probable bitmasks, consider |
The root cause is the use of addition operator + for bitmask defined macros variables
Replacing + with | fixes the issue.
Signed-off-by: Jules Irenge <[email protected]>
---
kernel/rcu/tree_plugin.h | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 097635c41135..a20135ece06a 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -153,9 +153,9 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp)
switch (blkd_state) {
case 0:
case RCU_EXP_TASKS:
- case RCU_EXP_TASKS + RCU_GP_BLKD:
+ case RCU_EXP_TASKS | RCU_GP_BLKD:
case RCU_GP_TASKS:
- case RCU_GP_TASKS + RCU_EXP_TASKS:
+ case RCU_GP_TASKS | RCU_EXP_TASKS:
/*
* Blocking neither GP, or first task blocking the normal
@@ -168,10 +168,10 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp)
case RCU_EXP_BLKD:
case RCU_GP_BLKD:
- case RCU_GP_BLKD + RCU_EXP_BLKD:
- case RCU_GP_TASKS + RCU_EXP_BLKD:
- case RCU_GP_TASKS + RCU_GP_BLKD + RCU_EXP_BLKD:
- case RCU_GP_TASKS + RCU_EXP_TASKS + RCU_GP_BLKD + RCU_EXP_BLKD:
+ case RCU_GP_BLKD | RCU_EXP_BLKD:
+ case RCU_GP_TASKS | RCU_EXP_BLKD:
+ case RCU_GP_TASKS | RCU_GP_BLKD | RCU_EXP_BLKD:
+ case RCU_GP_TASKS | RCU_EXP_TASKS | RCU_GP_BLKD | RCU_EXP_BLKD:
/*
* First task arriving that blocks either GP, or first task
@@ -184,9 +184,9 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp)
list_add_tail(&t->rcu_node_entry, &rnp->blkd_tasks);
break;
- case RCU_EXP_TASKS + RCU_EXP_BLKD:
- case RCU_EXP_TASKS + RCU_GP_BLKD + RCU_EXP_BLKD:
- case RCU_GP_TASKS + RCU_EXP_TASKS + RCU_EXP_BLKD:
+ case RCU_EXP_TASKS | RCU_EXP_BLKD:
+ case RCU_EXP_TASKS | RCU_GP_BLKD | RCU_EXP_BLKD:
+ case RCU_GP_TASKS | RCU_EXP_TASKS | RCU_EXP_BLKD:
/*
* Second or subsequent task blocking the expedited GP.
@@ -197,8 +197,8 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp)
list_add(&t->rcu_node_entry, rnp->exp_tasks);
break;
- case RCU_GP_TASKS + RCU_GP_BLKD:
- case RCU_GP_TASKS + RCU_EXP_TASKS + RCU_GP_BLKD:
+ case RCU_GP_TASKS | RCU_GP_BLKD:
+ case RCU_GP_TASKS | RCU_EXP_TASKS | RCU_GP_BLKD:
/*
* Second or subsequent task blocking the normal GP.
--
2.18.2
Sparse reports a warning at efx_ef10_try_update_nic_stats_vf()
warning: context imbalance in efx_ef10_try_update_nic_stats_vf()
- unexpected unlock
The root cause is the missing annotation at
efx_ef10_try_update_nic_stats_vf()
Add the missing _must_hold(&efx->stats_lock) annotation
Signed-off-by: Jules Irenge <[email protected]>
---
drivers/net/ethernet/sfc/ef10.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 3f16bd807c6e..e8bbbd366625 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -1820,6 +1820,7 @@ static size_t efx_ef10_update_stats_pf(struct efx_nic *efx, u64 *full_stats,
}
static int efx_ef10_try_update_nic_stats_vf(struct efx_nic *efx)
+ __must_hold(&efx->stats_lock)
{
MCDI_DECLARE_BUF(inbuf, MC_CMD_MAC_STATS_IN_LEN);
struct efx_ef10_nic_data *nic_data = efx->nic_data;
--
2.18.2
Coccinelle reports a warning
WARNING: Assignment of 0/1 to bool variable
The root cause is the variable rcu_boot_ended of bool type is initialised with integer 1
Replacing 1 with true fixes the issue.
Signed-off-by: Jules Irenge <[email protected]>
---
kernel/rcu/update.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 28a8bdc5072f..c18ae0cca512 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -193,7 +193,7 @@ void rcu_end_inkernel_boot(void)
rcu_unexpedite_gp();
if (rcu_normal_after_boot)
WRITE_ONCE(rcu_normal, 1);
- rcu_boot_ended = 1;
+ rcu_boot_ended = true;
}
/*
--
2.18.2
On Mon, 1 Jun 2020 19:45:51 +0100
Jules Irenge <[email protected]> wrote:
> Sparse reports warnings
>
> warning: context imbalance in ftrace_arch_code_modify_prepare()
> - wrong count at exit
> warning: context imbalance in ftrace_arch_code_modify_post_process()
> - wrong count at exit
>
> The root cause is that even if
> the annotations on the function are correct,
> mutex do not support annotation
> This makes Sparse to complain.
> To fix this,
> __acquire(&text_mutex) and
> __release(&text_mutex) annotations are added
> inside ftrace_arch_code_modify_prepare()
> and ftrace_arch_code_modify_post_process()
> respectively.
Wait what? This looks like either a bug in sparse, or we just remove the
annotations. This just makes the code ugly, and looks silly.
Nack!
-- Steve
>
> Signed-off-by: Jules Irenge <[email protected]>
> ---
> arch/x86/kernel/ftrace.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 37a0aeaf89e7..737c07ab2e07 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -46,6 +46,7 @@ int ftrace_arch_code_modify_prepare(void)
> * ftrace has it set to "read/write".
> */
> mutex_lock(&text_mutex);
> + __acquire(&text_mutex);
> ftrace_poke_late = 1;
> return 0;
> }
> @@ -61,6 +62,7 @@ int ftrace_arch_code_modify_post_process(void)
> text_poke_finish();
> ftrace_poke_late = 0;
> mutex_unlock(&text_mutex);
> + __release(&text_mutex);
> return 0;
> }
>
On Mon, 1 Jun 2020, Steven Rostedt wrote:
> On Mon, 1 Jun 2020 19:45:51 +0100
> Jules Irenge <[email protected]> wrote:
>
>> Sparse reports warnings
>>
>> warning: context imbalance in ftrace_arch_code_modify_prepare()
>> - wrong count at exit
>> warning: context imbalance in ftrace_arch_code_modify_post_process()
>> - wrong count at exit
>>
>> The root cause is that even if
>> the annotations on the function are correct,
>> mutex do not support annotation
>> This makes Sparse to complain.
>> To fix this,
>> __acquire(&text_mutex) and
>> __release(&text_mutex) annotations are added
>> inside ftrace_arch_code_modify_prepare()
>> and ftrace_arch_code_modify_post_process()
>> respectively.
>
> Wait what? This looks like either a bug in sparse, or we just remove the
> annotations. This just makes the code ugly, and looks silly.
>
> Nack!
>
> -- Steve
>
>
>
Thanks for the feedback, I take good note.
Jules
From: Jules Irenge <[email protected]>
Date: Mon, 1 Jun 2020 19:45:52 +0100
> Sparse reports a warning at efx_ef10_try_update_nic_stats_vf()
> warning: context imbalance in efx_ef10_try_update_nic_stats_vf()
> - unexpected unlock
> The root cause is the missing annotation at
> efx_ef10_try_update_nic_stats_vf()
> Add the missing _must_hold(&efx->stats_lock) annotation
>
> Signed-off-by: Jules Irenge <[email protected]>
Applied.
On Mon, Jun 01, 2020 at 07:45:47PM +0100, Jules Irenge wrote:
>
> Jules Irenge (5):
> rcu/rcutorture: replace 0 with false
> rcu: replace 1 with true
I queued these two, thank you!
> rcu: replace + with |
This one I am not all that excited about, so I am leaving it off.
Thanx, Paul
> x86/ftrace: Add annotations for ftrace_arch_code_modify_prepare() and
> ftrace_arch_code_modify_post_process()
> sfc: add missing annotation for efx_ef10_try_update_nic_stats_vf()
>
> arch/x86/kernel/ftrace.c | 2 ++
> drivers/net/ethernet/sfc/ef10.c | 1 +
> kernel/rcu/rcutorture.c | 2 +-
> kernel/rcu/tree_plugin.h | 22 +++++++++++-----------
> kernel/rcu/update.c | 2 +-
> 5 files changed, 16 insertions(+), 13 deletions(-)
>
> --
> 2.18.2
>
On Mon, Jun 01, 2020 at 03:46:47PM -0400, Steven Rostedt wrote:
> On Mon, 1 Jun 2020 19:45:51 +0100
> Jules Irenge <[email protected]> wrote:
>
> > Sparse reports warnings
> >
> > warning: context imbalance in ftrace_arch_code_modify_prepare()
> > - wrong count at exit
> > warning: context imbalance in ftrace_arch_code_modify_post_process()
> > - wrong count at exit
> >
> > The root cause is that even if
> > the annotations on the function are correct,
> > mutex do not support annotation
Yes.
> Wait what? This looks like either a bug in sparse, or we just remove the
> annotations. This just makes the code ugly, and looks silly.
The annotations added by commit
074376ac0e1d ("ftrace/x86: Anotate text_mutex split between ...
are indeed wrong (because they don't match what the functions are
really doing / mutex operations have never been annotated).
The're also pointless since their prototypes are un-annotated.
-- Luc
On Wed, 3 Jun 2020, Luc Van Oostenryck wrote:
> On Mon, Jun 01, 2020 at 03:46:47PM -0400, Steven Rostedt wrote:
>> On Mon, 1 Jun 2020 19:45:51 +0100
>> Jules Irenge <[email protected]> wrote:
>>
>>> Sparse reports warnings
>>>
>>> warning: context imbalance in ftrace_arch_code_modify_prepare()
>>> - wrong count at exit
>>> warning: context imbalance in ftrace_arch_code_modify_post_process()
>>> - wrong count at exit
>>>
>>> The root cause is that even if
>>> the annotations on the function are correct,
>>> mutex do not support annotation
>
> Yes.
>
>> Wait what? This looks like either a bug in sparse, or we just remove the
>> annotations. This just makes the code ugly, and looks silly.
>
> The annotations added by commit
> 074376ac0e1d ("ftrace/x86: Anotate text_mutex split between ...
> are indeed wrong (because they don't match what the functions are
> really doing / mutex operations have never been annotated).
> The're also pointless since their prototypes are un-annotated.
Interesting, I would think the best way would then be to remove the
annotations. There are quite a number of them.
I will have to investigate more on mutex and annotation before moving
forward.
Thanks for feedback.
Jules
On Tue, 2 Jun 2020, Paul E. McKenney wrote:
> On Mon, Jun 01, 2020 at 07:45:47PM +0100, Jules Irenge wrote:
>>
>> Jules Irenge (5):
>> rcu/rcutorture: replace 0 with false
>> rcu: replace 1 with true
>
> I queued these two, thank you!
>
>> rcu: replace + with |
>
> This one I am not all that excited about, so I am leaving it off.
>
> Thanx, Paul
Thanks for the feedback.
Jules