Go ahead and check it in, Bharata. Again, it was two years
ago, but the __asm__ required then to use a register for
moving %esp at one point, otherwise I wouldn't have used %ecx. :)
One more 'cvs update' ...
--Matt
On Wed, 3 Oct 2001, Bharata B Rao wrote:
|>Matt,
|>Here is a patch for dump_base.c and dump_i386.c which does the following:
|>
|>1. When dump is triggerred from kernel traps(and when regs pointer is valid),
|>the current method of getting esp value is not correct.
|>Currently, esp is collected as dha->dha_regs.esp = (unsigned long) (regs + 1);
|>Here you go down the stack by entire pt_regs structure. In case of kernel
|>mode traps, since stack switch doesn't happen, the esp and xss members of
|>pt_regs won't be filled up by the processor. So here you are going down the
|>stack by additional 8 bytes. This has been fixed in the patch below.
|>
|>I have seen that this eliminates the TRACE ERROR that is observed when
|>bt command is used in lcrash.
|>
|>2. Move the arch specific inline assembler code in dump_base.c to dump_i386.c
|>
|>3. Since collecting of esp/eip happens twice move it into
|>__dump_save_panic_regs(), which is called twice.
|>
|>Let me know if I can commit these into cvs.
|>
|>Regards,
|>Bharata.
|>--
|>
|>drivers/dump/dump_base.c
|>--- lkcd_cvs/2.4/drivers/dump/dump_base.c Wed Sep 26 15:29:25 2001
|>+++ dump_base.c Wed Oct 3 15:37:37 2001
|>@@ -912,16 +912,7 @@
|> * So for that reason, we save the eip/esp
|> * now so we can re-build the trace later.
|> */
|>- __asm__ __volatile__("
|>- pushl %%ecx\n
|>- movl %%esp, %%ecx\n
|>- movl %%ecx, %0\n
|>- popl %%ecx\n"
|>- : "=g" (dump_header_asm.dha_esp)
|>- );
|>- __asm__ __volatile__("pushl %ecx\n");
|> __dump_save_panic_regs(&dump_header_asm);
|>- __asm__ __volatile__("popl %ecx\n");
|> #endif
|>
|> /* update header to disk for the last time */
|>
|>
|>drivers/dump/dump_i386.c
|>--- lkcd_cvs/2.4/drivers/dump/dump_i386.c Tue Sep 25 10:47:03 2001
|>+++ dump_i386.c Wed Oct 3 15:35:34 2001
|>@@ -34,23 +34,20 @@
|> /*
|> * Name: __dump_save_panic_regs()
|> * Func: Save the EIP (really the RA). We may pass an argument later.
|>+ * Save ESP also here.
|> */
|>-void
|>+inline void
|> __dump_save_panic_regs(dump_header_asm_t *dha)
|> {
|>+ __asm__ __volatile__("movl %%esp, %0\n"
|>+ : "=r" (dha->dha_esp));
|> /* hate to do this, but ... */
|> #ifdef CONFIG_FRAME_POINTER
|>- __asm__ __volatile__("
|>- movl 4(%%esp), %%ecx\n
|>- movl %%ecx, %0\n"
|>- : "=g" (dha->dha_eip)
|>- );
|>+ __asm__ __volatile__("movl 4(%%esp), %0\n"
|>+ : "=r" (dha->dha_eip));
|> #else
|>- __asm__ __volatile__("
|>- movl (%%esp), %%ecx\n
|>- movl %%ecx, %0\n"
|>- : "=g" (dha->dha_eip)
|>- );
|>+ __asm__ __volatile__("movl (%%esp), %0\n"
|>+ : "=r" (dha->dha_eip));
|> #endif
|> }
|>
|>@@ -62,20 +59,11 @@
|> __dump_configure_header(dump_header_asm_t *dha, struct pt_regs *regs)
|> {
|> /* save the dump specific esp/eip */
|>- __asm__ __volatile__("
|>- pushl %%ecx\n
|>- movl %%esp, %%ecx\n
|>- movl %%ecx, %0\n
|>- popl %%ecx\n"
|>- : "=g" (dha->dha_esp)
|>- );
|>- __asm__ __volatile__("pushl %ecx\n");
|> __dump_save_panic_regs(dha);
|>- __asm__ __volatile__("popl %ecx\n");
|>
|> /* one final check -- modify if we're in user mode */
|> if ((regs) && (!user_mode(regs))) {
|>- dha->dha_regs.esp = (unsigned long) (regs + 1);
|>+ dha->dha_regs.esp = (unsigned long) &(regs->esp);
|> }
|>
|> return (1);
|>
|