| Message ID | 1462316319-4308-1-git-send-email-peter@lekensteyn.nl (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
On 3 May 2016 at 23:58, Peter Wu <peter@lekensteyn.nl> wrote: > While waiting for a gdb response, or while sending an acknowledgement > there is not much to do, so just mark the socket as non-blocking to > avoid a busy loop while paused at gdb. This only affects the user-mode > emulation (qemu-arm -g 1234 ./a.out). > > Note that this issue was reported before at > https://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02277.html. > > While at it, close the gdb client fd on EOF or error while reading. The commit message says "mark the socket as non-blocking"... > @@ -1599,8 +1603,6 @@ static void gdb_accept(void) > gdb_has_xml = false; > > gdbserver_state = s; > - > - fcntl(fd, F_SETFL, O_NONBLOCK); > } ...but the code change is *removing* a call to mark the socket as non-blocking. Which is correct? thanks -- PMM
On Thu, May 05, 2016 at 04:37:40PM +0100, Peter Maydell wrote: > On 3 May 2016 at 23:58, Peter Wu <peter@lekensteyn.nl> wrote: > > While waiting for a gdb response, or while sending an acknowledgement > > there is not much to do, so just mark the socket as non-blocking to > > avoid a busy loop while paused at gdb. This only affects the user-mode > > emulation (qemu-arm -g 1234 ./a.out). > > > > Note that this issue was reported before at > > https://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02277.html. > > > > While at it, close the gdb client fd on EOF or error while reading. > > The commit message says "mark the socket as non-blocking"... > > > @@ -1599,8 +1603,6 @@ static void gdb_accept(void) > > gdb_has_xml = false; > > > > gdbserver_state = s; > > - > > - fcntl(fd, F_SETFL, O_NONBLOCK); > > } > > ...but the code change is *removing* a call to mark the > socket as non-blocking. Which is correct? > > thanks > -- PMM The commit message is misleading, it should have been "so do not mark the socket as non-blocking". If you were to apply this, please fix it up :-)
On 3 May 2016 at 23:58, Peter Wu <peter@lekensteyn.nl> wrote: > While waiting for a gdb response, or while sending an acknowledgement > there is not much to do, so just mark the socket as non-blocking to > avoid a busy loop while paused at gdb. This only affects the user-mode > emulation (qemu-arm -g 1234 ./a.out). > > Note that this issue was reported before at > https://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02277.html. > > While at it, close the gdb client fd on EOF or error while reading. > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > --- > gdbstub.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index 0e431fd..b126bf5 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -332,7 +332,7 @@ static int get_char(GDBState *s) > if (ret < 0) { > if (errno == ECONNRESET) > s->fd = -1; > - if (errno != EINTR && errno != EAGAIN) > + if (errno != EINTR) > return -1; > } else if (ret == 0) { > close(s->fd); > @@ -393,7 +393,7 @@ static void put_buffer(GDBState *s, const uint8_t *buf, int len) > while (len > 0) { > ret = send(s->fd, buf, len, 0); > if (ret < 0) { > - if (errno != EINTR && errno != EAGAIN) > + if (errno != EINTR) > return; > } else { > buf += ret; > @@ -1542,9 +1542,13 @@ gdb_handlesig(CPUState *cpu, int sig) > for (i = 0; i < n; i++) { > gdb_read_byte(s, buf[i]); > } > - } else if (n == 0 || errno != EAGAIN) { > + } else { > /* XXX: Connection closed. Should probably wait for another > connection before continuing. */ > + if (n == 0) { > + close(s->fd); > + } > + s->fd = -1; > return sig; > } > } > @@ -1599,8 +1603,6 @@ static void gdb_accept(void) > gdb_has_xml = false; > > gdbserver_state = s; > - > - fcntl(fd, F_SETFL, O_NONBLOCK); > } > > static int gdbserver_open(int port) It does look like all the places we use s->fd: * are only for user-mode * are currently coded as "loop round until we get something", so effectively blocking but doing it with a busy loop I suspect the O_NONBLOCK here may be legacy from before the system-emulation gdbstub was reworked to use a chr backend (in system emulation there really are other things we need to attend to besides gdb stub packets, like the monitor). Other than the error in the commit message, Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/gdbstub.c b/gdbstub.c index 0e431fd..b126bf5 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -332,7 +332,7 @@ static int get_char(GDBState *s) if (ret < 0) { if (errno == ECONNRESET) s->fd = -1; - if (errno != EINTR && errno != EAGAIN) + if (errno != EINTR) return -1; } else if (ret == 0) { close(s->fd); @@ -393,7 +393,7 @@ static void put_buffer(GDBState *s, const uint8_t *buf, int len) while (len > 0) { ret = send(s->fd, buf, len, 0); if (ret < 0) { - if (errno != EINTR && errno != EAGAIN) + if (errno != EINTR) return; } else { buf += ret; @@ -1542,9 +1542,13 @@ gdb_handlesig(CPUState *cpu, int sig) for (i = 0; i < n; i++) { gdb_read_byte(s, buf[i]); } - } else if (n == 0 || errno != EAGAIN) { + } else { /* XXX: Connection closed. Should probably wait for another connection before continuing. */ + if (n == 0) { + close(s->fd); + } + s->fd = -1; return sig; } } @@ -1599,8 +1603,6 @@ static void gdb_accept(void) gdb_has_xml = false; gdbserver_state = s; - - fcntl(fd, F_SETFL, O_NONBLOCK); } static int gdbserver_open(int port)
While waiting for a gdb response, or while sending an acknowledgement there is not much to do, so just mark the socket as non-blocking to avoid a busy loop while paused at gdb. This only affects the user-mode emulation (qemu-arm -g 1234 ./a.out). Note that this issue was reported before at https://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg02277.html. While at it, close the gdb client fd on EOF or error while reading. Signed-off-by: Peter Wu <peter@lekensteyn.nl> --- gdbstub.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)