Messages in this thread | | | Subject | Re: [Fastboot] Re: [PATCH][2/6]Memory preserving reboot using kexec | From | (Eric W. Biederman) | Date | 19 Sep 2004 14:37:18 -0600 |
| |
Hariprasad Nellitheertha <hari@in.ibm.com> writes:
> Regards, Hari > -- > Hariprasad Nellitheertha > Linux Technology Center > India Software Labs > IBM India, Bangalore > > > > This patch contains the code that does the memory preserving reboot. It > copies over the first 640k into a backup region before handing over to > kexec. The second kernel will boot using only the backup region.
Do you know what the kernel does with the low 1M?
Nothing in the hardware architecture requires us to use the low 1M. So I think we would be safer if we could track down and remove this dependency.
In general I agree that we need to be prepared to save some of the original machine state, because some architectures give special meaning to addresses in memory. But x86 is not one of those.
Perhaps the proper abstraction is to add a use_mem= variable that simply tells us which memory addresses we can use.
By still doing some copying we run into the problem, of potentially running out of memory areas where ongoing DMA transfers may be happening. So this is worth tracking down.
> diff -puN /dev/null include/linux/crash_dump.h > --- /dev/null 2003-01-30 15:54:37.000000000 +0530 > +++ linux-2.6.9-rc1-hari/include/linux/crash_dump.h 2004-09-15 > 17:36:30.000000000 +0530 > > @@ -0,0 +1,28 @@ > +#include <linux/kexec.h> > +#include <linux/smp_lock.h> > +#include <linux/device.h> > +#include <asm/crash_dump.h> > + > +#ifdef CONFIG_CRASH_DUMP
Why is this function an inline in a header file? I know we only call it once but still unnecessary code in a header file seems silly.
> +extern int crash_dump_on; > +static inline void crash_machine_kexec(void) > +{ > + struct kimage *image; > + > + if ((!crash_dump_on) || (crashed)) > + return; > + > + image = xchg(&kexec_image, 0);
You are still not using a different global variable here.
With a separate kexec_crash_image variable if the image is present we do a crash dump. That should allow you to remove the crash_dump_on variable and the test above.
> + if (image) { > + crashed = 1; We should not need the magic global variable crashed. We can just call the one or two functions needed from crash_machine_kexec.
> + device_shutdown(); Why are you calling device_shutdown here? > + printk(KERN_EMERG "kexec: opening parachute\n");
To wrap things prettily we should probably add a machine_crash_shutdown() and place in machine_crash_shutdown whatever architecture specific magic needs to happen.
> + machine_kexec(image); > + while (1);
The while look is unnecessary machine_kexec cannot return.
> + } else { > + printk(KERN_EMERG "kexec: No kernel image loaded!\n"); > + } > +} > +#else > +#define crash_machine_kexec() do { } while(0) > +#endif - 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/
| |