| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5480E27A.8080004@broadcom.com>
Date: Thu, 4 Dec 2014 14:38:50 -0800
From: Scott Branden <sbranden@...adcom.com>
To: Brian Norris <computersforpeace@...il.com>
CC: David Woodhouse <dwmw2@...radead.org>, Ray Jui <rjui@...adcom.com>,
Corneliu Doban <cdoban@...adcom.com>,
<linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
Richard Weinberger <richard@....at>
Subject: Re: [PATCH] mtd: nand: added nand_shutdown
Hi Brian,
I just sent out a new patch with your changes incorporated.
Thanks,
Scott
On 14-11-26 01:10 AM, Brian Norris wrote:
> On Thu, Nov 20, 2014 at 11:18:05AM -0800, Scott Branden wrote:
>> Add nand_shutdown to wait for current nand operations to finish and prevent
>> further operations by changing the nand flash state to FL_SHUTDOWN.
>>
>> This is addressing a problem observed during reboot tests using UBIFS
>> root file system: NAND erase operations that are in progress during
>> system reboot/shutdown are causing partial erased blocks. Although UBI should
>> be able to detect and recover from this error, this change will avoid
>> the creation of partial erased blocks on reboot in the middle of a NAND erase
>> operation.
> [...]
>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>> index e4d451e..80e4367 100644
>> --- a/include/linux/mtd/nand.h
>> +++ b/include/linux/mtd/nand.h
>> @@ -48,6 +48,13 @@ extern int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>> /* unlocks specified locked blocks */
>> extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>>
>> +/*
>> + * Internal helper for board drivers which need to make sure that the current
>> + * nand operation is finished and further operations are prevented before
>> + * rebooting the system.
>> + */
>> +extern int nand_shutdown(struct mtd_info *mtd);
>
> I don't think we should push this out to the board drivers. And I
> definitely don't want to merge this function without any user. It'd turn
> out like our useless nand_lock() and nand_unlock() functions.
>
>> +
>> /* The maximum number of NAND chips in an array */
>> #define NAND_MAX_CHIPS 8
>>
>
> So, to do this right, I think maybe we should add an (optional)
> reboot_notifier field to struct mtd_info, and use it to register a
> system reboot notifier on behalf of the lower layers. We can do the
> registration in mtd_device_parse_register(), I think, and any
> unregistration in mtd_device_unregister().
>
> With this approach, we can actually move the cfi_cmdset_000{1,2}.c
> reboot notifiers over to the same common code. They just will have to
> fill out their mtd->reboot_notifier field.
>
> How about the following two untested patches?
>
> From: Brian Norris <computersforpeace@...il.com>
>
> cfi_cmdset_000{1,2}.c already implement their own reboot notifiers, and
> we're going to add one for NAND. Let's put the boilerplate in one place.
>
> Signed-off-by: Brian Norris <computersforpeace@...il.com>
> ---
> drivers/mtd/mtdcore.c | 20 ++++++++++++++++++++
> include/linux/mtd/mtd.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 4c611871d7e6..b80d44f9751d 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -37,6 +37,7 @@
> #include <linux/backing-dev.h>
> #include <linux/gfp.h>
> #include <linux/slab.h>
> +#include <linux/reboot.h>
>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
> @@ -365,6 +366,17 @@ static struct device_type mtd_devtype = {
> .release = mtd_release,
> };
>
> +static int mtd_reboot_notifier(struct notifier_block *n, unsigned long state,
> + void *cmd)
> +{
> + struct mtd_info *mtd;
> +
> + mtd = container_of(n, struct mtd_info, reboot_notifier);
> + mtd->_reboot(mtd);
> +
> + return NOTIFY_DONE;
> +}
> +
> /**
> * add_mtd_device - register an MTD device
> * @mtd: pointer to new MTD device info structure
> @@ -565,6 +577,11 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
> err = -ENODEV;
> }
>
> + if (mtd->_reboot) {
> + mtd->reboot_notifier.notifier_call = mtd_reboot_notifier;
> + register_reboot_notifier(&mtd->reboot_notifier);
> + }
> +
> return err;
> }
> EXPORT_SYMBOL_GPL(mtd_device_parse_register);
> @@ -579,6 +596,9 @@ 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;
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 031ff3a9a0bd..c06f5373d870 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -227,6 +227,7 @@ struct mtd_info {
> int (*_block_markbad) (struct mtd_info *mtd, loff_t ofs);
> int (*_suspend) (struct mtd_info *mtd);
> void (*_resume) (struct mtd_info *mtd);
> + void (*_reboot) (struct mtd_info *mtd);
> /*
> * If the driver is something smart, like UBI, it may need to maintain
> * its own reference counting. The below functions are only for driver.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists