lkml.org 
[lkml]   [2004]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH][2/6]Memory preserving reboot using kexec

I'll add these to -mm. Minor things:


> +config BACKUP_BASE
> + int "location of the crash dumps backup region"
> + depends on CRASH_DUMP
> + default 16
> + help
> + This is the location where the second kernel will boot from.
> +
> +config BACKUP_SIZE
> + int "Size of the crash dumps backup region"
> + depends on CRASH_DUMP
> + range 16 64
> + default 32
> + help
> + The size of the second kernel's memory.

You should tell the user the units of the parameter. So

"location of the crash dumps backup region (MB)"

would be nice.

> +++ linux-2.6.9-rc1-hari/include/asm-i386/crash_dump.h 2004-09-15 17:36:30.000000000 +0530
> @@ -0,0 +1,44 @@
> +/* asm-i386/crash_dump.h */
> +#include <linux/bootmem.h>
> +
> +extern unsigned int dump_enabled;
> +
> +void __crash_relocate_mem(unsigned long, unsigned long);
> +unsigned long __init find_max_low_pfn(void);
> +void __init find_max_pfn(void);
> +
> +extern unsigned int crashed;

Should the above declarations be inside CONFIG_CRASH_DUMP? We don't want
to leave them in scope if they don't exist.

> +static inline void crash_machine_kexec(void)
> +{
> + struct kimage *image;
> +
> + if ((!crash_dump_on) || (crashed))
> + return;
> +
> + image = xchg(&kexec_image, 0);
> + if (image) {
> + crashed = 1;
> + device_shutdown();
> + printk(KERN_EMERG "kexec: opening parachute\n");
> + machine_kexec(image);
> + while (1);
> + } else {
> + printk(KERN_EMERG "kexec: No kernel image loaded!\n");
> + }
> +}

I don't see why this is inlined??

> +#ifdef CONFIG_CRASH_DUMP
> +/*
> + * Enable kexec reboot upon panic; for dumping
> + */
> +static ssize_t write_crash_dump_on(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + if (count) {
> + if (get_user(crash_dump_on, buf))
> + return -EFAULT;
> + }
> + return count;
> +}
> +
> +static struct file_operations proc_crash_dump_on_operations = {
> + .write = write_crash_dump_on,
> +};
> +#endif
> +
> struct proc_dir_entry *proc_root_kcore;
>
> static void create_seq_entry(char *name, mode_t mode, struct file_operations *f)
> @@ -663,6 +683,11 @@ void __init proc_misc_init(void)
> if (entry)
> entry->proc_fops = &proc_sysrq_trigger_operations;
> #endif
> +#ifdef CONFIG_CRASH_DUMP
> + entry = create_proc_entry("kexec-dump", S_IWUSR, NULL);
> + if (entry)
> + entry->proc_fops = &proc_crash_dump_on_operations;
> +#endif
> #ifdef CONFIG_LOCKMETER
> entry = create_proc_entry("lockmeter", S_IWUSR | S_IRUGO, NULL);
> if (entry) {

Please do all the above in a crashdump-specific file, not inside proc_misc.c


-
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: 2005-03-22 14:06    [from the cache]
©2003-2011 Jasper Spaans