Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:"

Gary Benson
2015-08-19T10:51:00Z

Sandra, could you please try this patch on your Altera 3c120 and
on your PandaBoard? I'm interested to know what the times are
now.

Cheers,
Gary

Gary Benson wrote:
> Pedro Alves wrote:
> > The fact that Gary's chunk size limiting patch made things much
> > faster on the nios2 board is still mysterious to me. I'd expect
> > the slowness to be latency bound, given the request/response
> > nature of the RSP, but then I'd expect that more chunking would
> > slow things down, not speed it up.
>
> I think I figured this out...
>
> While handling "vFile:pread:" packets, gdbserver would read the
> number of bytes requested regardless of whether this would fit
> into the reply packet. gdbserver would then return a packet's
> worth of data and discard the remainder. When accessing large
> binaries GDB (via BFD) routinely makes large "vFile:pread:"
> requests, resulting in gdbserver allocating large unnecessary
> buffers and reading some portions of the file many times over.
>
> This commit causes gdbserver to limit the number of bytes to be
> read to a sensible maximum prior to allocating buffers and reading
> data.
>
> Built and regtested on RHEL 6.6 x86_64.
>
> May I push this to HEAD and to the branch?
>
> Thanks,
> Gary
>
> ---
> gdb/gdbserver/ChangeLog:
>
> * hostio.c (handle_pread): Do not attempt to read more data
> than hostio_reply_with_data can fit in a packet.
> ---
> gdb/gdbserver/ChangeLog | 5 +++++
> gdb/gdbserver/hostio.c | 12 ++++++++++++
> 2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
> index b38a6bd..8788f07 100644
> --- a/gdb/gdbserver/hostio.c
> +++ b/gdb/gdbserver/hostio.c
> @@ -344,6 +344,7 @@ handle_pread (char *own_buf, int *new_packet_len)
> {
> int fd, ret, len, offset, bytes_sent;
> char *p, *data;
> + static int max_reply_size = -1;
>
> p = own_buf + strlen ("vFile:pread:");
>
> @@ -359,6 +360,17 @@ handle_pread (char *own_buf, int *new_packet_len)
> return;
> }
>
> + /* Do not attempt to read more than the maximum number of bytes
> + hostio_reply_with_data can fit in a packet. We may still read
> + too much because of escaping, but this is handled below. */
> + if (max_reply_size == -1)
> + {
> + sprintf (own_buf, "F%x;", PBUFSIZ);
> + max_reply_size = PBUFSIZ - strlen (own_buf);
> + }
> + if (len > max_reply_size)
> + len = max_reply_size;
> +
> data = xmalloc (len);
> #ifdef HAVE_PREAD
> ret = pread (fd, data, len, offset);
> --
> 1.7.1
>

--