Re: [PATCH][RFC] vmcore: Remove saved_max_pfn check

[Vivek Goyal <vgoyal@redhat.com>]

On Mon, Sep 08, 2008 at 12:21:32PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@igel.co.jp>
> 
> This patch removes the saved_max_pfn check from the /proc/vmcore
> function read_from_oldmem(). No need to verify, we should be able
> to just trust that "elfcorehdr=" is correctly passed to the crash
> kernel on the kernel command line like we do with other parameters.
> 
> The read_from_oldmem() function in fs/proc/vmcore.c is quite similar
> to read_from_oldmem() in drivers/char/mem.c, but only in the latter
> it makes sense to use saved_max_pfn. For oldmem it is used to determine
> when to stop reading. For vmcore we already have the elf header info
> pointing out the physical memory regions, no need to pass the end-of-
> old-memory twice.
> 
> Removing the saved_max_pfn check from vmcore makes it possible for
> architectures to skip oldmem but still support crash dump through
> vmcore - without the need for the old saved_max_pfn cruft.
> 
> Architectures that want to play safe can do the saved_max_pfn check
> in copy_oldmem_page(). Not sure why anyone would want to do that,
> but that's even safer than today - the saved_max_pfn check in vmcore
> removed by this patch only checks the first page.
> 

Hi Magnus,

Though I don't feel very strongly for saved_max_pfn check in vmcore.c,
but at the same time I don't understand what are you gaining by removing
this check. Any way we are not getting rid of this symbol altogether
because /dev/oldmem needs it.

Is it sh arch for which you want to disable /dev/oldmem and only enable
/proc/vmcore and hence want to get rid of saved_max_pfn?

Though I agree that we should elfcorehdrs but at the same time it does not
hard doing additional check (We are anyway carrying saved_max_pfn for
/dev/oldmem). We can always extend current code to check for end page also
to make sure we are not reading beyond saved_max_pfn.

How much code is it to set value of saved_max_pfn in sh that you want to
completely get rid of it. My feeling is that it should be just few lines.

So though I don't feel strongly for saved_max_pfn check  in vmcore.c, at
the same time I don't see you are gaining anything significant by removing
it. We can just introduce saved_max_pfn in sh also.

Thanks
Vivek
 


> Signed-off-by: Magnus Damm <damm@igel.co.jp>
> ---
> 
>  fs/proc/vmcore.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> --- 0001/fs/proc/vmcore.c
> +++ work/fs/proc/vmcore.c	2008-09-08 11:33:10.000000000 +0900
> @@ -50,8 +50,6 @@ static ssize_t read_from_oldmem(char *bu
>  
>  	offset = (unsigned long)(*ppos % PAGE_SIZE);
>  	pfn = (unsigned long)(*ppos / PAGE_SIZE);
> -	if (pfn > saved_max_pfn)
> -		return -EINVAL;
>  
>  	do {
>  		if (count > (PAGE_SIZE - offset))

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec