bus: arm-ccn: Fix cpu notifier priority

PATCH New Jan Glauber 3 participants 5 messages Mar 8, 2016 – Mar 10, 2016

lore

Share & tools

lore

Jan Glauber

author

2016-03-08 12:14 UTC #0 lore raw

PATCH 1/1 bus: arm-ccn: Fix cpu notifier priority

Fix cpu priority of notifier block that was erroneously set to return value of next statement.

Signed-off-by Jan Glauber <[email protected]>

---
 drivers/bus/arm-ccn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/bus/arm-ccn.c +1 -1

index 7082c72..d4a939e 100644
@@ -1271,7 +1271,7 @@ static int arm_ccn_pmu_init(struct arm_ccn *ccn)
 	 * picked to have a chance to migrate events before perf is notified.
 	 */
 	ccn->dt.cpu_nb.notifier_call = arm_ccn_pmu_cpu_notifier;
-	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1,
+	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1;
 	err = register_cpu_notifier(&ccn->dt.cpu_nb);
 	if (err)
 		goto error_cpu_notifier;

Signature · 1 line

1.9.1

Pawel Moll

2016-03-09 08:40 UTC #1 lore raw

PATCH 1/1 bus: arm-ccn: Fix cpu notifier priority

Quoted 16 lines · Jan Glauber

Fix cpu priority of notifier block that was erroneously set to return value of next statement.
Signed-off-by Jan Glauber <[email protected]>
---
 drivers/bus/arm-ccn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-ccn.c +0 -0
index 7082c72..d4a939e 100644
@@ -1271,7 +1271,7 @@ static int arm_ccn_pmu_init(struct arm_ccn
 *ccn)
 	 * picked to have a chance to migrate events before perf is
notified.

drivers/bus/arm-ccn.c static int arm_ccn_pmu_init(struct arm_ccn *ccn) · line 1277

Quoted 7 lines

drivers/bus/arm-ccn.c +1 -1

 	 */
 	ccn->dt.cpu_nb.notifier_call = arm_ccn_pmu_cpu_notifier;
-	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1,
+	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1;
 	err = register_cpu_notifier(&ccn->dt.cpu_nb);
 	if (err)
 		goto error_cpu_notifier;

