lkcd
[Top] [All Lists]

Re: [PATCH] fix for incomplete backtrace due to wrong esp value + some c

To: "Matt D. Robinson" <yakker@xxxxxxxxxxx>
Subject: Re: [PATCH] fix for incomplete backtrace due to wrong esp value + some cleanups
From: Bharata B Rao <bharata@xxxxxxxxxx>
Date: Thu, 4 Oct 2001 14:24:11 +0530
Cc: lkcd@xxxxxxxxxxx
In-reply-to: <Pine.LNX.4.30.0110032256210.10742-100000@nakedeye.aparity.com>; from yakker@aparity.com on Wed, Oct 03, 2001 at 10:57:57PM -0700
References: <20011003155756.A13611@in.ibm.com> <Pine.LNX.4.30.0110032256210.10742-100000@nakedeye.aparity.com>
Reply-to: bharata@xxxxxxxxxx
Sender: owner-lkcd@xxxxxxxxxxx
User-agent: Mutt/1.2.5i
This code is now checked into cvs.(dump_base.c and dump_i386.c)

Regards,
Bharata.

On Wed, Oct 03, 2001 at 10:57:57PM -0700, Matt D. Robinson wrote:
> 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);
> |>

-- 
Bharata B Rao,
IBM Linux Technology Center,
IBM Software Lab, Bangalore.

Ph: 91-80-5262355 Ex: 3962
Mail: bharata@xxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>