Re: [PATCH][2/6]Memory preserving reboot using kexec

[Andrew Morton]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/