Damn, of course! I have never made (or heard about) such a typo before
- I wish the compiler warned about it :-(

By all means:

Acked-by Pawel Moll <[email protected]>

Cc <[email protected]> # 4.2+

Arnd, would you be still able to pick it up in this cycle?

Thanks Both!

Pawe?

Jan Glauber

2016-03-09 14:24 UTC #2 lore raw

PATCH 1/1 bus: arm-ccn: Fix cpu notifier priority

Quoted 19 lines · Pawel Moll

Quoted 18 lines · Jan Glauber
Fix cpu priority of notifier block that was erroneously set to return value of next statement.
Signed-off-by Jan Glauber <[email protected]>
---
 drivers/bus/arm-ccn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-ccn.c +0 -0
index 7082c72..d4a939e 100644
@@ -1271,7 +1271,7 @@ static int arm_ccn_pmu_init(struct arm_ccn
 *ccn)
 	 * picked to have a chance to migrate events before perf is
notified.
drivers/bus/arm-ccn.c +0 -0
 	 */
 	ccn->dt.cpu_nb.notifier_call = arm_ccn_pmu_cpu_notifier;

Quoted 8 lines

drivers/bus/arm-ccn.c static int arm_ccn_pmu_init(struct arm_ccn *ccn) · line 1277
Quoted 5 lines
drivers/bus/arm-ccn.c +1 -1
-	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1,
+	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1;
 	err = register_cpu_notifier(&ccn->dt.cpu_nb);
 	if (err)
 		goto error_cpu_notifier;
Damn, of course! I have never made (or heard about) such a typo before
- I wish the compiler warned about it :-(

Yeah, I was also surprised that we have plenty of cosmetical checks but nothing that catches this bug.

Maybe it's not too hard to catch the simple case, just check for a line with assignment and no brackets / parenthesis ended by a comma?

Jan

Quoted 10 lines

By all means:
Acked-by Pawel Moll <[email protected]>
Cc <[email protected]> # 4.2+
Arnd, would you be still able to pick it up in this cycle?
Thanks Both!
Pawe?

Jan Glauber

2016-03-10 14:34 UTC #3 lore raw

PATCH 1/1 bus: arm-ccn: Fix cpu notifier priority

In reply to Pawel Moll

Quoted 19 lines · Pawel Moll

Quoted 18 lines · Jan Glauber
Fix cpu priority of notifier block that was erroneously set to return value of next statement.
Signed-off-by Jan Glauber <[email protected]>
---
 drivers/bus/arm-ccn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-ccn.c +0 -0
index 7082c72..d4a939e 100644
@@ -1271,7 +1271,7 @@ static int arm_ccn_pmu_init(struct arm_ccn
 *ccn)
 	 * picked to have a chance to migrate events before perf is
notified.
drivers/bus/arm-ccn.c +0 -0
 	 */
 	ccn->dt.cpu_nb.notifier_call = arm_ccn_pmu_cpu_notifier;

Quoted 8 lines

drivers/bus/arm-ccn.c static int arm_ccn_pmu_init(struct arm_ccn *ccn) · line 1277
Quoted 5 lines
drivers/bus/arm-ccn.c +1 -1
-	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1,
+	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1;
 	err = register_cpu_notifier(&ccn->dt.cpu_nb);
 	if (err)
 		goto error_cpu_notifier;
Damn, of course! I have never made (or heard about) such a typo before
- I wish the compiler warned about it :-(

Hi Pawel,

actually my patch description is wrong. The compiler evaluates both statements but does _not_ overwrite the value of the first statement (would require parenthesis to do so). So it is just a cosmetical change, feel free to drop it.

Sorry for the noise,
Jan

Quoted 10 lines

By all means:
Acked-by Pawel Moll <[email protected]>
Cc <[email protected]> # 4.2+
Arnd, would you be still able to pick it up in this cycle?
Thanks Both!
Pawe?

Pawel Moll

2016-03-10 14:56 UTC #4 lore raw

PATCH 1/1 bus: arm-ccn: Fix cpu notifier priority

Quoted 31 lines · Jan Glauber

Quoted 21 lines · Pawel Moll
Quoted 20 lines · Jan Glauber
Fix cpu priority of notifier block that was erroneously set to return value of next statement.
Signed-off-by Jan Glauber <[email protected]>
---
 drivers/bus/arm-ccn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-ccn.c +0 -0
index 7082c72..d4a939e 100644
@@ -1271,7 +1271,7 @@ static int arm_ccn_pmu_init(struct arm_ccn
 *ccn)
 	 * picked to have a chance to migrate events before perf
is
notified.
drivers/bus/arm-ccn.c +0 -1
 	 */
 	ccn->dt.cpu_nb.notifier_call = arm_ccn_pmu_cpu_notifier;
-	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1,
Quoted 8 lines
drivers/bus/arm-ccn.c static int arm_ccn_pmu_init(struct arm_ccn *ccn) · line 1277
Quoted 4 lines
drivers/bus/arm-ccn.c +1 -0
+	ccn->dt.cpu_nb.priority = CPU_PRI_PERF + 1;
 	err = register_cpu_notifier(&ccn->dt.cpu_nb);
 	if (err)
 		goto error_cpu_notifier;
Damn, of course! I have never made (or heard about) such a typo before
- I wish the compiler warned about it :-(

Quoted 3 lines

actually my patch description is wrong. The compiler evaluates both statements but does _not_ overwrite the value of the first statement (would require parenthesis to do so).

I must admit I looked at the code change rather than at the description, but thinking about it now, comma has the lowest precedence, so the assignments will win. Still, with gcc 6 now warning about suspicious indentations, my wish still stays a wish ;-)

Quoted 1 line

So it is just a cosmetical change, feel free to drop it.

Oh no, it's still a bug (only maybe not a "stable-class" one). And one I'll remember :-)

Thanks!

Pawel