mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call

PATCH In review Brian Norris 3 participants 5 messages Nov 4, 2015 – Nov 6, 2015 Memory technology devices (MTD)

lore

Share & tools

lore

Brian Norris

author

2015-11-04 01:09 UTC #0 lore raw

PATCH 1/1 mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call

There are multiple types of users of mtd->reboot_notifier.notifier_call:

(1) A while back, the cfi_cmdset_000{1,2} chip drivers implemented a reboot notifier to (on a best effort basis) attempt to reset their flash chips before rebooting.

(2) More recently, we implemented a common _reboot() hook so that MTD drivers (particularly, NAND flash) could better halt I/O operations without having to reimplement the same notifier boilerplate.

Currently, the WARN_ONCE() condition here was written to handle (2), but at the same time it mis-diagnosed case (1) as an already-registered MTD. Let's fix this by having the WARN_ONCE() condition better imitate the condition that immediately follows it. (Wow, I don't know how I missed that one.)

(Side note: Unfortunately, we can't yet combine the reboot notifier code for (1) and (2) with a patch like [1], because some users of (1) also use mtdconcat, and so the mtd_info struct from cfi_cmdset_000{1,2} won't actually get registered with mtdcore, and therefore their reboot notifier won't get registered.)

[1] http://patchwork.ozlabs.org/patch/417981/

Signed-off-by Brian Norris <[email protected]>

Cc Jesper Nilsson <[email protected]>

Cc [email protected]

---
This is probably a 4.4 candidate

 drivers/mtd/mtdcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mtd/mtdcore.c +2 -1

index b1eea48c501d..a91cee90aef9 100644
@@ -616,7 +616,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 	 * does cause problems with parse_mtd_partitions() above (e.g.,
 	 * cmdlineparts will register partitions more than once).
 	 */
-	WARN_ONCE(mtd->reboot_notifier.notifier_call, "MTD already registered\n");
+	WARN_ONCE(mtd->_reboot && mtd->reboot_notifier.notifier_call,
+		  "MTD already registered\n");
 	if (mtd->_reboot && !mtd->reboot_notifier.notifier_call) {
 		mtd->reboot_notifier.notifier_call = mtd_reboot_notifier;
 		register_reboot_notifier(&mtd->reboot_notifier);

Signature · 1 line

2.6.0.rc2.230.g3dd15c0

Ezequiel Garcia

Tested-by on v1 1/1

2015-11-04 12:06 UTC #1 lore raw

Quoted 22 lines · Brian Norris

There are multiple types of users of mtd->reboot_notifier.notifier_call:
(1) A while back, the cfi_cmdset_000{1,2} chip drivers implemented a reboot notifier to (on a best effort basis) attempt to reset their flash chips before rebooting.
(2) More recently, we implemented a common _reboot() hook so that MTD drivers (particularly, NAND flash) could better halt I/O operations without having to reimplement the same notifier boilerplate.
Currently, the WARN_ONCE() condition here was written to handle (2), but at the same time it mis-diagnosed case (1) as an already-registered MTD. Let's fix this by having the WARN_ONCE() condition better imitate the condition that immediately follows it. (Wow, I don't know how I missed that one.)
(Side note: Unfortunately, we can't yet combine the reboot notifier code for (1) and (2) with a patch like [1], because some users of (1) also use mtdconcat, and so the mtd_info struct from cfi_cmdset_000{1,2} won't actually get registered with mtdcore, and therefore their reboot notifier won't get registered.)

Quoted 9 lines

[1] http://patchwork.ozlabs.org/patch/417981/
Suggested-by Guenter Roeck <[email protected]>
Signed-off-by Brian Norris <[email protected]>
Cc Jesper Nilsson <[email protected]>
Cc [email protected]
---
This is probably a 4.4 candidate

Tested-by Ezequiel Garcia <[email protected]>

Thanks,

Quoted 24 lines

 drivers/mtd/mtdcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mtd/mtdcore.c +2 -1

index b1eea48c501d..a91cee90aef9 100644
@@ -616,7 +616,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 	 * does cause problems with parse_mtd_partitions() above (e.g.,
 	 * cmdlineparts will register partitions more than once).
 	 */
-	WARN_ONCE(mtd->reboot_notifier.notifier_call, "MTD already registered\n");
+	WARN_ONCE(mtd->_reboot && mtd->reboot_notifier.notifier_call,
+		  "MTD already registered\n");
 	if (mtd->_reboot && !mtd->reboot_notifier.notifier_call) {
 		mtd->reboot_notifier.notifier_call = mtd_reboot_notifier;
 		register_reboot_notifier(&mtd->reboot_notifier);

Signature · 6 lines

2.6.0.rc2.230.g3dd15c0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Signature · 2 lines

Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar

Brian Norris

author

2015-11-05 20:40 UTC #2 lore raw

In reply to Brian Norris

+ Guenter

I guess I picked a tag (Suggested-by) that git-send-email *doesn't* automatically CC, then forgot to CC Guenter manually. And now I feel like a doofus waiting for Guenter's 'Tested-by'.

Brian

Quoted 51 lines · Brian Norris

There are multiple types of users of mtd->reboot_notifier.notifier_call:
(1) A while back, the cfi_cmdset_000{1,2} chip drivers implemented a reboot notifier to (on a best effort basis) attempt to reset their flash chips before rebooting.
(2) More recently, we implemented a common _reboot() hook so that MTD drivers (particularly, NAND flash) could better halt I/O operations without having to reimplement the same notifier boilerplate.
Currently, the WARN_ONCE() condition here was written to handle (2), but at the same time it mis-diagnosed case (1) as an already-registered MTD. Let's fix this by having the WARN_ONCE() condition better imitate the condition that immediately follows it. (Wow, I don't know how I missed that one.)
(Side note: Unfortunately, we can't yet combine the reboot notifier code for (1) and (2) with a patch like [1], because some users of (1) also use mtdconcat, and so the mtd_info struct from cfi_cmdset_000{1,2} won't actually get registered with mtdcore, and therefore their reboot notifier won't get registered.)
[1] http://patchwork.ozlabs.org/patch/417981/
Suggested-by Guenter Roeck <[email protected]>
Signed-off-by Brian Norris <[email protected]>
Cc Jesper Nilsson <[email protected]>
Cc [email protected]
---
This is probably a 4.4 candidate
 drivers/mtd/mtdcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mtd/mtdcore.c +2 -1
index b1eea48c501d..a91cee90aef9 100644
@@ -616,7 +616,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 	 * does cause problems with parse_mtd_partitions() above (e.g.,
 	 * cmdlineparts will register partitions more than once).
 	 */
-	WARN_ONCE(mtd->reboot_notifier.notifier_call, "MTD already registered\n");
+	WARN_ONCE(mtd->_reboot && mtd->reboot_notifier.notifier_call,
+		  "MTD already registered\n");
 	if (mtd->_reboot && !mtd->reboot_notifier.notifier_call) {
 		mtd->reboot_notifier.notifier_call = mtd_reboot_notifier;
 		register_reboot_notifier(&mtd->reboot_notifier);
Signature · 1 line
2.6.0.rc2.230.g3dd15c0

Guenter Roeck

Tested-by on v1 1/1

2015-11-06 14:16 UTC #3 lore raw

In reply to Brian Norris

Quoted 22 lines · Brian Norris

There are multiple types of users of mtd->reboot_notifier.notifier_call:
(1) A while back, the cfi_cmdset_000{1,2} chip drivers implemented a reboot notifier to (on a best effort basis) attempt to reset their flash chips before rebooting.
(2) More recently, we implemented a common _reboot() hook so that MTD drivers (particularly, NAND flash) could better halt I/O operations without having to reimplement the same notifier boilerplate.
Currently, the WARN_ONCE() condition here was written to handle (2), but at the same time it mis-diagnosed case (1) as an already-registered MTD. Let's fix this by having the WARN_ONCE() condition better imitate the condition that immediately follows it. (Wow, I don't know how I missed that one.)
(Side note: Unfortunately, we can't yet combine the reboot notifier code for (1) and (2) with a patch like [1], because some users of (1) also use mtdconcat, and so the mtd_info struct from cfi_cmdset_000{1,2} won't actually get registered with mtdcore, and therefore their reboot notifier won't get registered.)

Quoted 6 lines

[1] http://patchwork.ozlabs.org/patch/417981/
Suggested-by Guenter Roeck <[email protected]>
Signed-off-by Brian Norris <[email protected]>
Cc Jesper Nilsson <[email protected]>
Cc [email protected]

Tested-by Guenter Roeck <[email protected]>

Quoted 29 lines

---
This is probably a 4.4 candidate

 drivers/mtd/mtdcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mtd/mtdcore.c +2 -1

index b1eea48c501d..a91cee90aef9 100644
@@ -616,7 +616,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 	 * does cause problems with parse_mtd_partitions() above (e.g.,
 	 * cmdlineparts will register partitions more than once).
 	 */
-	WARN_ONCE(mtd->reboot_notifier.notifier_call, "MTD already registered\n");
+	WARN_ONCE(mtd->_reboot && mtd->reboot_notifier.notifier_call,
+		  "MTD already registered\n");
 	if (mtd->_reboot && !mtd->reboot_notifier.notifier_call) {
 		mtd->reboot_notifier.notifier_call = mtd_reboot_notifier;
 		register_reboot_notifier(&mtd->reboot_notifier);

Signature · 6 lines

2.6.0.rc2.230.g3dd15c0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Brian Norris

author

2015-11-06 17:35 UTC #4 lore raw

Quoted 23 lines · Guenter Roeck

Quoted 22 lines · Brian Norris
There are multiple types of users of mtd->reboot_notifier.notifier_call:
(1) A while back, the cfi_cmdset_000{1,2} chip drivers implemented a reboot notifier to (on a best effort basis) attempt to reset their flash chips before rebooting.
(2) More recently, we implemented a common _reboot() hook so that MTD drivers (particularly, NAND flash) could better halt I/O operations without having to reimplement the same notifier boilerplate.
Currently, the WARN_ONCE() condition here was written to handle (2), but at the same time it mis-diagnosed case (1) as an already-registered MTD. Let's fix this by having the WARN_ONCE() condition better imitate the condition that immediately follows it. (Wow, I don't know how I missed that one.)
(Side note: Unfortunately, we can't yet combine the reboot notifier code for (1) and (2) with a patch like [1], because some users of (1) also use mtdconcat, and so the mtd_info struct from cfi_cmdset_000{1,2} won't actually get registered with mtdcore, and therefore their reboot notifier won't get registered.)

Quoted 8 lines

Quoted 6 lines
[1] http://patchwork.ozlabs.org/patch/417981/
Suggested-by Guenter Roeck <[email protected]>
Signed-off-by Brian Norris <[email protected]>
Cc Jesper Nilsson <[email protected]>
Cc [email protected]
Tested-by Guenter Roeck <[email protected]>

Thanks! Pushed to l2-mtd.git.

I'll be queueing up the 4.4-rc1 pull request soon.

Brian