[commit] gdbserver async I/O simplifications

[commit] gdbserver async I/O simplifications

[Daniel Jacobowitz]

For the patch I'm working on, I needed asynchronous I/O to be disabled
during start_inferior and it was ending up enabled.  This version is
considerably simpler, and does not have that problem.  Async I/O can
and should be enabled once we have received a packet that will take a
long time, and must be disabled before we reply to GDB about it.

Tested on x86_64-linux and committed.

-- 
Daniel Jacobowitz
CodeSourcery

2007-12-06  Daniel Jacobowitz  <dan@codesourcery.com>

	* linux-low.c (linux_wait, linux_resume): Do not handle async I/O.
	* remote-utils.c (remote_open): Do not call disable_async_io.
	(block_async_io): Delete.
	(unblock_async_io): Make static.
	(initialize_async_io): New.
	* server.c (handle_v_cont): Handle async I/O here.
	(myresume): Likewise.  Move other common resume tasks here...
	(main): ... from here.  Call initialize_async_io.  Disable async
	I/O before the main loop.
	* server.h (initialize_async_io): Declare.
	(block_async_io, unblock_async_io): Delete prototypes.
	* spu-low.c (spu_resume, spu_wait): Do not handle async I/O here.

---
 gdb/gdbserver/linux-low.c    |    9 +--------
 gdb/gdbserver/remote-utils.c |   26 +++++++++++++-------------
 gdb/gdbserver/server.c       |   35 ++++++++++++++++++-----------------
 gdb/gdbserver/server.h       |    3 +--
 gdb/gdbserver/spu-low.c      |    8 --------
 5 files changed, 33 insertions(+), 48 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2007-12-06 10:43:44.000000000 -0500
+++ src/gdb/gdbserver/linux-low.c	2007-12-06 10:56:23.000000000 -0500
@@ -897,11 +897,8 @@ retry:
 	}
     }
 
-  enable_async_io ();
-  unblock_async_io ();
   w = linux_wait_for_event (child);
   stop_all_processes ();
-  disable_async_io ();
 
   if (must_set_ptrace_flags)
     {
@@ -1313,11 +1310,7 @@ linux_resume (struct thread_resume *resu
   if (pending_flag)
     for_each_inferior (&all_threads, linux_queue_one_thread);
   else
-    {
-      block_async_io ();
-      enable_async_io ();
-      for_each_inferior (&all_threads, linux_continue_one_thread);
-    }
+    for_each_inferior (&all_threads, linux_continue_one_thread);
 }
 
 #ifdef HAVE_LINUX_USRREGS
Index: src/gdb/gdbserver/remote-utils.c
===================================================================
--- src.orig/gdb/gdbserver/remote-utils.c	2007-12-06 10:43:44.000000000 -0500
+++ src/gdb/gdbserver/remote-utils.c	2007-12-06 14:24:20.000000000 -0500
@@ -275,7 +275,6 @@ remote_open (char *name)
   fcntl (remote_desc, F_SETOWN, getpid ());
 #endif
 #endif
-  disable_async_io ();
 }
 
 void
