Re: [PATCH v2 03/46] mtd: Don't unconditionally unregister reboot notifier

From: Brian Norris
Date: Sun Oct 09 2016 - 01:24:22 EST


I realize I didn't comment on the latest copy of this patch, so copying
my questions here:

On Wed, Sep 21, 2016 at 11:45:12AM +0200, Daniel Walter wrote:
> From: Richard Weinberger <richard@xxxxxx>
>
> del_mtd_device() is allowed to fail.
> i.e. when the MTD is busy.
> Unregister the reboot notifier only when we're really
> about to delete the MTD.
>
> Signed-off-by: Richard Weinberger <richard@xxxxxx>
> ---
> drivers/mtd/mtdcore.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index e3936b8..36e5fb0 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -654,17 +654,22 @@ int mtd_device_unregister(struct mtd_info *master)
> {
> int err;
>
> - if (master->_reboot)
> - unregister_reboot_notifier(&master->reboot_notifier);
> -
> err = del_mtd_partitions(master);
> if (err)
> return err;
>
> if (!device_is_registered(&master->dev))
> - return 0;
> + goto unregister;
>
> - return del_mtd_device(master);
> + err = del_mtd_device(master);
> + if (err)
> + return err;
> +
> +unregister:
> + if (master->_reboot)
> + unregister_reboot_notifier(&master->reboot_notifier);

Is there any kind of race issue with unregistering the notifier *after*
we've deleted the device? I had intentionally unregistered first,
because I didn't want any chance of the driver/module and/or data
structures being freed before we call the notifier.

I can't think of any particular issue yet, but I wanted to ask.

Brian

> + return 0;
> }
> EXPORT_SYMBOL_GPL(mtd_device_unregister);
>
> --
> 2.8.3
>