lkml.org 
[lkml]   [2009]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] MTD: Add UBI reboot notifier
From
Date
Looks good to me. I'm though wandering if UBIFS should also
register a reboot notifier an sync write-buffers in it...
But this is not related to your patch.

Some nit-picking.

On Mon, 2009-06-08 at 22:14 -0700, Kevin Cernekee wrote:
> /**
> + * ubi_reboot_notifier - halt UBI transactions immediately prior to a reboot
> + * @n: notifier_block struct (inside our struct ubi_device)
> + * @val: unused
> + * @v: unused
> + */
> +static int ubi_reboot_notifier(struct notifier_block *n, unsigned long val,
> + void *v)
> +{
> + struct ubi_device *ubi = container_of(n, struct ubi_device,
> + reboot_notifier);
> +
> + if (ubi->bgt_thread)
> + kthread_stop(ubi->bgt_thread);
> + return NOTIFY_DONE;
> +}

Several small things in comments and indentations. Take a look how
other UBI funcs do things, I'd be happier to keep the code consistent.

> +/**
> * ubi_attach_mtd_dev - attach an MTD device.
> * @mtd: MTD device description object
> * @ubi_num: number to assign to the new UBI device
> @@ -876,6 +894,11 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, int vid_hdr_offset)
> ubi->thread_enabled = 1;
> wake_up_process(ubi->bgt_thread);
>
> + /* Flash device priority is 0. UBI needs to shut down first. */
> + ubi->reboot_notifier.priority = 1;
> + ubi->reboot_notifier.notifier_call = ubi_reboot_notifier;
> + register_reboot_notifier(&ubi->reboot_notifier);
> +
> ubi_devices[ubi_num] = ubi;
> return ubi_num;
>
> @@ -945,6 +968,7 @@ int ubi_detach_mtd_dev(int ubi_num, int anyway)
> * Before freeing anything, we have to stop the background thread to
> * prevent it from doing anything on this device while we are freeing.
> */
> + unregister_reboot_notifier(&ubi->reboot_notifier);
> if (ubi->bgt_thread)
> kthread_stop(ubi->bgt_thread);
>
> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
> index c055511..44a45b9 100644
> --- a/drivers/mtd/ubi/ubi.h
> +++ b/drivers/mtd/ubi/ubi.h
> @@ -36,6 +36,7 @@
> #include <linux/device.h>
> #include <linux/string.h>
> #include <linux/vmalloc.h>
> +#include <linux/notifier.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/ubi.h>
>
> @@ -420,6 +421,7 @@ struct ubi_device {
> struct task_struct *bgt_thread;
> int thread_enabled;
> char bgt_name[sizeof(UBI_BGT_NAME_PATTERN)+2];
> + struct notifier_block reboot_notifier;

How about commenting this field above the struct ubi_device
definition?

--
Best regards,
Artem Bityutskiy (Битюцкий Артём)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-06-09 14:37    [W:0.143 / U:6.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site