@@ -645,22 +644,12 @@ check_remote_input_interrupt_request (vo
    accept Control-C from the client, and must be disabled when talking to
    the client.  */
 
-void
-block_async_io (void)
-{
-#ifndef USE_WIN32API
-  sigset_t sigio_set;
-  sigemptyset (&sigio_set);
-  sigaddset (&sigio_set, SIGIO);
-  sigprocmask (SIG_BLOCK, &sigio_set, NULL);
-#endif
-}
-
-void
+static void
 unblock_async_io (void)
 {
 #ifndef USE_WIN32API
   sigset_t sigio_set;
+
   sigemptyset (&sigio_set);
   sigaddset (&sigio_set, SIGIO);
   sigprocmask (SIG_UNBLOCK, &sigio_set, NULL);
@@ -696,6 +685,17 @@ disable_async_io (void)
   async_io_enabled = 0;
 }
 
+void
+initialize_async_io (void)
+{
+  /* Make sure that async I/O starts disabled.  */
+  async_io_enabled = 1;
+  disable_async_io ();
+
+  /* Make sure the signal is unblocked.  */
+  unblock_async_io ();
+}
+
 /* Returns next char from remote GDB.  -1 if error.  */
 
 static int
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2007-12-06 10:43:44.000000000 -0500
+++ src/gdb/gdbserver/server.c	2007-12-06 10:56:23.000000000 -0500
@@ -756,12 +756,14 @@ handle_v_cont (char *own_buf, char *stat
     cont_thread = -1;
   set_desired_inferior (0);
 
+  enable_async_io ();
   (*the_target->resume) (resume_info);
 
   free (resume_info);
 
   *signal = mywait (status, 1);
   prepare_resume_reply (own_buf, *status, *signal);
+  disable_async_io ();
   return;
 
 err:
@@ -798,10 +800,13 @@ handle_v_requests (char *own_buf, char *
 }
 
 void
-myresume (int step, int sig)
+myresume (int step, int *signalp, char *statusp)
 {
   struct thread_resume resume_info[2];
   int n = 0;
+  int sig = *signalp;
+
+  set_desired_inferior (0);
 
   if (step || sig || (cont_thread != 0 && cont_thread != -1))
     {
@@ -817,7 +822,11 @@ myresume (int step, int sig)
   resume_info[n].sig = 0;
   resume_info[n].leave_stopped = (cont_thread != 0 && cont_thread != -1);
 
+  enable_async_io ();
   (*the_target->resume) (resume_info);
+  *signalp = mywait (statusp, 1);
+  prepare_resume_reply (own_buf, *statusp, *signalp);
+  disable_async_io ();
 }
 
 static int attached;
@@ -895,6 +904,7 @@ main (int argc, char *argv[])
       exit (1);
     }
 
+  initialize_async_io ();
   initialize_low ();
 
   own_buf = malloc (PBUFSIZ + 1);
@@ -946,6 +956,7 @@ main (int argc, char *argv[])
 
     restart:
       setjmp (toplevel);
+      disable_async_io ();
       while (1)
 	{
 	  unsigned char sig;
@@ -1073,10 +1084,7 @@ main (int argc, char *argv[])
 		signal = target_signal_to_host (sig);
 	      else
 		signal = 0;
-	      set_desired_inferior (0);
-	      myresume (0, signal);
-	      signal = mywait (&status, 1);
-	      prepare_resume_reply (own_buf, status, signal);
+	      myresume (0, &signal, &status);
 	      break;
 	    case 'S':
 	      convert_ascii_to_int (own_buf + 1, &sig, 1);
@@ -1084,22 +1092,15 @@ main (int argc, char *argv[])
 		signal = target_signal_to_host (sig);
 	      else
 		signal = 0;
-	      set_desired_inferior (0);
-	      myresume (1, signal);
-	      signal = mywait (&status, 1);
-	      prepare_resume_reply (own_buf, status, signal);
+	      myresume (1, &signal, &status);
 	      break;
 	    case 'c':
-	      set_desired_inferior (0);
-	      myresume (0, 0);
-	      signal = mywait (&status, 1);
-	      prepare_resume_reply (own_buf, status, signal);
+	      signal = 0;
+	      myresume (0, &signal, &status);
 	      break;
 	    case 's':
-	      set_desired_inferior (0);
-	      myresume (1, 0);
-	      signal = mywait (&status, 1);
-	      prepare_resume_reply (own_buf, status, signal);
+	      signal = 0;
+	      myresume (1, &signal, &status);
 	      break;
 	    case 'Z':
 	      {
Index: src/gdb/gdbserver/server.h
===================================================================
--- src.orig/gdb/gdbserver/server.h	2007-12-06 10:43:44.000000000 -0500
+++ src/gdb/gdbserver/server.h	2007-12-06 10:44:06.000000000 -0500
@@ -171,10 +171,9 @@ void remote_open (char *name);
 void remote_close (void);
 void write_ok (char *buf);
 void write_enn (char *buf);
+void initialize_async_io (void);
 void enable_async_io (void);
 void disable_async_io (void);
-void unblock_async_io (void);
-void block_async_io (void);
 void check_remote_input_interrupt_request (void);
 void convert_ascii_to_int (char *from, unsigned char *to, int n);
 void convert_int_to_ascii (unsigned char *from, char *to, int n);
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2007-12-06 10:43:44.000000000 -0500
+++ src/gdb/gdbserver/spu-low.c	2007-12-06 10:44:06.000000000 -0500
@@ -348,9 +348,6 @@ spu_resume (struct thread_resume *resume
 	 && resume_info->thread != current_tid)
     resume_info++;
 
-  block_async_io ();
-  enable_async_io ();
-
   if (resume_info->leave_stopped)
     return;
 
@@ -375,9 +372,6 @@ spu_wait (char *status)
   int w;
   int ret;
 
-  enable_async_io ();
-  unblock_async_io ();
-
   while (1)
     {
       ret = waitpid (tid, &w, WNOHANG | __WALL | __WNOTHREAD);
@@ -407,8 +401,6 @@ spu_wait (char *status)
 	}
     }
 
-  disable_async_io ();
-
   if (WIFEXITED (w))
     {
       fprintf (stderr, "\nChild exited with retcode = %x \n", WEXITSTATUS (w));

Re: [commit] gdbserver async I/O simplifications

[Markus Deuling]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

Hi Daniel,

Daniel Jacobowitz schrieb:
> For the patch I'm working on, I needed asynchronous I/O to be disabled
> during start_inferior and it was ending up enabled.  This version is
> considerably simpler, and does not have that problem.  Async I/O can
> and should be enabled once we have received a packet that will take a
> long time, and must be disabled before we reply to GDB about it.
> 
> Tested on x86_64-linux and committed.
> 

it seems own_buf is missing in gdbserver/server.c (myresume). This fixes the build
but I havent tested it. Its just what I applied to my dev tree.

ChangeLog:

	* gdbserver/server.c (myresume): Add own_buf variable.


-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

	

[-- Attachment #2: fix-gdbsrv --]
[-- Type: text/plain, Size: 504 bytes --]

diff -urpN src/gdb/gdbserver/server.c dev/gdb/gdbserver/server.c
--- src/gdb/gdbserver/server.c	2007-12-07 06:32:33.000000000 +0100
+++ dev/gdb/gdbserver/server.c	2007-12-07 07:56:54.000000000 +0100
@@ -803,9 +803,11 @@ void
 myresume (int step, int *signalp, char *statusp)
 {
   struct thread_resume resume_info[2];
+  char *own_buf;
   int n = 0;
   int sig = *signalp;
 
+  own_buf = malloc (PBUFSIZ + 1);
   set_desired_inferior (0);
 
   if (step || sig || (cont_thread != 0 && cont_thread != -1))

Re: [commit] gdbserver async I/O simplifications

[Daniel Jacobowitz]

On Fri, Dec 07, 2007 at 08:00:55AM +0100, Markus Deuling wrote:
> Hi Daniel,
> 
> Daniel Jacobowitz schrieb:
> > For the patch I'm working on, I needed asynchronous I/O to be disabled
> > during start_inferior and it was ending up enabled.  This version is
> > considerably simpler, and does not have that problem.  Async I/O can
> > and should be enabled once we have received a packet that will take a
> > long time, and must be disabled before we reply to GDB about it.
> > Tested on x86_64-linux and committed.
> 
> it seems own_buf is missing in gdbserver/server.c (myresume). This fixes the 
> build
> but I havent tested it. Its just what I applied to my dev tree.
> 
> ChangeLog:
> 
> 	* gdbserver/server.c (myresume): Add own_buf variable.

I have apparently failed to test the patch I thought I was testing.
Fix checked in as below; tested, this time.

-- 
Daniel Jacobowitz
CodeSourcery

2007-12-07  Daniel Jacobowitz  <dan@codesourcery.com>

	* server.c (myresume): Add own_buf argument.
	(main): Update calls.

Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.60
diff -u -p -r1.60 server.c
--- server.c	7 Dec 2007 01:41:29 -0000	1.60
+++ server.c	7 Dec 2007 13:47:09 -0000
@@ -800,7 +800,7 @@ handle_v_requests (char *own_buf, char *
 }
 
 void
-myresume (int step, int *signalp, char *statusp)
+myresume (char *own_buf, int step, int *signalp, char *statusp)
 {
   struct thread_resume resume_info[2];
   int n = 0;
@@ -1084,7 +1084,7 @@ main (int argc, char *argv[])
 		signal = target_signal_to_host (sig);
 	      else
 		signal = 0;
-	      myresume (0, &signal, &status);
+	      myresume (own_buf, 0, &signal, &status);
 	      break;
 	    case 'S':
 	      convert_ascii_to_int (own_buf + 1, &sig, 1);
@@ -1092,15 +1092,15 @@ main (int argc, char *argv[])
 		signal = target_signal_to_host (sig);
 	      else
 		signal = 0;
-	      myresume (1, &signal, &status);
+	      myresume (own_buf, 1, &signal, &status);
 	      break;
 	    case 'c':
 	      signal = 0;
-	      myresume (0, &signal, &status);
+	      myresume (own_buf, 0, &signal, &status);
 	      break;
 	    case 's':
 	      signal = 0;
-	      myresume (1, &signal, &status);
+	      myresume (own_buf, 1, &signal, &status);
 	      break;
 	    case 'Z':
 	      {