Re: [PATCH 1/4]Diskdump Update

[Arjan van de Ven <arjanv@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]


> diff -Nur linux-2.6.6.org/arch/i386/kernel/traps.c linux-2.6.6/arch/i386/kernel/traps.c
> --- linux-2.6.6.org/arch/i386/kernel/traps.c	2004-05-10 11:32:02.000000000 +0900
> +++ linux-2.6.6/arch/i386/kernel/traps.c	2004-06-09 19:17:46.000000000 +0900
> @@ -258,7 +258,8 @@
>  	int nl = 0;
>  
>  	console_verbose();
> -	spin_lock_irq(&die_lock);
> +	if (!crashdump_mode())
> +		spin_lock_irq(&die_lock);


why is this??


> +#ifndef TRUE
> +#define TRUE	1
> +#endif
> +#ifndef FALSE
> +#define FALSE	0
> +#endif

it's kernel convention to just use 0 and !0 

> +MODULE_PARM(fallback_on_err, "i");
> +MODULE_PARM(allow_risky_dumps, "i");
> +MODULE_PARM(block_order, "i");
> +MODULE_PARM(sample_rate, "i");

please exclusively use the 2.6 module parameter mechanism for new code.


> +		page = mem_map + nr;

this is not safe for discontig mem and thus broken


> +	/*
> +	 * Check the checksum of myself
> +	 */
> +	spin_trylock(&disk_dump_lock);

you need to handle the failure case of trylock of course


> +#ifdef CONFIG_PROC_FS
> +static int proc_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long param)


ehhh this looks evil


> diff -Nur linux-2.6.6.org/drivers/block/genhd.c linux-2.6.6/drivers/block/genhd.c
> --- linux-2.6.6.org/drivers/block/genhd.c	2004-05-10 11:32:29.000000000 +0900
> +++ linux-2.6.6/drivers/block/genhd.c	2004-06-09 19:17:46.000000000 +0900
> @@ -224,6 +224,8 @@
>  	return  kobj ? to_disk(kobj) : NULL;
>  }
>  
> +EXPORT_SYMBOL(get_gendisk);


this is WRONG. VERY WRONG.





[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]