Xen devel - Dec 2007 - [PATCH][2 of 3] GDB serial port debugging: Respect opt_watchdog flag when resuming a debug session

Hello,

When trap_to_gdb in common/gdbstub.c returns, it calls watchdog_enable().
However, the watchdog functionality is an option controlled in
arch/x86/setup.c using the opt_watchdog boot parameter, and currently
defaults to ''off''.  When the user resumes from a gdb session
over the serial
port, the watchdog will be enabled regardless of the
''opt_watchdog'' flag.
This patch addresses this issue by propagating the optional parameter to
arch/x86/nmi.c and conditionally enabling/disabling the watchdog based on
the value of the now global ''opt_watchdog'' flag.


% diffstat opt_watchdog_patch
 nmi.c   |   11 ++++++++++-
 setup.c |    5 ++---
 2 files changed, 12 insertions(+), 4 deletions(-)

Signed-off-by:  Dan Doucette <doucette.daniel@gmail.com>

Dan.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

This shouldn¹t be true. If !opt_watchdog then setup.c will not call
watchdog_enable() and watchdog_disable_count should be forever non-zero
(because it is initialised to 1, and noone else should ever
watchdog_enable() if they haven¹t previously done watchdog_disable()).

Did you make this change because you saw problems, or just because the code
appeared broken?

By the way, I already applied the serial fix, and I¹ll hold off on the main
patch pending a new one that makes more of the code arch-generic as
suggested by Isaku Yamahata.

 Thanks,
 Keir

On 19/12/07 22:45, "Dan Doucette" <doucette.daniel@gmail.com>
wrote:
> Hello,
>  
> When trap_to_gdb in common/gdbstub.c returns, it calls watchdog_enable().
> However, the watchdog functionality is an option controlled in
> arch/x86/setup.c using the opt_watchdog boot parameter, and currently
defaults
> to ''off''.  When the user resumes from a gdb session over
the serial port, the
> watchdog will be enabled regardless of the ''opt_watchdog''
flag.  This patch
> addresses this issue by propagating the optional parameter to
arch/x86/nmi.c
> and conditionally enabling/disabling the watchdog based on the value of the
> now global ''opt_watchdog'' flag.
>  
>  
> % diffstat opt_watchdog_patch
>  nmi.c   |   11 ++++++++++-
>  setup.c |    5 ++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>  
> Signed-off-by:  Dan Doucette <doucette.daniel@gmail.com>
>  
> Dan.
>  
>  
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I made this change when a number of things were not working, because it
appeared broken as you said.
I never backed out the change and re-tested.

I agree, if watchdog_enable/disable are always called in even numbers, and
enable is not called initially, it should be fine.

Dan.
On Dec 20, 2007 2:38 AM, Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:
> This shouldn''t be true. If !opt_watchdog then setup.c will not
call
> watchdog_enable() and watchdog_disable_count should be forever non-zero
> (because it is initialised to 1, and noone else should ever
> watchdog_enable() if they haven''t previously done
watchdog_disable()).
>
> Did you make this change because you saw problems, or just because the
> code appeared broken?
>
> By the way, I already applied the serial fix, and I''ll hold off on
the
> main patch pending a new one that makes more of the code arch-generic as
> suggested by Isaku Yamahata.
>
>  Thanks,
>  Keir
>
>
> On 19/12/07 22:45, "Dan Doucette"
<doucette.daniel@gmail.com> wrote:
>
>   Hello,
>
> When trap_to_gdb in common/gdbstub.c returns, it calls watchdog_enable().
>  However, the watchdog functionality is an option controlled in
> arch/x86/setup.c using the opt_watchdog boot parameter, and currently
> defaults to ''off''.  When the user resumes from a gdb
session over the serial
> port, the watchdog will be enabled regardless of the
''opt_watchdog'' flag.
>  This patch addresses this issue by propagating the optional parameter to
> arch/x86/nmi.c and conditionally enabling/disabling the watchdog based on
> the value of the now global ''opt_watchdog'' flag.
>
>
> % diffstat opt_watchdog_patch
>  nmi.c   |   11 ++++++++++-
>  setup.c |    5 ++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> Signed-off-by:  Dan Doucette <doucette.daniel@gmail.com>
>
> Dan.
>
>
>
> ------------------------------
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel