* Re: [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call
2015-11-04 1:01 [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call Brian Norris
@ 2015-11-04 12:01 ` Ezequiel Garcia
2015-11-05 20:40 ` Brian Norris
2015-11-06 14:15 ` Guenter Roeck
2 siblings, 0 replies; 5+ messages in thread
From: Ezequiel Garcia @ 2015-11-04 12:01 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd, Richard Weinberger, linux-cris-kernel, Jesper Nilsson
On 03 Nov 05:01 PM, Brian Norris wrote:
> 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 <linux@roeck-us.net>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: Jesper Nilsson <jespern@axis.com>
> Cc: linux-cris-kernel@axis.com
> ---
> This is probably a 4.4 candidate
>
Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Thanks,
> drivers/mtd/mtdcore.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index b1eea48c501d..a91cee90aef9 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -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);
> --
> 2.6.0.rc2.230.g3dd15c0
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call
2015-11-04 1:01 [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call Brian Norris
2015-11-04 12:01 ` Ezequiel Garcia
@ 2015-11-05 20:40 ` Brian Norris
2015-11-06 14:15 ` Guenter Roeck
2 siblings, 0 replies; 5+ messages in thread
From: Brian Norris @ 2015-11-05 20:40 UTC (permalink / raw)
To: linux-mtd
Cc: Richard Weinberger, Jesper Nilsson, linux-cris-kernel,
Guenter Roeck
+ 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
On Tue, Nov 03, 2015 at 05:01:53PM -0800, Brian Norris wrote:
> 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 <linux@roeck-us.net>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: Jesper Nilsson <jespern@axis.com>
> Cc: linux-cris-kernel@axis.com
> ---
> This is probably a 4.4 candidate
>
> drivers/mtd/mtdcore.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index b1eea48c501d..a91cee90aef9 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -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);
> --
> 2.6.0.rc2.230.g3dd15c0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call
2015-11-04 1:01 [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call Brian Norris
2015-11-04 12:01 ` Ezequiel Garcia
2015-11-05 20:40 ` Brian Norris
@ 2015-11-06 14:15 ` Guenter Roeck
2015-11-06 17:35 ` Brian Norris
2 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2015-11-06 14:15 UTC (permalink / raw)
To: Brian Norris
Cc: linux-mtd, Richard Weinberger, linux-cris-kernel, Jesper Nilsson
On Tue, Nov 03, 2015 at 05:01:53PM -0800, Brian Norris wrote:
> 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 <linux@roeck-us.net>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: Jesper Nilsson <jespern@axis.com>
> Cc: linux-cris-kernel@axis.com
Tested-by: Guenter Roeck <linux@roeck-us.net>
> ---
> This is probably a 4.4 candidate
>
> drivers/mtd/mtdcore.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index b1eea48c501d..a91cee90aef9 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -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);
> --
> 2.6.0.rc2.230.g3dd15c0
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mtd: don't WARN about overloaded users of mtd->reboot_notifier.notifier_call
2015-11-06 14:15 ` Guenter Roeck
@ 2015-11-06 17:35 ` Brian Norris
0 siblings, 0 replies; 5+ messages in thread
From: Brian Norris @ 2015-11-06 17:35 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-mtd, Richard Weinberger, linux-cris-kernel, Jesper Nilsson
On Fri, Nov 06, 2015 at 06:15:37AM -0800, Guenter Roeck wrote:
> On Tue, Nov 03, 2015 at 05:01:53PM -0800, Brian Norris wrote:
> > 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 <linux@roeck-us.net>
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> > Cc: Jesper Nilsson <jespern@axis.com>
> > Cc: linux-cris-kernel@axis.com
>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
Thanks! Pushed to l2-mtd.git.
I'll be queueing up the 4.4-rc1 pull request soon.
Brian
^ permalink raw reply [flat|nested] 5+ messages in thread