Re: [PATCH 0/2] Better handling of slow remote transfers

Re: [PATCH 0/2] Better handling of slow remote transfers

[Doug Evans]

Gary Benson writes:
  > Hi all,
  >
  > Since March or so GDB has been able to access inferior binaries for
  > remote targets without having to be explicitly told to.  This caused
  > problems for some people with slow connections:
  >
  >   https://sourceware.org/ml/gdb/2015-07/msg00038.html
  >
  > The first patch in this series adds the warning messages requested
  > in that thread.  The second commit should make long transfers
  > interruptible.
  >
  > Built and regtested on RHEL 6.6 x86_64.
  >
  > Ok to commit?

For 7.10, one thought is to maintain the behaviour of 7.9
and give ourselves more time to address this.
IOW, can we have (or is there already) a configure
option that controls the default behaviour,
and can we default it to what 7.9 does
(not auto-fetch files) ?

Re: [PATCH 0/2] Better handling of slow remote transfers

[Jan Kratochvil]

On Tue, 11 Aug 2015 19:22:54 +0200, Doug Evans wrote:
> IOW, can we have (or is there already) a configure
> option that controls the default behaviour,
> and can we default it to what 7.9 does
> (not auto-fetch files) ?

I would like to pinpoint that for patches targeting source distribution such
an additional behavior change switch is a QA nightmare.


Jan

Re: [PATCH 0/2] Better handling of slow remote transfers

[Doug Evans]

On Tue, Aug 11, 2015 at 11:55 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 11 Aug 2015 19:22:54 +0200, Doug Evans wrote:
>> IOW, can we have (or is there already) a configure
>> option that controls the default behaviour,
>> and can we default it to what 7.9 does
>> (not auto-fetch files) ?
>
> I would like to pinpoint that for patches targeting source distribution such
> an additional behavior change switch is a QA nightmare.

I'm curious what the QA costs of, for example, --with-sysroot are.
[And in particular are they nightmarish?]

At any rate, I think the default behaviour for 7.10
has to be the default behaviour of 7.9
(given that, for example, we're not going to make file transfer
more adequately interruptible for 7.10).

Re: [PATCH 0/2] Better handling of slow remote transfers

[Joel Brobecker]

> At any rate, I think the default behaviour for 7.10
> has to be the default behaviour of 7.9
> (given that, for example, we're not going to make file transfer
> more adequately interruptible for 7.10).

That makes sense to me.

-- 
Joel

Re: [PATCH 0/2] Better handling of slow remote transfers

[Jan Kratochvil]

On Tue, 11 Aug 2015 21:43:47 +0200, Doug Evans wrote:
> On Tue, Aug 11, 2015 at 11:55 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > I would like to pinpoint that for patches targeting source distribution such
> > an additional behavior change switch is a QA nightmare.
> 
> I'm curious what the QA costs of, for example, --with-sysroot are.

You are right it would make sense to make some cross-check that --with-sysroot
containing the same data works the same.


> [And in particular are they nightmarish?]

It was just related to me personally because I am developing these days
a related patchset on top of it.
	https://sourceware.org/git/?p=archer.git;a=log;h=refs/heads/jankratochvil/gdbserverbuildid-locate


Jan

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Joel Brobecker wrote:
> > At any rate, I think the default behaviour for 7.10
> > has to be the default behaviour of 7.9
> > (given that, for example, we're not going to make file transfer
> > more adequately interruptible for 7.10).
> 
> That makes sense to me.

If we are to reset the default sysroot to "" then please consider the
series I posted that added the auto-target-prefix functionality:

  https://sourceware.org/ml/gdb-patches/2015-07/msg00828.html

With these patches the "target:" prefix is only enabled if the user
does "target remote" with no sysroot or file specified, a case that
in 7.9 would result in a debug session with no symbols.
  
This has the following matrix of behaviours:

  * User wants to supply the file and not have libraries fetched
    (Sandra Loosemore's use case):

      bash$ gdb
      (gdb) file ./a.out
      (gdb) target remote :9999

    and:
      
      bash$ gdb ./a.out
      (gdb) target remote :9999

    7.9  -> no files transferred
    7.10 -> no files transferred

  * User wants to specify a file AND have GDB pull libraries from
    the remote:

      bash$ gdb
      (gdb) file ./a.out
      (gdb) set sysroot remote:
      (gdb) target remote :9999

    and:
    
      bash$ gdb ./a.out
      (gdb) set sysroot remote:
      (gdb) target remote :9999
      
    7.9  -> libraries transferred, executable read locally
    7.10 -> libraries transferred, executable read locally

  * User wants to connect to remote target and have GDB do the work:

      bash$ gdb
      (gdb) target remote :9999

    7.9  -> no files transferred, debug session with no symbols
    7.10 -> all files transferred, debug session with symbols

If the user actually wants to debug with no symbols at all they can
do this:

  bash$ gdb
  (gdb) set auto-target-prefix off
  (gdb) target remote :9999

With this series all 7.9 use cases are supported, and only the use
case where the user wants no symbols requires extra typing.  Most
users are going to want symbols, and any user capable of debugging
without symbols is capable of adding a line to their .gdbinit.

Thanks,
Gary
   
-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/11/2015 06:22 PM, Doug Evans wrote:
> Gary Benson writes:
>   > Hi all,
>   >
>   > Since March or so GDB has been able to access inferior binaries for
>   > remote targets without having to be explicitly told to.  This caused
>   > problems for some people with slow connections:
>   >
>   >   https://sourceware.org/ml/gdb/2015-07/msg00038.html
>   >
>   > The first patch in this series adds the warning messages requested
>   > in that thread.  The second commit should make long transfers
>   > interruptible.
>   >
>   > Built and regtested on RHEL 6.6 x86_64.
>   >
>   > Ok to commit?
> 
> For 7.10, one thought is to maintain the behaviour of 7.9
> and give ourselves more time to address this.

Agreed.  My opinion, as expressed elsewhere in the gdb@ thread,
is that in order to be able to have target: by default in 7.10, we'd
try to both sort out the interruptibility and add a suggestive
warning, assuming both were easy to do, and not invasive,
and check if that would be a sufficient resolution.  Seems like
the interruptibility issue isn't trivial to solve, so I think we
need to go do that -- change the default sysroot back to "", (and tweak
the docs/NEWS accordingly), and buy some time to sort this out on master.
Users can still then put "set sysroot target:" in .gdbinit with 7.10,
integrators should still be able to build with --with-sysroot=target: (or
revert the future-default-sysroot-reversion patch), and we can
continue addressing identified issues until "target:" (or something
around it, maybe building up on Jan's buildid work) can be made
the default, on master.

> IOW, can we have (or is there already) a configure
> option that controls the default behaviour,
> and can we default it to what 7.9 does
> (not auto-fetch files) ?

I think that would be the existing --with-sysroot.

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 10:48 AM, Gary Benson wrote:
> Joel Brobecker wrote:
>>> At any rate, I think the default behaviour for 7.10
>>> has to be the default behaviour of 7.9
>>> (given that, for example, we're not going to make file transfer
>>> more adequately interruptible for 7.10).
>>
>> That makes sense to me.
> 
> If we are to reset the default sysroot to "" then please consider the
> series I posted that added the auto-target-prefix functionality:
> 
>   https://sourceware.org/ml/gdb-patches/2015-07/msg00828.html

I'd really prefer not adding magic at the last minute to
the 7.10 release.  That would leave no breathing space to
sort out further design mistakes, which I'm sure we'll
trip on.

I think we need to unblock 7.10 as soon as possible so that 7.11
with all the neat sysroot features happens sooner too.  :-)

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 10:48 AM, Gary Benson wrote:
> > Joel Brobecker wrote:
> > > > At any rate, I think the default behaviour for 7.10 has to be
> > > > the default behaviour of 7.9 (given that, for example, we're
> > > > not going to make file transfer more adequately interruptible
> > > > for 7.10).
> > >
> > > That makes sense to me.
> > 
> > If we are to reset the default sysroot to "" then please
> > consider the series I posted that added the auto-target-prefix
> > functionality:
> > 
> >   https://sourceware.org/ml/gdb-patches/2015-07/msg00828.html
> 
> I'd really prefer not adding magic at the last minute to the 7.10
> release.  That would leave no breathing space to sort out further
> design mistakes, which I'm sure we'll trip on.

The only real "magic" that series adds is this:

+  if (target_filesystem_is_local ())
 ...
+  else if (auto_target_prefix && *gdb_sysroot == '\0')
+    {
+      /* Set the absolute prefix to "target:" for executable files
+	 and for shared libraries whose executable filename has a
+	 "target:"-prefix.  */
+      if (!is_solib
+	  || (exec_filename != NULL
+	      && is_target_filename (exec_filename)))
+	{
+	  sysroot = xstrdup (TARGET_FILENAME_PREFIX);
+	  make_cleanup (xfree, sysroot);
+	}
+    }

*If* it proves to be a problem then we can deprecate the set/show
auto-target-prefix boolean.

It seems like you're saying this series is a big change, but it's
really not: the core of it is that little snippet of logic, which
is easy enough to reason about:

  IF target filesystem is remote
     AND auto_target_prefix is enabled
     AND no sysroot is set
     AND (we're looking for an executable
          OR we're looking for a solib loaded by a target-prefixed executable):
       Prefix the filename with "target:"

It's certainly way less invasive a change than making transfers
interruptible would be.

> I think we need to unblock 7.10 as soon as possible so that 7.11
> with all the neat sysroot features happens sooner too.  :-)

Sure, but why not unblock it this way so that 7.10 users can have
the neat sysroot features, *if and only if* they use GDB in a way
that didn't make sense in 7.9?

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 11:38 AM, Gary Benson wrote:

> It seems like you're saying this series is a big change, but it's
> really not: the core of it is that little snippet of logic, which
> is easy enough to reason about:
> 
>   IF target filesystem is remote
>      AND auto_target_prefix is enabled
>      AND no sysroot is set
>      AND (we're looking for an executable
>           OR we're looking for a solib loaded by a target-prefixed executable):
>        Prefix the filename with "target:"
> 

IIUC, it still auto fetches the executable and then the solibs from the
target by default (e.g., after "attach"), so still subject to lack
of interruptibility?

> It's certainly way less invasive a change than making transfers
> interruptible would be.

I was only OK with trying to make transfers interruptible in the
branch assuming it was something non-invasive, like a missing QUIT
here and there.

>> I think we need to unblock 7.10 as soon as possible so that 7.11
>> with all the neat sysroot features happens sooner too.  :-)
> 
> Sure, but why not unblock it this way so that 7.10 users can have
> the neat sysroot features, *if and only if* they use GDB in a way
> that didn't make sense in 7.9?

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 11:38 AM, Gary Benson wrote:
> > It seems like you're saying this series is a big change, but it's
> > really not: the core of it is that little snippet of logic, which
> > is easy enough to reason about:
> > 
> >   IF target filesystem is remote
> >      AND auto_target_prefix is enabled
> >      AND no sysroot is set
> >      AND (we're looking for an executable
> >           OR we're looking for a solib loaded by a target-prefixed executable):
> >        Prefix the filename with "target:"
> > 
> 
> IIUC, it still auto fetches the executable and then the solibs from
> the target by default (e.g., after "attach"), so still subject to
> lack of interruptibility?

Yes and no.  It will fetch the executable from the remote iff one has
not been otherwise specified (i.e. by "file", or on the command line).
It will *only* fetch libraries from the remote if the parent executable
has a target prefix.  So:

  (gdb) file a.out
  (gdb) target remote :9999

   - exec_filename is "a.out"
   - exec_filename has no "target:" prefix
   - "target:" prefix is NOT applied to shared libraries
   - solib paths end up as "/path/to/libsolib.so.1"
   - solibs are NOT fetched over RSP

But:

  (gdb) target remote :9999

    - exec_filename is set to, e.g., "target:/path/to/a.out"
    - exec_filename HAS a "target:" prefix
    - "target:" prefix IS applied to shared libraries
    - solib paths end up as "target:/path/to/libsolib.so.1"
    - solibs ARE fetched over RSP

Basically it fetches the libraries over RSP if and only if the
executable was fetched over RSP.  So it works as Sandra expects
when she uses GDB her way, but it still has the automatic
executable filename discovery and automatic fetch-from-remote
for users who just do "target remote ..." on it's own (which is
something that doesn't make much sense in 7.9).
       
> > It's certainly way less invasive a change than making transfers
> > interruptible would be.
> 
> I was only OK with trying to make transfers interruptible in the
> branch assuming it was something non-invasive, like a missing QUIT
> here and there.

No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
data a character at a time.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 01:32 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 08/12/2015 11:38 AM, Gary Benson wrote:
>>> It seems like you're saying this series is a big change, but it's
>>> really not: the core of it is that little snippet of logic, which
>>> is easy enough to reason about:
>>>
>>>   IF target filesystem is remote
>>>      AND auto_target_prefix is enabled
>>>      AND no sysroot is set
>>>      AND (we're looking for an executable
>>>           OR we're looking for a solib loaded by a target-prefixed executable):
>>>        Prefix the filename with "target:"
>>>
>>
>> IIUC, it still auto fetches the executable and then the solibs from
>> the target by default (e.g., after "attach"), so still subject to
>> lack of interruptibility?
> 
> Yes and no.  It will fetch the executable from the remote iff one has
> not been otherwise specified (i.e. by "file", or on the command line).
> It will *only* fetch libraries from the remote if the parent executable
> has a target prefix.  So:
> 
>   (gdb) file a.out
>   (gdb) target remote :9999
> 
>    - exec_filename is "a.out"
>    - exec_filename has no "target:" prefix
>    - "target:" prefix is NOT applied to shared libraries
>    - solib paths end up as "/path/to/libsolib.so.1"
>    - solibs are NOT fetched over RSP
> 

But to me it looks like GDB _should_ retrieve the libraries
out of the target in this case.  You'll usually have a local copy
of the executable, because you just compiled it, but not of
the shared libraries.  It seems to me we're only considering
this option because we didn't make transfers interruptible?

>> I was only OK with trying to make transfers interruptible in the
>> branch assuming it was something non-invasive, like a missing QUIT
>> here and there.
> 
> No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
> data a character at a time.

Can you expand on this?  What code is it that reads the data a
character at a time?  What data is gdb getting at when it does that?

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 01:32 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > IIUC, it still auto fetches the executable and then the solibs
> > > from the target by default (e.g., after "attach"), so still
> > > subject to lack of interruptibility?
> > 
> > Yes and no.  It will fetch the executable from the remote iff one
> > has not been otherwise specified (i.e. by "file", or on the
> > command line).  It will *only* fetch libraries from the remote if
> > the parent executable has a target prefix.  So:
> > 
> >   (gdb) file a.out
> >   (gdb) target remote :9999
> > 
> >    - exec_filename is "a.out"
> >    - exec_filename has no "target:" prefix
> >    - "target:" prefix is NOT applied to shared libraries
> >    - solib paths end up as "/path/to/libsolib.so.1"
> >    - solibs are NOT fetched over RSP
> > 
> 
> But to me it looks like GDB _should_ retrieve the libraries out of
> the target in this case.  You'll usually have a local copy of the
> executable, because you just compiled it, but not of the shared
> libraries.  It seems to me we're only considering this option
> because we didn't make transfers interruptible?

Right, but users who have been doing that will be using "set sysroot
remote:":

  (gdb) file a.out
  (gdb) set sysroot remote:
  (gdb) target remote :9999

  - gdb_sysroot is "target:"
  - solib paths end up as "target:/path/to/libsolib.so.1"
  - solibs ARE fetched over RSP

This series basically means that:

  a) users get to type whatever they were typing before and have the
     same thing happen, with the exception of the 0.001% of users who
     have been typing "target remote :9999" with no "file" or "set sysroot"
     commands and debugging with no symbols whatsoever.

  b) those users can add "set auto-target-prefix off" in their .gdbinit

  c) users who want GDB to connect to a remote target and Just Work
     get to type "target remote :9999" without messing about with
     "file" and "set sysroot" commands.

> > > I was only OK with trying to make transfers interruptible in the
> > > branch assuming it was something non-invasive, like a missing
> > > QUIT here and there.
> > 
> > No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
> > data a character at a time.
> 
> Can you expand on this?  What code is it that reads the data a
> character at a time?  What data is gdb getting at when it does that?

I was looking in getpkt_or_notif_sane_1, but I think maybe I misread
it.  I'll get back to you on this...

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 01:32 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > I was only OK with trying to make transfers interruptible in the
> > > branch assuming it was something non-invasive, like a missing
> > > QUIT here and there.
> > 
> > No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
> > data a character at a time.
> 
> Can you expand on this?  What code is it that reads the data a
> character at a time?  What data is gdb getting at when it does that?

getpkt_or_notif_sane_1 in gdb/remote.c.  If I'm reading it right GDB
is reading every single byte coming over RSP individually.  It doesn't
work to put a QUIT in there as there's still stuff coming over the
wire.  If gdbserver sends data, GDB has to read it.

You're way more familiar with RSP than I am.  Do you know any way to
make vFile:pread: interruptible?

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 02:02 PM, Gary Benson wrote:
>>>> > > > I was only OK with trying to make transfers interruptible in the
>>>> > > > branch assuming it was something non-invasive, like a missing
>>>> > > > QUIT here and there.
>>> > > 
>>> > > No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
>>> > > data a character at a time.
>> > 
>> > Can you expand on this?  What code is it that reads the data a
>> > character at a time?  What data is gdb getting at when it does that?
> I was looking in getpkt_or_notif_sane_1, but I think maybe I misread
> it.  I'll get back to you on this...

That's the very low level of RSP packets, which as you noted will
have a reasonable cap.  It sounds to me there's a loop somewhere in
a higher layer that is missing a QUIT.  E.g., we have quits
in dwarf2read.c which allow interrupting reading big binaries,
even if locally.  So what is the higher level operation that
gdb is doing when you try to interrupt, but can't?

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 02:02 PM, Gary Benson wrote:
> >>>> > > > I was only OK with trying to make transfers interruptible in the
> >>>> > > > branch assuming it was something non-invasive, like a missing
> >>>> > > > QUIT here and there.
> >>> > > 
> >>> > > No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
> >>> > > data a character at a time.
> >> > 
> >> > Can you expand on this?  What code is it that reads the data a
> >> > character at a time?  What data is gdb getting at when it does that?
> > I was looking in getpkt_or_notif_sane_1, but I think maybe I misread
> > it.  I'll get back to you on this...
> 
> That's the very low level of RSP packets, which as you noted will
> have a reasonable cap.  It sounds to me there's a loop somewhere in
> a higher layer that is missing a QUIT.  E.g., we have quits
> in dwarf2read.c which allow interrupting reading big binaries,
> even if locally.  So what is the higher level operation that
> gdb is doing when you try to interrupt, but can't?

remote_hostio_pread.  I'm trying to make remote_hostio_pread
interruptible.  BFD is doing large remote_hostio_pread which
are resulting in large vFile:pread: packet responses.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Gary Benson wrote:
> Pedro Alves wrote:
> > On 08/12/2015 02:02 PM, Gary Benson wrote:
> > > I was looking in getpkt_or_notif_sane_1, but I think maybe I
> > > misread it.  I'll get back to you on this...
> > 
> > That's the very low level of RSP packets, which as you noted will
> > have a reasonable cap.  It sounds to me there's a loop somewhere
> > in a higher layer that is missing a QUIT.  E.g., we have quits in
> > dwarf2read.c which allow interrupting reading big binaries, even
> > if locally.  So what is the higher level operation that gdb is
> > doing when you try to interrupt, but can't?
> 
> remote_hostio_pread.  I'm trying to make remote_hostio_pread
> interruptible.  BFD is doing large remote_hostio_pread which
> are resulting in large vFile:pread: packet responses.

To elaborate:
  remote_hostio_pread calls remote_hostio_send_command once
  remote_hostio_send_command calls getpkt_sane once
  getpkt_sane calls getpkt_or_notif_sane_1 once

I've already posted a patch that did QUIT once per vFile:pread:
but that wasn't good enough.  For anything finer-grained the
QUIT needs to be in getpkt_or_notif_sane_1 but this doesn't seem
workable given how the protocol is.

The alternative is for remote_hostio_pread to make multiple
vFile:pread: packets, but that's going to introduce extra
traffic and latency.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 02:38 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 08/12/2015 02:02 PM, Gary Benson wrote:
>>>>>>>>> I was only OK with trying to make transfers interruptible in the
>>>>>>>>> branch assuming it was something non-invasive, like a missing
>>>>>>>>> QUIT here and there.
>>>>>>>
>>>>>>> No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
>>>>>>> data a character at a time.
>>>>>
>>>>> Can you expand on this?  What code is it that reads the data a
>>>>> character at a time?  What data is gdb getting at when it does that?
>>> I was looking in getpkt_or_notif_sane_1, but I think maybe I misread
>>> it.  I'll get back to you on this...
>>
>> That's the very low level of RSP packets, which as you noted will
>> have a reasonable cap.  It sounds to me there's a loop somewhere in
>> a higher layer that is missing a QUIT.  E.g., we have quits
>> in dwarf2read.c which allow interrupting reading big binaries,
>> even if locally.  So what is the higher level operation that
>> gdb is doing when you try to interrupt, but can't?
> 
> remote_hostio_pread.  I'm trying to make remote_hostio_pread
> interruptible.  BFD is doing large remote_hostio_pread which
> are resulting in large vFile:pread: packet responses.

And what is BFD doing that ends up in those remote_hostio_pread
calls?  Reading the elf headers, parsing the symbol table, etc?
Or maybe something else?

GDB will usually cap the transfers to before they get to the
lower layers.  E.g., look for 4096 in memory_xfer_partial,
target_read_alloc_1 and target_fileio_read_alloc_1.

As this request is coming from the BFD side, we should probably
make remote_hostio_pread also cap the size of the vFile:pread
request.  A reasonable number like a few KBs should not
introduce any noticeable slow down.

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 02:58 PM, Pedro Alves wrote:
> On 08/12/2015 02:38 PM, Gary Benson wrote:
>> Pedro Alves wrote:
>>> On 08/12/2015 02:02 PM, Gary Benson wrote:
>>>>>>>>>> I was only OK with trying to make transfers interruptible in the
>>>>>>>>>> branch assuming it was something non-invasive, like a missing
>>>>>>>>>> QUIT here and there.
>>>>>>>>
>>>>>>>> No, gdbserver sends the data in PBUFSIZ chunks, but GDB reads the
>>>>>>>> data a character at a time.
>>>>>>
>>>>>> Can you expand on this?  What code is it that reads the data a
>>>>>> character at a time?  What data is gdb getting at when it does that?
>>>> I was looking in getpkt_or_notif_sane_1, but I think maybe I misread
>>>> it.  I'll get back to you on this...
>>>
>>> That's the very low level of RSP packets, which as you noted will
>>> have a reasonable cap.  It sounds to me there's a loop somewhere in
>>> a higher layer that is missing a QUIT.  E.g., we have quits
>>> in dwarf2read.c which allow interrupting reading big binaries,
>>> even if locally.  So what is the higher level operation that
>>> gdb is doing when you try to interrupt, but can't?
>>
>> remote_hostio_pread.  I'm trying to make remote_hostio_pread
>> interruptible.  BFD is doing large remote_hostio_pread which
>> are resulting in large vFile:pread: packet responses.
> 
> And what is BFD doing that ends up in those remote_hostio_pread
> calls?  Reading the elf headers, parsing the symbol table, etc?
> Or maybe something else?
> 
> GDB will usually cap the transfers to before they get to the
> lower layers.  E.g., look for 4096 in memory_xfer_partial,
> target_read_alloc_1 and target_fileio_read_alloc_1.
> 
> As this request is coming from the BFD side, we should probably
> make remote_hostio_pread also cap the size of the vFile:pread
> request.  A reasonable number like a few KBs should not
> introduce any noticeable slow down.

But wait, I'm now confused -- isn't this a red herring?  Since
gdbserver is already limiting transfers to PBUFSIZE, this
change should have no practical effect, right?

How can BFD's large remote_hostio_pread result in large
vFile:pread: packet responses then?

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 02:58 PM, Pedro Alves wrote:
> > GDB will usually cap the transfers to before they get to the
> > lower layers.  E.g., look for 4096 in memory_xfer_partial,
> > target_read_alloc_1 and target_fileio_read_alloc_1.
> > 
> > As this request is coming from the BFD side, we should probably
> > make remote_hostio_pread also cap the size of the vFile:pread
> > request.  A reasonable number like a few KBs should not
> > introduce any noticeable slow down.
> 
> But wait, I'm now confused -- isn't this a red herring?  Since
> gdbserver is already limiting transfers to PBUFSIZE, this change
> should have no practical effect, right?
> 
> How can BFD's large remote_hostio_pread result in large vFile:pread:
> packet responses then?

I think gdbserver is returning multiple packets but something in GDB
(getpkt_or_notif_sane_1?) is concatenating them together somehow.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/12/2015 04:08 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 08/12/2015 02:58 PM, Pedro Alves wrote:
>>> GDB will usually cap the transfers to before they get to the
>>> lower layers.  E.g., look for 4096 in memory_xfer_partial,
>>> target_read_alloc_1 and target_fileio_read_alloc_1.
>>>
>>> As this request is coming from the BFD side, we should probably
>>> make remote_hostio_pread also cap the size of the vFile:pread
>>> request.  A reasonable number like a few KBs should not
>>> introduce any noticeable slow down.
>>
>> But wait, I'm now confused -- isn't this a red herring?  Since
>> gdbserver is already limiting transfers to PBUFSIZE, this change
>> should have no practical effect, right?
>>
>> How can BFD's large remote_hostio_pread result in large vFile:pread:
>> packet responses then?
> 
> I think gdbserver is returning multiple packets but something in GDB
> (getpkt_or_notif_sane_1?) is concatenating them together somehow.

No, getpkt_or_notif_sane_1 will return as soon as it has a single
packet, which should then be bubbling up the layers and reaching
gdb_bfd_iovec_fileio_pread.  Something else is going on.
Either the QUIT is being lost/eaten, or ... hmm ... maybe the
SIGINT handler is set to remote.c:async_handle_remote_sigint
when the ctrl-c is typed, which means the ctrl-c doesn't
actually set_quit_flag()?

Thanks,
Pedro Alves

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> On 08/12/2015 04:08 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > On 08/12/2015 02:58 PM, Pedro Alves wrote:
> > > > GDB will usually cap the transfers to before they get to the
> > > > lower layers.  E.g., look for 4096 in memory_xfer_partial,
> > > > target_read_alloc_1 and target_fileio_read_alloc_1.
> > > >
> > > > As this request is coming from the BFD side, we should
> > > > probably make remote_hostio_pread also cap the size of the
> > > > vFile:pread request.  A reasonable number like a few KBs
> > > > should not introduce any noticeable slow down.
> > >
> > > But wait, I'm now confused -- isn't this a red herring?  Since
> > > gdbserver is already limiting transfers to PBUFSIZE, this change
> > > should have no practical effect, right?
> > >
> > > How can BFD's large remote_hostio_pread result in large
> > > vFile:pread: packet responses then?
> > 
> > I think gdbserver is returning multiple packets but something in
> > GDB (getpkt_or_notif_sane_1?) is concatenating them together
> > somehow.
> 
> No, getpkt_or_notif_sane_1 will return as soon as it has a single
> packet, which should then be bubbling up the layers and reaching
> gdb_bfd_iovec_fileio_pread.  Something else is going on.  Either the
> QUIT is being lost/eaten, or ... hmm ... maybe the SIGINT handler is
> set to remote.c:async_handle_remote_sigint when the ctrl-c is typed,
> which means the ctrl-c doesn't actually set_quit_flag()?

I've no idea.  Really I haven't.

I have to finish for the day now.  I'll be back in 16 hours.
Maybe somebody who'll benefit from interruptible remote transfers
could look into this while I'm away.  Sandra?  Pedro?  Doug?

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Joel Brobecker]

> If we are to reset the default sysroot to "" then please consider the
> series I posted that added the auto-target-prefix functionality:
> 
>   https://sourceware.org/ml/gdb-patches/2015-07/msg00828.html
> 
> With these patches the "target:" prefix is only enabled if the user
> does "target remote" with no sysroot or file specified, a case that
> in 7.9 would result in a debug session with no symbols.

Trying to summarize where we are, right now:

   1. Part of the discussion was about trying to figure out why
      C-c would not interrupt file transfers midway through;

   2. Before that, there was some discussion about whether or not
      we should transfer all files in the case where the no
      executable was provided and sysroot is empty.

      I'm unclear whether this part was generally accepted or not.
      I have a feeling that having (1) resolved would go a long way
      towards accepting (2).

That being said, I understand Pedro's concerns regarding adding
this extra logic that late in the game, even if it is fairly simple.
But if he or others agree with it, then I would be fine with it too.
It does seem to only affect a corner case.

-- 
Joel

Re: [PATCH 0/2] Better handling of slow remote transfers

[Sandra Loosemore]

On 08/14/2015 12:26 PM, Joel Brobecker wrote:
>> If we are to reset the default sysroot to "" then please consider the
>> series I posted that added the auto-target-prefix functionality:
>>
>>    https://sourceware.org/ml/gdb-patches/2015-07/msg00828.html
>>
>> With these patches the "target:" prefix is only enabled if the user
>> does "target remote" with no sysroot or file specified, a case that
>> in 7.9 would result in a debug session with no symbols.
>
> Trying to summarize where we are, right now:
>
>     1. Part of the discussion was about trying to figure out why
>        C-c would not interrupt file transfers midway through;
>
>     2. Before that, there was some discussion about whether or not
>        we should transfer all files in the case where the no
>        executable was provided and sysroot is empty.
>
>        I'm unclear whether this part was generally accepted or not.
>        I have a feeling that having (1) resolved would go a long way
>        towards accepting (2).
>
> That being said, I understand Pedro's concerns regarding adding
> this extra logic that late in the game, even if it is fairly simple.
> But if he or others agree with it, then I would be fine with it too.
> It does seem to only affect a corner case.

It actually looks to me like we are not any closer to having this 
resolved than we were at the beginning of the week.  Gary's last patch 
for ^C handling didn't work for me and he said he is out of ideas.  I am 
willing to try out patches, but I'm really swamped with other tasks 
right now as well as being totally unfamiliar with the internals of this 
code, so it's not reasonable to think I could fix this myself in time 
for the 7.10 release.  And AFAIK nobody else is working on this either.  :-(

-Sandra

Re: [PATCH 0/2] Better handling of slow remote transfers

[Joel Brobecker]

> It actually looks to me like we are not any closer to having this resolved
> than we were at the beginning of the week.  Gary's last patch for ^C
> handling didn't work for me and he said he is out of ideas.  I am willing to
> try out patches, but I'm really swamped with other tasks right now as well
> as being totally unfamiliar with the internals of this code, so it's not
> reasonable to think I could fix this myself in time for the 7.10 release.
> And AFAIK nobody else is working on this either.  :-(

I think the situation is a little better than you describe.
As far as I understand, there is one patch that changes the default
for sysroot back to ""; it is expected to restore the current behavior
in your case, but at the same time introduces a change in behavior
in one specific situation where someone is debugging remotely without
providing the executable to GDB (either through the command-line or
using the "file" command). The change of behavior and how to control it
is what's being debated at the moment, and is why you're still seeing
the issue.

Parallel to that, my understanding of the situation is that there is
a secondary issue of not being able interrupt a file transfer. That
is what you've been testing so far, I believe. Lack of success so far
is a little fustrating for everyone, I'm sure. But I am still wondering
whether you should even be in the situation where you need to interrupt
in the first place.

That's why, to me, the first discussion has a little more weight.
We'll want to figure out the mystery in the secondary issue, but
if we can find the right approach in the first discussion for 7.10,
that would buy us some extra time in terms of being able to interrupt
file transfers.

-- 
Joel

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Joel Brobecker wrote:
> Sandra Loosemore wrote:
> > It actually looks to me like we are not any closer to having this
> > resolved than we were at the beginning of the week.  Gary's last
> > patch for ^C handling didn't work for me and he said he is out of
> > ideas.  I am willing to try out patches, but I'm really swamped
> > with other tasks right now as well as being totally unfamiliar
> > with the internals of this code, so it's not reasonable to think I
> > could fix this myself in time for the 7.10 release.  And AFAIK
> > nobody else is working on this either.  :-(
> 
> I think the situation is a little better than you describe.  As far
> as I understand, there is one patch that changes the default for
> sysroot back to ""; it is expected to restore the current behavior
> in your case, but at the same time introduces a change in behavior
> in one specific situation where someone is debugging remotely
> without providing the executable to GDB (either through the
> command-line or using the "file" command). The change of behavior
> and how to control it is what's being debated at the moment, and is
> why you're still seeing the issue.

Just to clarify, the default sysroot of "target:" has two purposes:

 1. It allows GDB to locate and access binaries on remote targets
    without having to be told where they are.

 2. It allows GDB to locate and access binaries on native targets
    when the inferior is running in a container.

Resetting the default sysroot to "" disables both cases.  The second
is arguably more important, because without it users can attach to a
local process (with, e.g., gdb -p PID) but GDB can end up loading the
wrong symbols if binaries with the same paths exist both within the
container and on the host machine.

> Parallel to that, my understanding of the situation is that there is
> a secondary issue of not being able interrupt a file transfer. That
> is what you've been testing so far, I believe. Lack of success so
> far is a little fustrating for everyone, I'm sure. But I am still
> wondering whether you should even be in the situation where you need
> to interrupt in the first place.
> 
> That's why, to me, the first discussion has a little more weight.
> We'll want to figure out the mystery in the secondary issue, but
> if we can find the right approach in the first discussion for 7.10,
> that would buy us some extra time in terms of being able to interrupt
> file transfers.

It seems to me that being able to interrupt file transfers is polish.
With the warning patch alone, users will see the warning and the hint
about how to restore the previous default, which they can apply and
continue as before.  If they have to wait out a transfer then it will
presumably only be once.  I know some people use GDB on systems with
5,000+ shared libraries, and others use GDB on slow serial links, but
I don't think anybody combines these cases.

So, would the warning+hint patch alone be enough?

FWIW the reason I am out of ideas for making transfers interruptible
is that both QUIT patches I supplied allow me to interrupt transfers.
Something is obviously different on Sandra's setup to mine, but I
don't know what, and without a reproducer I'm just stabbing in the
dark.

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Sandra Loosemore]

On 08/17/2015 02:53 AM, Gary Benson wrote:

> It seems to me that being able to interrupt file transfers is polish.
> With the warning patch alone, users will see the warning and the hint
> about how to restore the previous default, which they can apply and
> continue as before.  If they have to wait out a transfer then it will
> presumably only be once.  I know some people use GDB on systems with
> 5,000+ shared libraries, and others use GDB on slow serial links, but
> I don't think anybody combines these cases.

FYI, I am not debugging over a "slow serial link".  I've been testing 
this on an Altera 3c120 board (Nios II) with 10/100 Ethernet; it 
NFS-mounts the sysroot under test and before now that has worked fine 
with no obvious responsiveness issues.

> So, would the warning+hint patch alone be enough?

Is it really user-friendly to make the user either wait 4 minutes or 
kill GDB through a separate terminal before they can act on the hint?

-Sandra

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Sandra Loosemore wrote:
> On 08/17/2015 02:53 AM, Gary Benson wrote:
> > It seems to me that being able to interrupt file transfers is
> > polish.  With the warning patch alone, users will see the warning
> > and the hint about how to restore the previous default, which they
> > can apply and continue as before.  If they have to wait out a
> > transfer then it will presumably only be once.  I know some people
> > use GDB on systems with 5,000+ shared libraries, and others use
> > GDB on slow serial links, but I don't think anybody combines these
> > cases.
> 
> FYI, I am not debugging over a "slow serial link".  I've been
> testing this on an Altera 3c120 board (Nios II) with 10/100
> Ethernet; it NFS-mounts the sysroot under test and before now that
> has worked fine with no obvious responsiveness issues.
> 
> > So, would the warning+hint patch alone be enough?
> 
> Is it really user-friendly to make the user either wait 4 minutes
> of kill GDB through a separate terminal before they can act on the
> hint?

This user-friendliness argument is almost a straw man.  Is it
user-friendly to make the user wait 4 minutes before they can update
their .gdbinit and continue as before?  Probably not.  Is it
user-friendly to make the user set up NFS on the target or copy all
the files across (and keep them synchronized)?  Also probably not.
Is it user friendly to expect users who want GDB to locate binaries
for them to add "set sysroot target:" to their .gdbinit?  Also
probably not.  Is it user friendly to expect users who want to use
GDB across containers to add "set sysroot target:" to their .gdbinit?
Also probably not.  So lets leave user-friendliness to one side and
talk about what's actually happening.

For some reason, the Altera 3c120 board you are using is very much
slower to transfer files over RSP than it is over NFS.

For some reason, neither of the two QUIT patches I mailed work on your
setup with this Altera 3c120 board you are using even though they work
just fine on this x86_64 machine I am using.

Your PandaBoard takes 8 seconds.  That doesn't seem so bad to me.
If this Altera board is the only one with the massive slowdown then
I don't think we should delay 7.10 any further on this issue--and I
certainly don't think we should lose the functionality that the
default sysroot of "target:" brings.

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Sandra Loosemore]

On 08/18/2015 03:58 AM, Gary Benson wrote:
>
> For some reason, the Altera 3c120 board you are using is very much
> slower to transfer files over RSP than it is over NFS.
>
> For some reason, neither of the two QUIT patches I mailed work on your
> setup with this Altera 3c120 board you are using even though they work
> just fine on this x86_64 machine I am using.

I would be happy to try to help debug what is going wrong here, but I'm 
not a GDB expert by any means and I'm totally unfamiliar with this part 
of the code and don't know where to start.  If somebody has a theory on 
this, is there someplace I could put in printfs to trace what is 
happening?  From the previous discussion, it seemed like the flow 
through the different layers of abstraction to what the RSP was actually 
doing was very obscure, in terms of where the chunking is happening, etc.

> Your PandaBoard takes 8 seconds.  That doesn't seem so bad to me.
> If this Altera board is the only one with the massive slowdown then
> I don't think we should delay 7.10 any further on this issue--and I
> certainly don't think we should lose the functionality that the
> default sysroot of "target:" brings.

FWIW, I have just tried on a random sampling of our MIPS target boards 
as well, with the same test program and same scenario of continuing to a 
breakpoint on main.  The timings were 1:40 (SEAD-3 LX110 board, M14Kc 
microMips), 1:38 (MALTA 74Kc MIPS16), and 1:20 (MALTA 5Kef MIPS64).  I 
could try some PowerPC boards as well if we need more datapoints.

In any case, I think we should be cautious about declaring that 
functionality that seems to work on x86_64 native should work 
everywhere, and if it doesn't it's the fault of the target or user 
instead of buggy code or poor design.  IMO this is especially true for 
remote debugging, where I think debugging an embedded target is a more 
common scenario than a native one.

-Sandra

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

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

On 08/18/2015 05:50 PM, Sandra Loosemore wrote:

> I would be happy to try to help debug what is going wrong here, but I'm
> not a GDB expert by any means and I'm totally unfamiliar with this part 
> of the code and don't know where to start.  If somebody has a theory on 
> this, is there someplace I could put in printfs to trace what is 
> happening?  From the previous discussion, it seemed like the flow 
> through the different layers of abstraction to what the RSP was actually 
> doing was very obscure, in terms of where the chunking is happening, etc.

That was just a misunderstanding.  It's not really obscure.  GDB has a file io
abstraction that wires the open/close/pread/pwrite syscalls to RSP packets.
Callers just need to use the target_fileio_open/pread etc. functions instead
of the usual syscalls.  Whenever BFD or GDB accesses a file that is prefixed
target: or remote:, the open/read/write requests go through those calls.
For remote targets, those end up in remote.c:remote_hostio_pread, etc.  The only
chunking going on is the server trimming the pread results to the maximum packet
size it can return to gdb.  Both gdb and gdbserver have that maximum buffer
size hardcoded at 16384 for historical reasons.

>> Your PandaBoard takes 8 seconds.  That doesn't seem so bad to me.
>> If this Altera board is the only one with the massive slowdown then
>> I don't think we should delay 7.10 any further on this issue--and I
>> certainly don't think we should lose the functionality that the
>> default sysroot of "target:" brings.
> 
> FWIW, I have just tried on a random sampling of our MIPS target boards 
> as well, with the same test program and same scenario of continuing to a 
> breakpoint on main.  The timings were 1:40 (SEAD-3 LX110 board, M14Kc 
> microMips), 1:38 (MALTA 74Kc MIPS16), and 1:20 (MALTA 5Kef MIPS64).  I 
> could try some PowerPC boards as well if we need more datapoints.

Thanks for doing this.  As yet another data point, it'd be interesting to
know whether Gary's rate-limit patch speeds up any other of these boards
as well.

Did you try getting the nfs mount out of the picture?  That is, copy the
debug-build files to the target, and have gdb pull them automatically.

It's expected that reading a file out of remote filesystem
is going to be slower than not reading it.  Obviously some slowdown
is expected.  There's no free lunch.

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'm trying to build a more complete picture of the potential for
trouble.  The more data, the better...

So tonight I played with this.  To amplify latency issues against a remote target,
I've built gdbserver on gcc76 on the gcc compile farm.  Anyone can request
access to the gcc compile farm machines and to reproduce this.  That's an x86-64
in France almost 2000km away from me.  I've got a ping time of ~85ms to
that machine, and I'm behind a ~16Mbit ADSL.

As an illustration, here's what it takes to download the gdbserver binary using ssh/scp.
Not the fastest file transfer method, but the most convenient:

$ time scp gcc76.fsffrance.org:/home/palves/gdb/build/gdb/gdbserver/gdbserver /tmp/gdbserver
gdbserver             100% 1451KB   1.4MB/s   00:01

real    0m1.296s
user    0m0.015s
sys     0m0.013s

Now let's compare with GDB's file retrieval:

[palves@gcc76] $ ./gdbserver --multi :9999

[palves@home] $ ssh -L 9999:localhost:9999 gcc76.fsffrance.org
[palves@home] $ cat /tmp/remote-get
define getfile
       shell date
       remote get /home/palves/gdb/build/gdb/gdbserver/gdbserver /tmp/gdbserver
       shell date
       end
[palves@home] $ ./gdb -data-directory=data-directory -ex "tar extended-rem :9999" -x /tmp/remote-get

(gdb) getfile
Detaching after fork from child process 31567.
Wed Aug 19 01:04:13 WEST 2015
Detaching after fork from child process 31568.
Wed Aug 19 01:04:23 WEST 2015
(gdb)

~10s GDB/vFile vs ~1.3s scp/ssh.  Not that impressive.

So then I tried bumping the RSP max packet buffer size.  That required
a patch to gdb and another to gdbserver.  I bumped the buffer size 16 times.
The result was:

(gdb) getfile
Wed Aug 19 01:13:01 WEST 2015
Wed Aug 19 01:13:04 WEST 2015
(gdb) getfile
Wed Aug 19 01:13:12 WEST 2015
Wed Aug 19 01:13:14 WEST 2015
(gdb) getfile
Wed Aug 19 01:13:19 WEST 2015
Wed Aug 19 01:13:21 WEST 2015

So 2s/3s.  Not bad.

Bumping the packet buffer size to 4MB, to make sure
a single vFile:pread covers the ~1.5MB file, I consistently
get ~1s:

(gdb) getfile
Wed Aug 19 01:17:52 WEST 2015
Wed Aug 19 01:17:53 WEST 2015
(gdb) getfile
Wed Aug 19 01:17:55 WEST 2015
Wed Aug 19 01:17:56 WEST 2015
(gdb) getfile
Wed Aug 19 01:17:57 WEST 2015
Wed Aug 19 01:17:58 WEST 2015
(gdb)

which is then in the ball park of scp.  Seems clear to me that for
whole-file transfers, the bottleneck (on this use case) is the number
of chunks / round trips.  For file transfer, obviously a stream-based
transfer, like scp's, that avoids roundtrip hiccups, wins.

For the use case of connecting to gdbserver with "target remote",
and having gdb fetch the binary out of gdbserver with vFile, I tried
something else.

This time, I'll simply debug gdbserver itself remotely, letting gdb
fetch the gdbserver binary, and run to main.  Once main is reached,
quit:

[palves@gcc76] $ ./gdbserver :9999 ./gdbserver
[palves@home] $ ssh -L 9999:localhost:9999 gcc76.fsffrance.org
[palves@home] time ./gdb -data-directory=data-directory -ex "tar rem :9999" -ex "b main" -ex "c" -ex "set confirm off" -ex "quit"

Pristine gdb 7.10.50.20150816-cvs gets me:
...
Remote debugging using :9999
Reading symbols from target:/home/palves/gdb/build/gdb/gdbserver/gdbserver...done.
Reading symbols from target:/lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
0x00007ffff7ddd190 in ?? () from target:/lib64/ld-linux-x86-64.so.2
Breakpoint 1 at 0x41200c: file ../../../src/gdb/gdbserver/server.c, line 3635.
Continuing.

Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
/home/palves/gdb/build/gdb/gdbserver/gdbserver: No such file or directory.

real    0m55.315s
user    0m0.400s
sys     0m0.087s

Bumping the remote max packet buffer size to (16384*16) helps a bit,
though not impressive:

real    0m51.600s
user    0m0.404s
sys     0m0.085s

Bumping to 0x100000 helps a bit further:

real    0m48.335s
user    0m0.421s
sys     0m0.091s

Looking at "set debug remote 1" output, I noticed that gdb often issues
a sequence of small vFile:pread requests for the same file.  Ctrl-c'ing
gdb while it was doing that and getting a backtrace shows that those requests
often came from bfd.  So I thought that maybe adding a readahead buffer/cache
to gdb's vFile:pread requests could help.  And indeed, that dropped the
time for the same use case further down to:

real    0m29.055s
user    0m2.625s
sys     0m0.090s

I added a few counters to show cache hit/miss, and got:

 readahead cache miss 44
 readahead cache hit 377

Not sure exactly what makes the "user" time go up that much (maybe
reading too much ahead causes too much RSP escaping, etc.?)
If I repeat the same leaving gdbserver's maximum buffer size
set at the original 16384, I still get <30s:

real    0m27.169s
user    0m0.464s
sys     0m0.061s

though notice the user time goes down again.  Maybe this indicates
that bigger chunk sizes indeed increase CPU usage, which on slower boards
would have a more profound effect than on the x86_64 machines I'm using.
An observation in line with Sandra's observation of Gary's patch speeding
up the nios2 board.  Or maybe that's just a coincidence.

BTW, the transfers seem to be always interruptible for me, even without
Gary's patch, and even the slow ones.

And finally, here's the time using a local sysroot and specifying a local
program:

$ time $g -ex "set sysroot /" -ex "tar rem :9999" -ex "b main" -ex "c" -ex "set confirm off" -ex "quit" /tmp/gdbserver
Reading symbols from /tmp/gdbserver...done.
Remote debugging using :9999
Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib64/ld-2.18.so.debug...done.
done.
0x00007ffff7ddd190 in ?? ()
Breakpoint 1 at 0x41200c: file ../../../src/gdb/gdbserver/server.c, line 3635.
Continuing.

Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.

real    0m7.385s
user    0m0.132s
sys     0m0.028s

I'm attaching the 3 patches that I used.  I wouldn't be surprised
if they contain obvious blunders; it's getting quite late here...

Thanks,
Pedro Alves


[-- Attachment #2: 0001-gdb-remove-packet-size-limit.patch --]
[-- Type: text/x-patch, Size: 4013 bytes --]

From 5fec4911ec7cd9ea2ee4b5d3be0502e7a7df1f9f Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 18 Aug 2015 22:53:56 +0100
Subject: [PATCH 1/3] gdb: remove packet size limit

---
 gdb/remote.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index ca1f0df..a839adf 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -894,14 +894,6 @@ get_memory_packet_size (struct memory_packet_config *config)
   struct remote_state *rs = get_remote_state ();
   struct remote_arch_state *rsa = get_remote_arch_state ();
 
-  /* NOTE: The somewhat arbitrary 16k comes from the knowledge (folk
-     law?) that some hosts don't cope very well with large alloca()
-     calls.  Eventually the alloca() code will be replaced by calls to
-     xmalloc() and make_cleanups() allowing this restriction to either
-     be lifted or removed.  */
-#ifndef MAX_REMOTE_PACKET_SIZE
-#define MAX_REMOTE_PACKET_SIZE 16384
-#endif
   /* NOTE: 20 ensures we can write at least one byte.  */
 #ifndef MIN_REMOTE_PACKET_SIZE
 #define MIN_REMOTE_PACKET_SIZE 20
@@ -910,7 +902,7 @@ get_memory_packet_size (struct memory_packet_config *config)
   if (config->fixed_p)
     {
       if (config->size <= 0)
-	what_they_get = MAX_REMOTE_PACKET_SIZE;
+	what_they_get = 16384;
       else
 	what_they_get = config->size;
     }
@@ -929,8 +921,6 @@ get_memory_packet_size (struct memory_packet_config *config)
 	  && what_they_get > rsa->actual_register_packet_size)
 	what_they_get = rsa->actual_register_packet_size;
     }
-  if (what_they_get > MAX_REMOTE_PACKET_SIZE)
-    what_they_get = MAX_REMOTE_PACKET_SIZE;
   if (what_they_get < MIN_REMOTE_PACKET_SIZE)
     what_they_get = MIN_REMOTE_PACKET_SIZE;
 
@@ -3911,6 +3901,7 @@ remote_check_symbols (void)
   char *msg, *reply, *tmp;
   struct bound_minimal_symbol sym;
   int end;
+  struct cleanup *old_chain;
 
   /* The remote side has no concept of inferiors that aren't running
      yet, it only knows about running processes.  If we're connected
@@ -3929,7 +3920,8 @@ remote_check_symbols (void)
 
   /* Allocate a message buffer.  We can't reuse the input buffer in RS,
      because we need both at the same time.  */
-  msg = alloca (get_remote_packet_size ());
+  msg = xmalloc (get_remote_packet_size ());
+  old_chain = make_cleanup (xfree, msg);
 
   /* Invite target to request symbol lookups.  */
 
@@ -3967,6 +3959,8 @@ remote_check_symbols (void)
       getpkt (&rs->buf, &rs->buf_size, 0);
       reply = rs->buf;
     }
+
+  do_cleanups (old_chain);
 }
 
 static struct serial *
@@ -4089,13 +4083,6 @@ remote_packet_size (const struct protocol_feature *feature,
       return;
     }
 
-  if (packet_size > MAX_REMOTE_PACKET_SIZE)
-    {
-      warning (_("limiting remote suggested packet size (%d bytes) to %d"),
-	       packet_size, MAX_REMOTE_PACKET_SIZE);
-      packet_size = MAX_REMOTE_PACKET_SIZE;
-    }
-
   /* Record the new maximum packet size.  */
   rs->explicit_packet_size = packet_size;
 }
@@ -7645,7 +7632,8 @@ putpkt_binary (const char *buf, int cnt)
   struct remote_state *rs = get_remote_state ();
   int i;
   unsigned char csum = 0;
-  char *buf2 = alloca (cnt + 6);
+  char *buf2 = xmalloc (cnt + 6);
+  struct cleanup *old_chain = make_cleanup (xfree, buf2);
 
   int ch;
   int tcount = 0;
@@ -7738,6 +7726,7 @@ putpkt_binary (const char *buf, int cnt)
 	    case '+':
 	      if (remote_debug)
 		fprintf_unfiltered (gdb_stdlog, "Ack\n");
+	      do_cleanups (old_chain);
 	      return 1;
 	    case '-':
 	      if (remote_debug)
@@ -7746,7 +7735,10 @@ putpkt_binary (const char *buf, int cnt)
 	    case SERIAL_TIMEOUT:
 	      tcount++;
 	      if (tcount > 3)
-		return 0;
+		{
+		  do_cleanups (old_chain);
+		  return 0;
+		}
 	      break;		/* Retransmit buffer.  */
 	    case '$':
 	      {
@@ -7833,6 +7825,8 @@ putpkt_binary (const char *buf, int cnt)
 	}
 #endif
     }
+
+  do_cleanups (old_chain);
   return 0;
 }
 
-- 
1.9.3


[-- Attachment #3: 0002-gdbserver-bump-max-packet-buffer-size.patch --]
[-- Type: text/x-patch, Size: 934 bytes --]

From 0becb8d88cc5e364ce413258de32d326c6395ad6 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 19 Aug 2015 00:04:18 +0200
Subject: [PATCH 2/3] gdbserver: bump max packet buffer size

---
 gdb/gdbserver/server.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 9080151..9ec3708 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -127,6 +127,8 @@ extern int handle_target_event (int err, gdb_client_data client_data);
 /* Buffer sizes for transferring memory, registers, etc.   Set to a constant
    value to accomodate multiple register formats.  This value must be at least
    as large as the largest register set supported by gdbserver.  */
-#define PBUFSIZ 16384
-
+//#define PBUFSIZ (16384)
+#define PBUFSIZ (16384*16)
+//#define PBUFSIZ 0x100000
+//#define PBUFSIZ 0x400000
 #endif /* SERVER_H */
-- 
1.9.3


[-- Attachment #4: 0003-add-readahead-cache-to-gdb-s-vFile-pread.patch --]
[-- Type: text/x-patch, Size: 4511 bytes --]

From d426ce0a36830378a8ec2e2cbdd94d9fcb47b891 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 18 Aug 2015 23:27:20 +0100
Subject: [PATCH 3/3] add readahead cache to gdb's vFile:pread

---
 gdb/remote.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 99 insertions(+), 3 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index a839adf..8a739c8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10311,6 +10311,26 @@ remote_hostio_send_command (int command_bytes, int which_packet,
   return ret;
 }
 
+struct readahead_cache
+{
+  int fd;
+  gdb_byte *buf;
+  ULONGEST offset;
+  size_t bufsize;
+};
+
+static struct readahead_cache *readahead_cache;
+
+static void
+readahead_cache_invalidate (void)
+{
+  if (readahead_cache != NULL)
+    readahead_cache->fd = -1;
+}
+
+static unsigned int readahead_cache_hit_count;
+static unsigned int readahead_cache_miss_count;
+
 /* Set the filesystem remote_hostio functions that take FILENAME
    arguments will use.  Return 0 on success, or -1 if an error
    occurs (and set *REMOTE_ERRNO).  */
@@ -10325,6 +10345,8 @@ remote_hostio_set_filesystem (struct inferior *inf, int *remote_errno)
   char arg[9];
   int ret;
 
+  readahead_cache_invalidate ();
+
   if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
     return 0;
 
@@ -10389,6 +10411,9 @@ remote_hostio_pwrite (struct target_ops *self,
   int left = get_remote_packet_size ();
   int out_len;
 
+  if (readahead_cache != NULL && readahead_cache->fd == fd)
+    readahead_cache_invalidate ();
+
   remote_buffer_add_string (&p, &left, "vFile:pwrite:");
 
   remote_buffer_add_int (&p, &left, fd);
@@ -10407,9 +10432,9 @@ remote_hostio_pwrite (struct target_ops *self,
 /* Implementation of to_fileio_pread.  */
 
 static int
-remote_hostio_pread (struct target_ops *self,
-		     int fd, gdb_byte *read_buf, int len,
-		     ULONGEST offset, int *remote_errno)
+remote_hostio_pread_1 (struct target_ops *self,
+		       int fd, gdb_byte *read_buf, int len,
+		       ULONGEST offset, int *remote_errno)
 {
   struct remote_state *rs = get_remote_state ();
   char *p = rs->buf;
@@ -10443,6 +10468,74 @@ remote_hostio_pread (struct target_ops *self,
   return ret;
 }
 
+static int
+remote_hostio_pread_from_cache (struct readahead_cache *cache,
+				int fd, gdb_byte *read_buf, int len,
+				ULONGEST offset)
+{
+  if (cache->fd == fd
+      && cache->offset <= offset
+      && offset < cache->offset + cache->bufsize)
+    {
+      ULONGEST max = cache->offset + cache->bufsize;
+
+      if (offset + len > max)
+	len = max - offset;
+
+      if (remote_debug)
+	fprintf_unfiltered (gdb_stdlog,
+			    "readahead cache hit %d\n",
+			    ++readahead_cache_hit_count);
+      memcpy (read_buf, cache->buf + offset - cache->offset, len);
+      return len;
+    }
+
+  return -1;
+}
+
+static int
+remote_hostio_pread (struct target_ops *self,
+		     int fd, gdb_byte *read_buf, int len,
+		     ULONGEST offset, int *remote_errno)
+{
+  int ret;
+
+  if (readahead_cache != NULL)
+    {
+      ret = remote_hostio_pread_from_cache (readahead_cache, fd,
+					    read_buf, len, offset);
+      if (ret >= 0)
+	return ret;
+
+      readahead_cache_invalidate ();
+    }
+
+  if (remote_debug)
+    fprintf_unfiltered (gdb_stdlog, "readahead cache miss %d\n",
+			++readahead_cache_miss_count);
+
+  if (readahead_cache == NULL)
+    readahead_cache = XCNEW (struct readahead_cache);
+  readahead_cache->fd = fd;
+  readahead_cache->offset = offset;
+  readahead_cache->bufsize = get_remote_packet_size ();
+  readahead_cache->buf = xrealloc (readahead_cache->buf, readahead_cache->bufsize);
+
+  ret = remote_hostio_pread_1 (self,
+			       readahead_cache->fd,
+			       readahead_cache->buf, readahead_cache->bufsize,
+			       readahead_cache->offset, remote_errno);
+  if (ret <= 0)
+    {
+      readahead_cache_invalidate ();
+      return ret;
+    }
+
+  readahead_cache->bufsize = ret;
+  return remote_hostio_pread_from_cache (readahead_cache, fd,
+					 read_buf, len, offset);
+}
+
 /* Implementation of to_fileio_close.  */
 
 static int
@@ -10452,6 +10545,9 @@ remote_hostio_close (struct target_ops *self, int fd, int *remote_errno)
   char *p = rs->buf;
   int left = get_remote_packet_size () - 1;
 
+  if (readahead_cache != NULL && readahead_cache->fd == fd)
+    readahead_cache_invalidate ();
+
   remote_buffer_add_string (&p, &left, "vFile:close:");
 
   remote_buffer_add_int (&p, &left, fd);
-- 
1.9.3

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

[Gary Benson]

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

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

[Gary Benson]

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
> 

-- 
http://gbenson.net/

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

[Pedro Alves]

On 08/19/2015 11:41 AM, 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?

OK.

Thanks,
Pedro Alves

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

[Pedro Alves]

On 08/19/2015 11:50 AM, Gary Benson wrote:
> 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.

Yes please.  If you could then also test the readahead cache patch
I sent, I'd be also interested in whether it further makes a
difference for you.

Thanks,
Pedro Alves

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

[Gary Benson]

Pedro Alves wrote:
> On 08/19/2015 11:41 AM, 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?
> 
> OK.

Pushed to both.

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> BTW, the transfers seem to be always interruptible for me, even without
> Gary's patch, and even the slow ones.

Ok, I'm glad I'm not the only one :)

> From d426ce0a36830378a8ec2e2cbdd94d9fcb47b891 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 18 Aug 2015 23:27:20 +0100
> Subject: [PATCH 3/3] add readahead cache to gdb's vFile:pread

I tried this out on its own, and got similar hit/miss ratios, so it
looks like a good addition.  Comments below.

> diff --git a/gdb/remote.c b/gdb/remote.c
> index a839adf..8a739c8 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -10311,6 +10311,26 @@ remote_hostio_send_command (int command_bytes, int which_packet,
>    return ret;
>  }
>  
> +struct readahead_cache
> +{
> +  int fd;
> +  gdb_byte *buf;
> +  ULONGEST offset;
> +  size_t bufsize;
> +};
> +
> +static struct readahead_cache *readahead_cache;

Would this be better in struct remote_state (and maybe not allocated,
just inlined in the main remote_state--it's only 16 or 32 bytes)?

> @@ -10325,6 +10345,8 @@ remote_hostio_set_filesystem
>    char arg[9];
>    int ret;
>  
> +  readahead_cache_invalidate ();
> +
>    if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
>      return 0;
>  

This isn't necessary, file descriptors persist across setns calls.

> +      if (remote_debug)
> +	fprintf_unfiltered (gdb_stdlog,
> +			    "readahead cache hit %d\n",
> +			    ++readahead_cache_hit_count);
and
> +  if (remote_debug)
> +    fprintf_unfiltered (gdb_stdlog, "readahead cache miss %d\n",
> +			++readahead_cache_miss_count);

These only update the counters when debug printing is switched on.
Is this what you intended?

> Looking at "set debug remote 1" output, I noticed that gdb often
> issues a sequence of small vFile:pread requests for the same file.
> Ctrl-c'ing gdb while it was doing that and getting a backtrace shows
> that those requests often came from bfd.  So I thought that maybe
> adding a readahead buffer/cache to gdb's vFile:pread requests could
> help.  And indeed, that dropped the time for the same use case
> further down to:

Another thing I noticed when I was testing the warning messages patch
is that GDB seems to (always? often?) open each file twice:

    Remote debugging using :9999
  * Reading /home/gary/work/archer/fast-transfer/build64/gdb/gdbserver/gdbserver from remote target...
    warning: File transfers from remote targets can be slow. Use "set sysroot" with no arguments to access files locally instead.
  * Reading /home/gary/work/archer/fast-transfer/build64/gdb/gdbserver/gdbserver from remote target...
    Reading symbols from target:/home/gary/work/archer/fast-transfer/build64/gdb/gdbserver/gdbserver...done.
  * Reading /lib64/ld-linux-x86-64.so.2 from remote target...
  * Reading /lib64/ld-linux-x86-64.so.2 from remote target...
    ...etc

Figuring out why this is happening could save us some more time.

Cheers,
Gary

--
http://gbenson.net/

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

[Sandra Loosemore]

On 08/19/2015 04:50 AM, Gary Benson wrote:
> 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.

Wow, this patch made a big improvement!  On the nios2 board the startup 
took 18 seconds the first time and 10 seconds on subsequent attempts -- 
probably some NFS-level caching?  On the PandaBoard it was 3 seconds or 
less.

On 08/19/2015 07:42 AM, Gary Benson wrote:
> Pedro Alves wrote:
>> BTW, the transfers seem to be always interruptible for me, even without
>> Gary's patch, and even the slow ones.
>
> Ok, I'm glad I'm not the only one :)

Unfortunately, I still can't see that ^C is doing anything useful for 
me.  It is not coming back to a gdb prompt any sooner and "info 
sharedlibrary" afterwards prints the same thing whether or not I've 
tried to interrupt it.  This was with unpatched FSF trunk.  How am I 
supposed to tell whether ^C did anything?  How are you guys telling that 
it is doing something useful?  Is there supposed to be some sort of 
message?  If the file transfer from the target is aborted, I think it 
should say that.

I'm also not seeing the warning on the initial sysroot file transfer 
from the target.  I've lost track of this; was that patch not 
approved/committed yet?

-Sandra

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

[Gary Benson]

Sandra Loosemore wrote:
> On 08/19/2015 04:50 AM, Gary Benson wrote:
> > 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.
> 
> Wow, this patch made a big improvement!  On the nios2 board the
> startup took 18 seconds the first time and 10 seconds on subsequent
> attempts -- probably some NFS-level caching?  On the PandaBoard it
> was 3 seconds or less.

Great :)  It's in master and 7.10 now.

Could you try Pedro's readahead patch too?  It's the third one here:

https://sourceware.org/ml/gdb-patches/2015-08/msg00489.html

Maybe with the readahead_cache_invalidate in
remote_hostio_set_filesystem removed?  I'm interested to see if that
helps any.  You don't need the other two patches in that message.

> On 08/19/2015 07:42 AM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > BTW, the transfers seem to be always interruptible for me, even
> > > without Gary's patch, and even the slow ones.
> >
> > Ok, I'm glad I'm not the only one :)
> 
> Unfortunately, I still can't see that ^C is doing anything useful
> for me.  It is not coming back to a gdb prompt any sooner and "info
> sharedlibrary" afterwards prints the same thing whether or not I've
> tried to interrupt it.  This was with unpatched FSF trunk.  How am I
> supposed to tell whether ^C did anything?  How are you guys telling
> that it is doing something useful?  Is there supposed to be some
> sort of message?  If the file transfer from the target is aborted, I
> think it should say that.

It stops immediately when I try it.  I'm not familiar with ^C handling
either, so I don't know what would affect it.  Pedro, could this be
a sync/async thing, or something to do with all-stop/non-stop?

> I'm also not seeing the warning on the initial sysroot file transfer
> from the target.  I've lost track of this; was that patch not
> approved/committed yet?

It's not committed yet.  It was part of a pair of patches, the other
being the interruptibility patch that didn't work for you.

If somebody approves the warning patch alone it I'll push it to master
and the branch.  I'm slightly wary about adding a warning as one of
the Valgrind people said their testsuite started tripping over another
warning I recently added, but I think making sure users know what's
happening is more important... testsuites can be fixed :)

Thanks,
Gary

-- 
http://gbenson.net/

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

[Sandra Loosemore]

On 08/19/2015 11:20 AM, Gary Benson wrote:
> Sandra Loosemore wrote:
>> On 08/19/2015 04:50 AM, Gary Benson wrote:
>>> 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.
>>
>> Wow, this patch made a big improvement!  On the nios2 board the
>> startup took 18 seconds the first time and 10 seconds on subsequent
>> attempts -- probably some NFS-level caching?  On the PandaBoard it
>> was 3 seconds or less.
>
> Great :)  It's in master and 7.10 now.
>
> Could you try Pedro's readahead patch too?  It's the third one here:
>
> https://sourceware.org/ml/gdb-patches/2015-08/msg00489.html
>
> Maybe with the readahead_cache_invalidate in
> remote_hostio_set_filesystem removed?  I'm interested to see if that
> helps any.  You don't need the other two patches in that message.

This didn't do anything to help; the startup time is still 10-11 seconds.

>> On 08/19/2015 07:42 AM, Gary Benson wrote:
>>> Pedro Alves wrote:
>>>> BTW, the transfers seem to be always interruptible for me, even
>>>> without Gary's patch, and even the slow ones.
>>>
>>> Ok, I'm glad I'm not the only one :)
>>
>> Unfortunately, I still can't see that ^C is doing anything useful
>> for me.  It is not coming back to a gdb prompt any sooner and "info
>> sharedlibrary" afterwards prints the same thing whether or not I've
>> tried to interrupt it.  This was with unpatched FSF trunk.  How am I
>> supposed to tell whether ^C did anything?  How are you guys telling
>> that it is doing something useful?  Is there supposed to be some
>> sort of message?  If the file transfer from the target is aborted, I
>> think it should say that.
>
> It stops immediately when I try it.  I'm not familiar with ^C handling
> either, so I don't know what would affect it.  Pedro, could this be
> a sync/async thing, or something to do with all-stop/non-stop?

Well, here is a clue.  I tried logging the RSP traffic so I could 
compare the interrupted and non-interrupted behavior.  Aside from 
differences in PID numbers (etc), the *only* difference is that the log 
from the interrupted run shows that it's sending a \x03 character to the 
remote target after it does a vCont, after it has already read the 
entire contents of libc.so.  Here's the relevant snippet from the 
transcript:

w $vFile:pread:5,3fff,b77987#e0
r $F3fca; [...]
w $qSymbol::#5b
r $qSymbol:6e70746c5f76657273696f6e#13
w $qSymbol::6e70746c5f76657273696f6e#4d
r $OK#9a
w $m2760,4#9c
r $fa6f3b00#58
w $m2ab0374c,4#f3
r $04fcffde#c2
w $m2ab0374c,4#f3
r $04fcffde#c2
w $m2ab0374c,4#f3
r $04fcffde#c2
w $m2aab6d98,4#2e
r $fa6f3b00#58
w $m2aab6d94,4#2a
r $3ae83e10#2a
w $X2aab6d98,4::(\x00\xf8#ad
r $OK#9a
w $m2aab6d98,4#2e
r $3a2800f8#fc
w $g#67
r $0*,2c84ac2a4084ac2a0*-10**58020100986dab2a0866aa2a030*"9a080* 
1c70ac2a3ceab42
a84b7c22a0*"004084ac2a8883ac2a4084ac2a646eac2a487bac2a0070ac2a3827c32a0*4f8f6fe7
f98f7fe7f986dab2a0*"00d0b1aa2a986dab2a0*}0*;#15
w $m2aaab1d0,4#49
r $17a082b0#f5
w $m2aaab1d0,4#49
r $17a082b0#f5
w $X2aaab1d0,4:\xfao;\x00#12
r $OK#9a
w $QPassSignals:#f3
r $OK#9a
w $vCont;c:p33e.33e#16\x03  <=== here
r $T05swbreak:;1b:f8f6fe7f;20:d0b1aa2a;thread:p33e.33e;core:0;#89

FAOD, this is a trunk checkout with the patch described above applied 
and nothing else.  And, the exact sequence of commands I'm using to try 
to reproduce this is

<target>-gdb a.out
target remote ...
break main
c
^C

-Sandra

Re: [PATCH 0/2] Better handling of slow remote transfers

[Pedro Alves]

On 08/19/2015 02:42 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> BTW, the transfers seem to be always interruptible for me, even without
>> Gary's patch, and even the slow ones.
> 
> Ok, I'm glad I'm not the only one :)
> 
>> From d426ce0a36830378a8ec2e2cbdd94d9fcb47b891 Mon Sep 17 00:00:00 2001
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 18 Aug 2015 23:27:20 +0100
>> Subject: [PATCH 3/3] add readahead cache to gdb's vFile:pread
> 
> I tried this out on its own, and got similar hit/miss ratios, so it
> looks like a good addition.  Comments below.
> 
>> diff --git a/gdb/remote.c b/gdb/remote.c
>> index a839adf..8a739c8 100644
>> --- a/gdb/remote.c
>> +++ b/gdb/remote.c
>> @@ -10311,6 +10311,26 @@ remote_hostio_send_command (int command_bytes, int which_packet,
>>    return ret;
>>  }
>>  
>> +struct readahead_cache
>> +{
>> +  int fd;
>> +  gdb_byte *buf;
>> +  ULONGEST offset;
>> +  size_t bufsize;
>> +};
>> +
>> +static struct readahead_cache *readahead_cache;
> 
> Would this be better in struct remote_state (and maybe not allocated,
> just inlined in the main remote_state--it's only 16 or 32 bytes)?

Definitely.

(At first I made the cache invalidation free
the cache object (and set it NULL).  Then I when I saw
that the CPU usage went up, I made the invalidation just
clear fd to -1.  The CPU did not go down.  Then I lowered
back the max packet size and the CPU usage went back down,
but I kept the cache invalidation as it was.)

> 
>> @@ -10325,6 +10345,8 @@ remote_hostio_set_filesystem
>>    char arg[9];
>>    int ret;
>>  
>> +  readahead_cache_invalidate ();
>> +
>>    if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE)
>>      return 0;
>>  
> 
> This isn't necessary, file descriptors persist across setns calls.

OK.

> 
>> +      if (remote_debug)
>> +	fprintf_unfiltered (gdb_stdlog,
>> +			    "readahead cache hit %d\n",
>> +			    ++readahead_cache_hit_count);
> and
>> +  if (remote_debug)
>> +    fprintf_unfiltered (gdb_stdlog, "readahead cache miss %d\n",
>> +			++readahead_cache_miss_count);
> 
> These only update the counters when debug printing is switched on.
> Is this what you intended?

Hey, that was a quick and dirty patch.  :-)

Here's a cleaned up version.  WDYT?

---
From d853f786962906127da99c0ac42745447a9bbd05 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 20 Aug 2015 14:15:48 +0100
Subject: [PATCH] Add readahead cache to gdb's vFile:pread

This patch almost halves the time it takes to "target remote + run to
main" on a higher-latency connection.

E.g., I've got a ping time of ~85ms to an x86-64 machine on the gcc
compile farm (almost 2000km away from me), and I'm behind a ~16Mbit
ADSL.  When I connect to a gdbserver debugging itself on that machine
and run to main, it takes almost 55 seconds:

 [palves@gcc76] $ ./gdbserver :9999 ./gdbserver
 [palves@home] $ ssh -L 9999:localhost:9999 gcc76.fsffrance.org
 [palves@home] time ./gdb -data-directory=data-directory -ex "tar rem :9999" -ex "b main" -ex "c" -ex "set confirm off" -ex "quit"

 Pristine gdb 7.10.50.20150820-cvs gets us:
 ...
 Remote debugging using :9999
 Reading symbols from target:/home/palves/gdb/build/gdb/gdbserver/gdbserver...done.
 Reading symbols from target:/lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
 0x00007ffff7ddd190 in ?? () from target:/lib64/ld-linux-x86-64.so.2
 Breakpoint 1 at 0x41200c: file ../../../src/gdb/gdbserver/server.c, line 3635.
 Continuing.

 Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
 3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
 /home/palves/gdb/build/gdb/gdbserver/gdbserver: No such file or directory.

 real    0m54.803s
 user    0m0.329s
 sys     0m0.064s

While the readahead cache added by this patch, it drops to:

 real    0m29.462s
 user    0m0.454s
 sys     0m0.054s

I added a few counters to show cache hit/miss, and got:

 readahead cache miss 142
 readahead cache hit 310

Tested on x86_64 Fedora 20.

gdb/ChangeLog:
2015-08-20  Pedro Alves  <palves@redhat.com>

	* remote.c (struct readahead_cache): New.
	(struct remote_state) <readahead_cache>: New field.
	(remote_open_1): Invalidate the cache.
	(readahead_cache_invalidate, readahead_cache_invalidate_fd): New
	functions.
	(remote_hostio_pwrite): Invalidate the readahead cache.
	(remote_hostio_pread): Rename to ...
	(remote_hostio_pread_vFile): ... this.
	(remote_hostio_pread_from_cache): New function.
	(remote_hostio_pread): Reimplement.
	(remote_hostio_close): Invalidate the readahead cache.
---
 gdb/remote.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 135 insertions(+), 4 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 4e483fd..bfa5469 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -226,6 +226,8 @@ static int remote_can_run_breakpoint_commands (struct target_ops *self);
 
 static void remote_btrace_reset (void);
 
+static void readahead_cache_invalidate (void);
+
 /* For "remote".  */
 
 static struct cmd_list_element *remote_cmdlist;
@@ -262,6 +264,29 @@ typedef unsigned char threadref[OPAQUETHREADBYTES];
 
 #define MAXTHREADLISTRESULTS 32
 
+/* Data for the vFile:pread readahead cache.  */
+
+struct readahead_cache
+{
+  /* The file descriptor for the file that is being cached.  -1 if the
+     cache is invalid.  */
+  int fd;
+
+  /* The offset into the file that the cache buffer corresponds
+     to.  */
+  ULONGEST offset;
+
+  /* The buffer holding the cache contents.  */
+  gdb_byte *buf;
+  /* The buffer's size.  We try to read as much as fits into a packet
+     at a time.  */
+  size_t bufsize;
+
+  /* Cache hit and miss counters.  */
+  ULONGEST hit_count;
+  ULONGEST miss_count;
+};
+
 /* Description of the remote protocol state for the currently
    connected target.  This is per-target state, and independent of the
    selected architecture.  */
@@ -381,6 +406,14 @@ struct remote_state
      Initialized to -1 to indicate that no "vFile:setfs:" packet
      has yet been sent.  */
   int fs_pid;
+
+  /* A readahead cache for vFile:pread.  Often, reading a binary
+     involves a sequence of small reads.  E.g., when parsing an ELF
+     file.  A readahead cache helps mostly the case of remote
+     debugging on a connection with higher latency, due to the
+     request/reply nature of the RSP.  We only cache data for a single
+     file descriptor at a time.  */
+  struct readahead_cache readahead_cache;
 };
 
 /* Private data that we'll store in (struct thread_info)->private.  */
@@ -4498,6 +4531,8 @@ remote_open_1 (const char *name, int from_tty,
   rs->use_threadinfo_query = 1;
   rs->use_threadextra_query = 1;
 
+  readahead_cache_invalidate ();
+
   if (target_async_permitted)
     {
       /* With this target we start out by owning the terminal.  */
@@ -10329,6 +10364,27 @@ remote_hostio_send_command (int command_bytes, int which_packet,
   return ret;
 }
 
+/* Invalidate the readahead cache.  */
+
+static void
+readahead_cache_invalidate (void)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  rs->readahead_cache.fd = -1;
+}
+
+/* Invalidate the readahead cache if it is holding data for FD.  */
+
+static void
+readahead_cache_invalidate_fd (int fd)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (rs->readahead_cache.fd == fd)
+    rs->readahead_cache.fd = -1;
+}
+
 /* Set the filesystem remote_hostio functions that take FILENAME
    arguments will use.  Return 0 on success, or -1 if an error
    occurs (and set *REMOTE_ERRNO).  */
@@ -10407,6 +10463,8 @@ remote_hostio_pwrite (struct target_ops *self,
   int left = get_remote_packet_size ();
   int out_len;
 
+  readahead_cache_invalidate_fd (fd);
+
   remote_buffer_add_string (&p, &left, "vFile:pwrite:");
 
   remote_buffer_add_int (&p, &left, fd);
@@ -10422,12 +10480,13 @@ remote_hostio_pwrite (struct target_ops *self,
 				     remote_errno, NULL, NULL);
 }
 
-/* Implementation of to_fileio_pread.  */
+/* Helper for the implementation of to_fileio_pread.  Read the file
+   from the remote side with vFile:pread.  */
 
 static int
-remote_hostio_pread (struct target_ops *self,
-		     int fd, gdb_byte *read_buf, int len,
-		     ULONGEST offset, int *remote_errno)
+remote_hostio_pread_vFile (struct target_ops *self,
+			   int fd, gdb_byte *read_buf, int len,
+			   ULONGEST offset, int *remote_errno)
 {
   struct remote_state *rs = get_remote_state ();
   char *p = rs->buf;
@@ -10461,6 +10520,76 @@ remote_hostio_pread (struct target_ops *self,
   return ret;
 }
 
+/* Serve pread from the readahead cache.  Returns number of bytes
+   read, or 0 if the request can't be served from the cache.  */
+
+static int
+remote_hostio_pread_from_cache (struct remote_state *rs,
+				int fd, gdb_byte *read_buf, size_t len,
+				ULONGEST offset)
+{
+  struct readahead_cache *cache = &rs->readahead_cache;
+
+  if (cache->fd == fd
+      && cache->offset <= offset
+      && offset < cache->offset + cache->bufsize)
+    {
+      ULONGEST max = cache->offset + cache->bufsize;
+
+      if (offset + len > max)
+	len = max - offset;
+
+      memcpy (read_buf, cache->buf + offset - cache->offset, len);
+      return len;
+    }
+
+  return 0;
+}
+
+/* Implementation of to_fileio_pread.  */
+
+static int
+remote_hostio_pread (struct target_ops *self,
+		     int fd, gdb_byte *read_buf, int len,
+		     ULONGEST offset, int *remote_errno)
+{
+  int ret;
+  struct remote_state *rs = get_remote_state ();
+  struct readahead_cache *cache = &rs->readahead_cache;
+
+  ret = remote_hostio_pread_from_cache (rs, fd, read_buf, len, offset);
+  if (ret > 0)
+    {
+      cache->hit_count++;
+
+      if (remote_debug)
+	fprintf_unfiltered (gdb_stdlog, "readahead cache hit %s\n",
+			    pulongest (cache->hit_count));
+      return ret;
+    }
+
+  cache->miss_count++;
+  if (remote_debug)
+    fprintf_unfiltered (gdb_stdlog, "readahead cache miss %s\n",
+			pulongest (cache->miss_count));
+
+  cache->fd = fd;
+  cache->offset = offset;
+  cache->bufsize = get_remote_packet_size ();
+  cache->buf = xrealloc (cache->buf, cache->bufsize);
+
+  ret = remote_hostio_pread_vFile (self, cache->fd, cache->buf, cache->bufsize,
+				   cache->offset, remote_errno);
+  if (ret <= 0)
+    {
+      readahead_cache_invalidate_fd (fd);
+      return ret;
+    }
+
+  cache->bufsize = ret;
+  return remote_hostio_pread_from_cache (rs, fd, read_buf, len, offset);
+}
+
 /* Implementation of to_fileio_close.  */
 
 static int
@@ -10470,6 +10599,8 @@ remote_hostio_close (struct target_ops *self, int fd, int *remote_errno)
   char *p = rs->buf;
   int left = get_remote_packet_size () - 1;
 
+  readahead_cache_invalidate_fd (fd);
+
   remote_buffer_add_string (&p, &left, "vFile:close:");
 
   remote_buffer_add_int (&p, &left, fd);
-- 
1.9.3

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

[Pedro Alves]

On 08/19/2015 10:12 PM, Sandra Loosemore wrote:
> On 08/19/2015 11:20 AM, Gary Benson wrote:
>> Sandra Loosemore wrote:
>>> On 08/19/2015 04:50 AM, Gary Benson wrote:
>>>> 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.
>>>
>>> Wow, this patch made a big improvement!  On the nios2 board the
>>> startup took 18 seconds the first time and 10 seconds on subsequent
>>> attempts -- probably some NFS-level caching?  On the PandaBoard it
>>> was 3 seconds or less.
>>
>> Great :)  It's in master and 7.10 now.
>>
>> Could you try Pedro's readahead patch too?  It's the third one here:
>>
>> https://sourceware.org/ml/gdb-patches/2015-08/msg00489.html
>>
>> Maybe with the readahead_cache_invalidate in
>> remote_hostio_set_filesystem removed?  I'm interested to see if that
>> helps any.  You don't need the other two patches in that message.
> 
> This didn't do anything to help; the startup time is still 10-11 seconds.

OK, that shows that the issue with that board is not really
roundtrip latency.  Not that surprising given you're connected
through ethernet.  Knowing it didn't slow down is already very
useful.

Thanks,
Pedro Alves

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

[Pedro Alves]

On 08/19/2015 10:12 PM, Sandra Loosemore wrote:

>>> Unfortunately, I still can't see that ^C is doing anything useful
>>> for me.  It is not coming back to a gdb prompt any sooner and "info
>>> sharedlibrary" afterwards prints the same thing whether or not I've
>>> tried to interrupt it.  This was with unpatched FSF trunk.  How am I
>>> supposed to tell whether ^C did anything?  How are you guys telling
>>> that it is doing something useful?  Is there supposed to be some
>>> sort of message?  If the file transfer from the target is aborted, I
>>> think it should say that.

For me, it just says the usual "Quit":

$ gdb -ex "tar rem :9999"
...
Remote debugging using :9999
^CQuit
(gdb)

That interrupts the main executable transfer.  But from below, I
see we weren't testing the same exact thing.

> Well, here is a clue.  I tried logging the RSP traffic so I could 
> compare the interrupted and non-interrupted behavior.  Aside from 
> differences in PID numbers (etc), the *only* difference is that the log 
> from the interrupted run shows that it's sending a \x03 character to the 
> remote target after it does a vCont, after it has already read the 
> entire contents of libc.so.  Here's the relevant snippet from the 
> transcript:

...

> w $QPassSignals:#f3
> r $OK#9a
> w $vCont;c:p33e.33e#16\x03  <=== here
> r $T05swbreak:;1b:f8f6fe7f;20:d0b1aa2a;thread:p33e.33e;core:0;#89

OK, so ctrl-c while the program is running just causes the target
to report a SIGINT stop.  If you ctrl-c at that time, then it should
make no difference whether GDB is retrieving remote files or not.

The trouble is ctrl-c after the target has stopped for the shared
library load event, and gdb is busy parsing the shared library
(elf headers, debug info, etc.), which involves reading chunks of
the file out of the target side with the vFile:pread packets.

> 
> FAOD, this is a trunk checkout with the patch described above applied 
> and nothing else.  And, the exact sequence of commands I'm using to try 
> to reproduce this is
> 
> <target>-gdb a.out
> target remote ...
> break main
> c
> ^C
> 

Thanks.  Given the recipe, it's easy to reproduce.  Gary, can you
check whether you also see this?

(this is again remote debugging gcc76 on the gcc compile farm
from my office machine)

$ gdb -ex "set verbose on" /tmp/gdbserver -ex "tar rem :9999"
GNU gdb (GDB) 7.10.50.20150820-cvs
Reading symbols from /tmp/gdbserver...done.
Remote debugging using :9999
Created trace state variable $trace_timestamp for target's variable 1.
Reading symbols from target:/lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
Reading symbols from system-supplied DSO at 0x7ffff7ff8000...(no debugging symbols found)...done.
0x00007ffff7ddd190 in ?? () from target:/lib64/ld-linux-x86-64.so.2
(gdb) b main
Breakpoint 1 at 0x41200c: file ../../../src/gdb/gdbserver/server.c, line 3635.
(gdb) c
Continuing.
^CReading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done.
Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.

Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
(gdb)

There's a "^C" right after "Continuing.", but it was ignored.


Trying again, but this time debugging gdb, I get:

$ gdb --args $g -ex "set verbose on" /tmp/gdbserver  -ex "tar rem :9999" -ex "b main"
...
Breakpoint 1 at 0x41200c: file ../../../src/gdb/gdbserver/server.c, line 3635.
(gdb) c
Continuing.
Reading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done.
Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...^C
Program received signal SIGINT, Interrupt.
0x0000003615eec5d3 in __select_nocancel () at ../sysdeps/unix/syscall-template.S:81
81      T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)

stepping into the signal handler is a good way to find what
handler is installed:

(top-gdb) handle SIGINT pass
SIGINT is used by the debugger.
Are you sure you want to change it? (y or n) y
Signal        Stop      Print   Pass to program Description
SIGINT        Yes       Yes     Yes             Interrupt
(top-gdb) s
async_handle_remote_sigint (sig=0) at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:5202
5202    {

That is going to send a ctrl-c request to the target.  Let's resume.

(top-gdb) c
Continuing.
(no debugging symbols found)...done.

Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
(gdb)

Again, now with getting a backtrace on that SIGINT:

(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0x0000003615eec5d3 in __select_nocancel () at ../sysdeps/unix/syscall-template.S:81
81      T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
Missing separate debuginfos, use: debuginfo-install python-libs-2.7.5-16.fc20.x86_64
(top-gdb) bt
#0  0x0000003615eec5d3 in __select_nocancel () at ../sysdeps/unix/syscall-template.S:81
#1  0x000000000057849c in gdb_select (n=10, readfds=0x7fffffffc310, writefds=0x0, exceptfds=0x7fffffffc390, timeout=0x7fffffffc410)
    at /home/pedro/gdb/mygit/build/../src/gdb/posix-hdep.c:31
#2  0x00000000004b685a in ser_base_wait_for (scb=0x191e2f0, timeout=1) at /home/pedro/gdb/mygit/build/../src/gdb/ser-base.c:224
#3  0x00000000004b6af2 in do_ser_base_readchar (scb=0x191e2f0, timeout=10000) at /home/pedro/gdb/mygit/build/../src/gdb/ser-base.c:335
#4  0x00000000004b6c36 in generic_readchar (scb=0x191e2f0, timeout=10000, do_readchar=0x4b6a9d <do_ser_base_readchar>)
    at /home/pedro/gdb/mygit/build/../src/gdb/ser-base.c:410
#5  0x00000000004b6cb0 in ser_base_readchar (scb=0x191e2f0, timeout=10000) at /home/pedro/gdb/mygit/build/../src/gdb/ser-base.c:437
#6  0x000000000075d5dd in serial_readchar (scb=0x191e2f0, timeout=10000) at /home/pedro/gdb/mygit/build/../src/gdb/serial.c:382
#7  0x00000000004e2174 in readchar (timeout=10000) at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:7593
#8  0x00000000004e2cdd in getpkt_or_notif_sane_1 (buf=0xec2650, sizeof_buf=0xec2658, forever=0, expecting_notif=0, is_notif=0x0)
    at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:8134
#9  0x00000000004e2fa1 in getpkt_sane (buf=0xec2650, sizeof_buf=0xec2658, forever=0) at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:8234
#10 0x00000000004e71ad in remote_hostio_send_command (command_bytes=20, which_packet=12, remote_errno=0x7fffffffc7a4, attachment=0x7fffffffc680,
    attachment_len=0x7fffffffc678) at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:10319
#11 0x00000000004e77d0 in remote_hostio_pread_vFile (self=0xdcf120 <remote_ops>, fd=5, read_buf=0x19ed770 "\024", len=16383, offset=0, remote_errno=0x7fffffffc7a4)
    at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:10508
#12 0x00000000004e7a76 in remote_hostio_pread (self=0xdcf120 <remote_ops>, fd=5, read_buf=0x7fffffffc940 "", len=64, offset=0, remote_errno=0x7fffffffc7a4)
    at /home/pedro/gdb/mygit/build/../src/gdb/remote.c:10581
#13 0x0000000000691b98 in target_fileio_pread (fd=0, read_buf=0x7fffffffc940 "", len=64, offset=0, target_errno=0x7fffffffc7a4)
    at /home/pedro/gdb/mygit/build/../src/gdb/target.c:2853
#14 0x0000000000678c68 in gdb_bfd_iovec_fileio_pread (abfd=0x1a63240, stream=0x19203a0, buf=0x7fffffffc940, nbytes=64, offset=0)
    at /home/pedro/gdb/mygit/build/../src/gdb/gdb_bfd.c:294
#15 0x00000000007f7c14 in opncls_bread (abfd=0x1a63240, buf=0x7fffffffc940, nbytes=64) at /home/pedro/gdb/mygit/build/../src/bfd/opncls.c:503
#16 0x00000000007f21a8 in bfd_bread (ptr=0x7fffffffc940, size=64, abfd=0x1a63240) at /home/pedro/gdb/mygit/build/../src/bfd/bfdio.c:196
#17 0x000000000080fe2f in bfd_elf64_object_p (abfd=0x1a63240) at /home/pedro/gdb/mygit/build/../src/bfd/elfcode.h:503
#18 0x00000000007f4acb in bfd_check_format_matches (abfd=0x1a63240, format=bfd_object, matching=0x0) at /home/pedro/gdb/mygit/build/../src/bfd/format.c:305
#19 0x00000000007f4545 in bfd_check_format (abfd=0x1a63240, format=bfd_object) at /home/pedro/gdb/mygit/build/../src/bfd/format.c:94
#20 0x00000000007872f2 in solib_bfd_open (pathname=0x1a5dac0 "/lib/x86_64-linux-gnu/libdl.so.2") at /home/pedro/gdb/mygit/build/../src/gdb/solib.c:504
#21 0x0000000000787418 in solib_map_sections (so=0x1a62ba0) at /home/pedro/gdb/mygit/build/../src/gdb/solib.c:544
#22 0x0000000000787c8f in update_solib_list (from_tty=0, target=0xded2a0 <current_target>) at /home/pedro/gdb/mygit/build/../src/gdb/solib.c:892
#23 0x0000000000787e7c in solib_add (pattern=0x0, from_tty=0, target=0xded2a0 <current_target>, readsyms=1) at /home/pedro/gdb/mygit/build/../src/gdb/solib.c:991
#24 0x00000000007886ba in handle_solib_event () at /home/pedro/gdb/mygit/build/../src/gdb/solib.c:1340
#25 0x00000000005b0c19 in bpstat_stop_status (aspace=0x1095140, bp_addr=140737351955008, ptid=..., ws=0x7fffffffd110)
    at /home/pedro/gdb/mygit/build/../src/gdb/breakpoint.c:5612
#26 0x0000000000638379 in handle_signal_stop (ecs=0x7fffffffd0f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:5599
#27 0x0000000000637167 in handle_inferior_event_1 (ecs=0x7fffffffd0f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:5028
#28 0x0000000000637209 in handle_inferior_event (ecs=0x7fffffffd0f0) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:5053
...


That is, the target stopped for the shared library event, and GDB is busy
reading the shared libraries.

While this is ongoing, the inferior still owns terminal _input_
(including ctrl-c):

(top-gdb) p terminal_state
$1 = terminal_is_ours_for_output


The "step into signal handler" above shows that at the time the problematic
ctrl-c is pressed, the sigint handler that is registered is
async_handle_remote_sigint.  That's the function that sends the remote-interrupt
packet (\x03) to the remote side.  But the target won't be able to report
a SIGINT until the next time it is resumed (and the kernel reports a SIGINT
to ptrace).  So supposedly, the ctrl-c should be ignored until the
whole file is transferred and gdb re-resumes the target.

I don't see a quick, easy and good way around waiting for the whole file to read.
Having GDB handle ctrl-c differently while it is handling internal events
is already a source of trouble, and the fix always seemed to me to be to make ctrl-c
work like what is happening here.  Specifically, I mean that e.g., say that the
user sets a conditional breakpoint that is evaluated on gdb's side, and then the
target stops for that conditional breakpoint, gdb evaluates the expression,
and then resumes the target again, on and on.  If the user presses ctrl-c just
while the conditional breakpoint's expression is being evaluated, and something along
that code path called target_terminal_ours (thus pulling input/ctrl-c to the
regular "Quit" SIGINT handler), gdb behaves differently (shows a "Quit" and the
debug session ends up likely broken) from when the ctrl-c is pressed while the target
is really running.  I'd argue that the ctrl-c in both cases should be passed
down all the way to the target the same way, and that any internal stop and
breakpoint condition evaluation is magic that should be hidden.  Just like
what is happening here with file reading.

Though having said that, it does look like even that isn't working properly,
as I'd expect this:

(top-gdb) c
Continuing.
(no debugging symbols found)...done.

Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
(gdb)

to be slow (that is, the file reading isn't really interrupted), but then
the target stops with SIGINT as soon as gdb resumes it again after reading
the DSOs.  But it is reaching main anyway, and there's no sign
of "Program stopped with SIGINT"...

Not sure where the bug is.  It may be on the gdbserver side.

Thanks,
Pedro Alves

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

[Pedro Alves]

On 08/20/2015 04:51 PM, Pedro Alves wrote:

> I don't see a quick, easy and good way around waiting for the whole file to read.
> Having GDB handle ctrl-c differently while it is handling internal events
> is already a source of trouble, and the fix always seemed to me to be to make ctrl-c
> work like what is happening here.  Specifically, I mean that e.g., say that the
> user sets a conditional breakpoint that is evaluated on gdb's side, and then the
> target stops for that conditional breakpoint, gdb evaluates the expression,
> and then resumes the target again, on and on.  If the user presses ctrl-c just
> while the conditional breakpoint's expression is being evaluated, and something along
> that code path called target_terminal_ours (thus pulling input/ctrl-c to the
> regular "Quit" SIGINT handler), gdb behaves differently (shows a "Quit" and the
> debug session ends up likely broken) from when the ctrl-c is pressed while the target
> is really running.  I'd argue that the ctrl-c in both cases should be passed
> down all the way to the target the same way, and that any internal stop and
> breakpoint condition evaluation is magic that should be hidden.  Just like
> what is happening here with file reading.
> 
> Though having said that, it does look like even that isn't working properly,
> as I'd expect this:
> 
> (top-gdb) c
> Continuing.
> (no debugging symbols found)...done.
> 
> Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
> 3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
> (gdb)
> 
> to be slow (that is, the file reading isn't really interrupted), but then
> the target stops with SIGINT as soon as gdb resumes it again after reading
> the DSOs.  But it is reaching main anyway, and there's no sign
> of "Program stopped with SIGINT"...
> 
> Not sure where the bug is.  It may be on the gdbserver side.

OK, so gdbserver receives the \003, but since the target is stopped,
the normal packet processing loop discards the \003, as it doesn't
look like a start of a RSP packet frame (that is, it isn't a $).

I'm thinking that maybe the best way to handle this may be to still
leave SIGINT forwarding to the target, so that in case gdb re-resumes
the target quick enough, the ctrl-c turns into a real SIGINT on the
target.  But then if it takes long for gdb or gdbserver or the target
to react to the ctrl-c, then the user presses ctrl-c again, and gdb
shows the old:

  Interrupted while waiting for the program.
  Give up (and stop debugging it)? (y or n) 

AFAICS, that query is never issued anywhere if the target
is async.  And the remote target is always async nowadays.

To make that query appear promptly, we'd hook it to the
QUIT macro.  Something like this:

(gdb) c
Continuing.
Reading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done.
Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
^CInterrupted while waiting for the program.
Give up (and stop debugging it)? (y or n) y
Quit
(gdb) 

Could you try the hacky patch below just to see if it makes a
difference to you?  It seems that GDB still doesn't react
as soon as it could, but I'd guess that Gary's previous patch
to add a QUIT around vFile:pread's would fix that.

-- 
From 9fc660f2b74bf660965fc97cad1932e458521b79 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 20 Aug 2015 17:24:57 +0100
Subject: [PATCH] set_quit_flag

---
 gdb/extension.c |  4 ++++
 gdb/remote.c    | 29 ++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/gdb/extension.c b/gdb/extension.c
index dac203b..d754eb1 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -822,6 +822,8 @@ set_quit_flag (void)
     quit_flag = 1;
 }
 
+void target_check_quit_flag (void);
+
 /* Return true if the quit flag has been set, false otherwise.
    Note: The flag is cleared as a side-effect.
    The flag is checked in all extension languages that support cooperative
@@ -833,6 +835,8 @@ check_quit_flag (void)
   int i, result = 0;
   const struct extension_language_defn *extlang;
 
+  target_check_quit_flag ();
+
   ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
     {
       if (extlang->ops->check_quit_flag != NULL)
diff --git a/gdb/remote.c b/gdb/remote.c
index bfa5469..e0c61b7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5206,6 +5206,8 @@ async_handle_remote_sigint (int sig)
   gdb_call_async_signal_handler (async_sigint_remote_token, 0);
 }
 
+static int sigint_twice = 0;
+
 /* Signal handler for SIGINT, installed after SIGINT has already been
    sent once.  It will take effect the second time that the user sends
    a ^C.  */
@@ -5215,6 +5217,19 @@ async_handle_remote_sigint_twice (int sig)
   signal (sig, async_handle_remote_sigint);
   /* See note in async_handle_remote_sigint.  */
   gdb_call_async_signal_handler (async_sigint_remote_twice_token, 0);
+  sigint_twice = 1;
+}
+
+void target_check_quit_flag (void);
+
+void
+target_check_quit_flag (void)
+{
+  if (sigint_twice)
+    {
+      sigint_twice = 0;
+      gdb_call_async_signal_handler (async_sigint_remote_twice_token, 1);
+    }
 }
 
 /* Perform the real interruption of the target execution, in response
@@ -5391,20 +5406,12 @@ interrupt_query (void)
 {
   target_terminal_ours ();
 
-  if (target_is_async_p ())
+  if (query (_("Interrupted while waiting for the program.\n\
+Give up (and stop debugging it)? ")))
     {
-      signal (SIGINT, handle_sigint);
+      remote_unpush_target ();
       quit ();
     }
-  else
-    {
-      if (query (_("Interrupted while waiting for the program.\n\
-Give up (and stop debugging it)? ")))
-	{
-	  remote_unpush_target ();
-	  quit ();
-	}
-    }
 
   target_terminal_inferior ();
 }
-- 
1.9.3

Re: [PATCH 0/2] Better handling of slow remote transfers

[Gary Benson]

Pedro Alves wrote:
> Hey, that was a quick and dirty patch.  :-)

I know, no worries :)

> Here's a cleaned up version.  WDYT?
[snip
> This patch almost halves the time it takes to "target remote + run
> to main" on a higher-latency connection.
> 
> E.g., I've got a ping time of ~85ms to an x86-64 machine on the gcc
> compile farm (almost 2000km away from me), and I'm behind a ~16Mbit
> ADSL.  When I connect to a gdbserver debugging itself on that machine
> and run to main, it takes almost 55 seconds:
[snip]
>  real    0m54.803s
>  user    0m0.329s
>  sys     0m0.064s
> 
> While the readahead cache added by this patch, it drops to:
> 
>  real    0m29.462s
>  user    0m0.454s
>  sys     0m0.054s
> 
> I added a few counters to show cache hit/miss, and got:
> 
>  readahead cache miss 142
>  readahead cache hit 310
> 
> Tested on x86_64 Fedora 20.

Very nice, please commit!

Thanks,
Gary

-- 
http://gbenson.net/

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

[Sandra Loosemore]

On 08/20/2015 11:00 AM, Pedro Alves wrote:
> On 08/20/2015 04:51 PM, Pedro Alves wrote:
>
>> I don't see a quick, easy and good way around waiting for the whole file to read.
>> Having GDB handle ctrl-c differently while it is handling internal events
>> is already a source of trouble, and the fix always seemed to me to be to make ctrl-c
>> work like what is happening here.  Specifically, I mean that e.g., say that the
>> user sets a conditional breakpoint that is evaluated on gdb's side, and then the
>> target stops for that conditional breakpoint, gdb evaluates the expression,
>> and then resumes the target again, on and on.  If the user presses ctrl-c just
>> while the conditional breakpoint's expression is being evaluated, and something along
>> that code path called target_terminal_ours (thus pulling input/ctrl-c to the
>> regular "Quit" SIGINT handler), gdb behaves differently (shows a "Quit" and the
>> debug session ends up likely broken) from when the ctrl-c is pressed while the target
>> is really running.  I'd argue that the ctrl-c in both cases should be passed
>> down all the way to the target the same way, and that any internal stop and
>> breakpoint condition evaluation is magic that should be hidden.  Just like
>> what is happening here with file reading.
>>
>> Though having said that, it does look like even that isn't working properly,
>> as I'd expect this:
>>
>> (top-gdb) c
>> Continuing.
>> (no debugging symbols found)...done.
>>
>> Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
>> 3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
>> (gdb)
>>
>> to be slow (that is, the file reading isn't really interrupted), but then
>> the target stops with SIGINT as soon as gdb resumes it again after reading
>> the DSOs.  But it is reaching main anyway, and there's no sign
>> of "Program stopped with SIGINT"...
>>
>> Not sure where the bug is.  It may be on the gdbserver side.
>
> OK, so gdbserver receives the \003, but since the target is stopped,
> the normal packet processing loop discards the \003, as it doesn't
> look like a start of a RSP packet frame (that is, it isn't a $).
>
> I'm thinking that maybe the best way to handle this may be to still
> leave SIGINT forwarding to the target, so that in case gdb re-resumes
> the target quick enough, the ctrl-c turns into a real SIGINT on the
> target.  But then if it takes long for gdb or gdbserver or the target
> to react to the ctrl-c, then the user presses ctrl-c again, and gdb
> shows the old:
>
>    Interrupted while waiting for the program.
>    Give up (and stop debugging it)? (y or n)
>
> AFAICS, that query is never issued anywhere if the target
> is async.  And the remote target is always async nowadays.
>
> To make that query appear promptly, we'd hook it to the
> QUIT macro.  Something like this:
>
> (gdb) c
> Continuing.
> Reading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done.
> Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
> ^CInterrupted while waiting for the program.
> Give up (and stop debugging it)? (y or n) y
> Quit
> (gdb)
>
> Could you try the hacky patch below just to see if it makes a
> difference to you?  It seems that GDB still doesn't react
> as soon as it could, but I'd guess that Gary's previous patch
> to add a QUIT around vFile:pread's would fix that.

Thanks!  The combination of these two patches does make GDB respond 
quickly to the second ^C and abort the file transfer.  However, both GDB 
and gdbserver seem to be left in a wonky state.  On the GDB side, I'm 
seeing:

(gdb) c
Continuing.
^C^CInterrupted while waiting for the program.
Give up (and stop debugging it)? (y or n) Interrupted while waiting for 
the program.
y
You can't do that when your target is `exec'
No registers.
(gdb)

And, meanwhile on the target, gdbserver has done this:

Process a.out created; pid = 856
Listening on port 6789
Remote debugging from host 134.86.188.141
readchar: Got EOF
Remote side has terminated connection.  GDBserver will reopen the 
connection.
/scratch/sandra/nios2-linux-trunk/src/gdb-nios2r2/gdb/gdbserver/../common/cleanups.c:265: 
A problem internal to GDBserver has been detected.
restore_my_cleanups has found a stale cleanup
Listening on port 6789

I was thinking that the "right" behavior here would be for GDB to just 
try to continue without library symbol or debug information if the file 
transfer is interrupted, but a clean shutdown with fewer confusing 
messages would be OK, too.  Especially if we've given users a hint that 
they need "set sysroot", the probable scenario is for the user to start 
over with a fresh GDB and add that command before "target remote".

-Sandra

[pushed] Add readahead cache to gdb's vFile:pread (Re: [PATCH 0/2] Better handling of slow remote transfers)

[Pedro Alves]

On 08/20/2015 07:01 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> Hey, that was a quick and dirty patch.  :-)
> 
> I know, no worries :)
> 
>> Here's a cleaned up version.  WDYT?
> [snip
>> This patch almost halves the time it takes to "target remote + run
>> to main" on a higher-latency connection.
>>
>> E.g., I've got a ping time of ~85ms to an x86-64 machine on the gcc
>> compile farm (almost 2000km away from me), and I'm behind a ~16Mbit
>> ADSL.  When I connect to a gdbserver debugging itself on that machine
>> and run to main, it takes almost 55 seconds:
> [snip]
>>  real    0m54.803s
>>  user    0m0.329s
>>  sys     0m0.064s
>>
>> While the readahead cache added by this patch, it drops to:
>>
>>  real    0m29.462s
>>  user    0m0.454s
>>  sys     0m0.054s
>>
>> I added a few counters to show cache hit/miss, and got:
>>
>>  readahead cache miss 142
>>  readahead cache hit 310
>>
>> Tested on x86_64 Fedora 20.
> 
> Very nice, please commit!

Pushed, master and 7.10.

Thanks,
Pedro Alves

[PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")

[Pedro Alves]

On 08/20/2015 07:21 PM, Sandra Loosemore wrote:
> On 08/20/2015 11:00 AM, Pedro Alves wrote:
>> On 08/20/2015 04:51 PM, Pedro Alves wrote:
>>
>>> I don't see a quick, easy and good way around waiting for the whole file to read.
>>> Having GDB handle ctrl-c differently while it is handling internal events
>>> is already a source of trouble, and the fix always seemed to me to be to make ctrl-c
>>> work like what is happening here.  Specifically, I mean that e.g., say that the
>>> user sets a conditional breakpoint that is evaluated on gdb's side, and then the
>>> target stops for that conditional breakpoint, gdb evaluates the expression,
>>> and then resumes the target again, on and on.  If the user presses ctrl-c just
>>> while the conditional breakpoint's expression is being evaluated, and something along
>>> that code path called target_terminal_ours (thus pulling input/ctrl-c to the
>>> regular "Quit" SIGINT handler), gdb behaves differently (shows a "Quit" and the
>>> debug session ends up likely broken) from when the ctrl-c is pressed while the target
>>> is really running.  I'd argue that the ctrl-c in both cases should be passed
>>> down all the way to the target the same way, and that any internal stop and
>>> breakpoint condition evaluation is magic that should be hidden.  Just like
>>> what is happening here with file reading.
>>>
>>> Though having said that, it does look like even that isn't working properly,
>>> as I'd expect this:
>>>
>>> (top-gdb) c
>>> Continuing.
>>> (no debugging symbols found)...done.
>>>
>>> Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635
>>> 3635    ../../../src/gdb/gdbserver/server.c: No such file or directory.
>>> (gdb)
>>>
>>> to be slow (that is, the file reading isn't really interrupted), but then
>>> the target stops with SIGINT as soon as gdb resumes it again after reading
>>> the DSOs.  But it is reaching main anyway, and there's no sign
>>> of "Program stopped with SIGINT"...
>>>
>>> Not sure where the bug is.  It may be on the gdbserver side.
>>
>> OK, so gdbserver receives the \003, but since the target is stopped,
>> the normal packet processing loop discards the \003, as it doesn't
>> look like a start of a RSP packet frame (that is, it isn't a $).
>>
>> I'm thinking that maybe the best way to handle this may be to still
>> leave SIGINT forwarding to the target, so that in case gdb re-resumes
>> the target quick enough, the ctrl-c turns into a real SIGINT on the
>> target.  But then if it takes long for gdb or gdbserver or the target
>> to react to the ctrl-c, then the user presses ctrl-c again, and gdb
>> shows the old:
>>
>>    Interrupted while waiting for the program.
>>    Give up (and stop debugging it)? (y or n)
>>
>> AFAICS, that query is never issued anywhere if the target
>> is async.  And the remote target is always async nowadays.
>>
>> To make that query appear promptly, we'd hook it to the
>> QUIT macro.  Something like this:
>>
>> (gdb) c
>> Continuing.
>> Reading symbols from target:/lib/x86_64-linux-gnu/libdl.so.2...(no debugging symbols found)...done.
>> Reading symbols from target:/lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
>> ^CInterrupted while waiting for the program.
>> Give up (and stop debugging it)? (y or n) y
>> Quit
>> (gdb)
>>
>> Could you try the hacky patch below just to see if it makes a
>> difference to you?  It seems that GDB still doesn't react
>> as soon as it could, but I'd guess that Gary's previous patch
>> to add a QUIT around vFile:pread's would fix that.
> 
> Thanks!  The combination of these two patches does make GDB respond 
> quickly to the second ^C and abort the file transfer.  However, both GDB 
> and gdbserver seem to be left in a wonky state.  On the GDB side, I'm 
> seeing:
> 
> (gdb) c
> Continuing.
> ^C^CInterrupted while waiting for the program.
> Give up (and stop debugging it)? (y or n) Interrupted while waiting for 
> the program.
> y
> You can't do that when your target is `exec'
> No registers.
> (gdb)

Those are caused by the abrupt disconnection.  This should be triggerable
even without the patch (though of course unexpected disconnections
are supposedly rare).  But please try the new patch below.

> 
> And, meanwhile on the target, gdbserver has done this:
> 
> Process a.out created; pid = 856
> Listening on port 6789
> Remote debugging from host 134.86.188.141
> readchar: Got EOF
> Remote side has terminated connection.  GDBserver will reopen the 
> connection.
> /scratch/sandra/nios2-linux-trunk/src/gdb-nios2r2/gdb/gdbserver/../common/cleanups.c:265: 
> A problem internal to GDBserver has been detected.
> restore_my_cleanups has found a stale cleanup
> Listening on port 6789

Yeah, I see that too.  It's orthogonal though.  Seems to trigger for
all disconnections.

> I was thinking that the "right" behavior here would be for GDB to just 
> try to continue without library symbol or debug information if the file 
> transfer is interrupted, 

Maybe.  I'd want that at least behind a query though.  The patch
below gets you back to the prompt, and given a hint about "set sysroot",
the user can then decide what to do.  I think that's sufficient, at least 
for 7.10.

> but a clean shutdown with fewer confusing 
> messages would be OK, too.  Especially if we've given users a hint that 
> they need "set sysroot", the probable scenario is for the user to start 
> over with a fresh GDB and add that command before "target remote".

Agreed.

From efc760942467848780cab6891e6d5ff4d86487de Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 21 Aug 2015 15:42:48 +0100
Subject: [PATCH] remote: allow aborting long operations (e.g., file transfers)

Currently, when remote debugging, if you type Ctrl-c just while the
target stopped for an internal event, and GDB is busy doing something
that takes a while (e.g., fetching chunks of a shared library off of
the target, with vFile, to process ELF headers and debug info), the
Ctrl-c is lost.

The patch hooks up the QUIT macro to a new target method that lets the
target react to the double-Ctrl-c before the event loop is reached,
which allows reacting to a double-Ctrl-c even when GDB is busy doing
some long operation and not waiting for a stop reply.  That end result
is:

 (gdb) c
 Continuing.
 ^C
 ^C
 Interrupted while waiting for the program.
 Give up waiting? (y or n) y
 Quit
 (gdb) info threads
   Id   Target Id         Frame
 * 1    Thread 11673      0x00007ffff7deb240 in _dl_debug_state () from target:/lib64/ld-linux-x86-64.so.2
 (gdb)

If, however, GDB is waiting for a stop reply (because the target has
been resumed, with e.g., vCont;c), but the target isn't responding, we
now get:

 (gdb) c
 Continuing.
 ^C
 ^C
 The target is not responding to interrupt requests.
 Stop debugging it? (y or n) y
 Disconnected from target.
 (gdb) info threads
 No threads.

This offers to disconnect, because when we're waiting for a stop
reply, there's nothing else we can send the target other than an
interrupt request.  And if that doesn't work, there's nothing else we
can do.

The ctrl-c is presently lost because until we get to a user-visible
stop, the SIGINT handler that is installed is the one that forwards
the interrupt to the remote side, with the \003 "packet" [1].  But,
gdbserver ignores an interrupt request if the program is stopped.
Still, even if it didn't, the server can only report back a
stop-because-of-SIGINT when the program is next resumed.  And it may
take a while to actually re-resume the target.

[1] - In the old sync days, the remote target would react to a
double-Ctrl-c by asking users whether they wanted to give up waiting
and disconnect.  The code is still there, but it it isn't reacheable
on most hosts, which support serial connections in async mode
(probably only DJGPP doesn't).  Even then, in sync mode, remote.c's
SIGINT handler is only installed while the target is resumed, and is
removed as soon as the target sends back a stop reply.  That means
that a Ctrl-c just while GDB is processing an internal event can end
up with an odd "Quit" at the prompt instead of "Program stopped by
SIGINT".  In contrast, in async mode, remote.c's SIGINT handler is set
up as long as target_terminal_inferior or
target_terminal_ours_for_output are in effect (IOW, until we get a
user-visible stop and call target_terminal_ours), so the user
shouldn't get back a spurious Quit.  However, it's still desirable to
be able to interrupt a long-running GDB operation, if GDB takes a
while to re-resume the target or get back to the event loop.

Tested on x86_64 Fedora 20.

gdb/ChangeLog:
2015-08-21  Pedro Alves  <palves@redhat.com>

	* defs.h (maybe_quit): Declare.
	(QUIT): Now calls maybe_quit.
	* event-loop.c (clear_async_signal_handler)
	(async_signal_handler_is_marked): New functions.
	* event-loop.h (async_signal_handler_is_marked)
	(clear_async_signal_handler): New declarations.
	* remote.c (remote_check_pending_interrupt): New function.
	(interrupt_query): Use make_cleanup_restore_target_terminal.  No
	longer check whether the target is async.  If waiting for a stop
	reply, and a ctrl-c as been sent to the target, offer to
	disconnect, and throw TARGET_CLOSE_ERROR instead of a quit.
	Otherwise do not disconnect and throw a quit.
	(_initialize_remote): Install remote_check_pending_interrupt as
	to_check_pending_interrupt.
	* target.c (target_check_pending_interrupt): New function.
	* target.h (struct target_ops) <to_check_pending_interrupt>: New
	field.
	(target_check_pending_interrupt): New declaration.
	* utils.c (maybe_quit): New function.
	* target-delegates.c: Regenerate.
---
 gdb/defs.h             | 20 +++++++++-----------
 gdb/event-loop.c       | 16 ++++++++++++++++
 gdb/event-loop.h       |  9 +++++++++
 gdb/remote.c           | 40 ++++++++++++++++++++++++++++++----------
 gdb/target-delegates.c | 26 ++++++++++++++++++++++++++
 gdb/target.c           |  8 ++++++++
 gdb/target.h           | 10 ++++++++++
 gdb/utils.c            | 12 ++++++++++++
 8 files changed, 120 insertions(+), 21 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index f4951ab..464a3f8 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -149,17 +149,15 @@ extern int immediate_quit;
 
 extern void quit (void);
 
-/* FIXME: cagney/2000-03-13: It has been suggested that the peformance
-   benefits of having a ``QUIT'' macro rather than a function are
-   marginal.  If the overhead of a QUIT function call is proving
-   significant then its calling frequency should probably be reduced
-   [kingdon].  A profile analyzing the current situtation is
-   needed.  */
-
-#define QUIT { \
-  if (check_quit_flag () || sync_quit_force_run) quit (); \
-  if (deprecated_interactive_hook) deprecated_interactive_hook (); \
-}
+/* Helper for the QUIT macro.  */
+
+extern void maybe_quit (void);
+
+/* Check whether a Ctrl-C was typed, and if so, call quit.  The target
+   is given a chance to process the ctrl-c.  E.g., it may detect that
+   repeated Ctrl-c requests were issued, and choose to close the
+   connection.  */
+#define QUIT maybe_quit ()
 
 /* * Languages represented in the symbol table and elsewhere.
    This should probably be in language.h, but since enum's can't
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 9ac4908..9e54c94 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -908,6 +908,22 @@ mark_async_signal_handler (async_signal_handler * async_handler_ptr)
   async_handler_ptr->ready = 1;
 }
 
+/* See event-loop.h.  */
+
+void
+clear_async_signal_handler (async_signal_handler *async_handler_ptr)
+{
+  async_handler_ptr->ready = 0;
+}
+
+/* See event-loop.h.  */
+
+int
+async_signal_handler_is_marked (async_signal_handler *async_handler_ptr)
+{
+  return async_handler_ptr->ready;
+}
+
 /* Call all the handlers that are ready.  Returns true if any was
    indeed ready.  */
 static int
diff --git a/gdb/event-loop.h b/gdb/event-loop.h
index 6ae4013..598d0da 100644
--- a/gdb/event-loop.h
+++ b/gdb/event-loop.h
@@ -102,6 +102,15 @@ void call_async_signal_handler (struct async_signal_handler *handler);
    below, with IMMEDIATE_P == 0.  */
 void mark_async_signal_handler (struct async_signal_handler *handler);
 
+/* Returns true if HANDLER is marked ready.  */
+
+extern int
+  async_signal_handler_is_marked (struct async_signal_handler *handler);
+
+/* Mark HANDLER as NOT ready.  */
+
+extern void clear_async_signal_handler (struct async_signal_handler *handler);
+
 /* Wrapper for the body of signal handlers.  Call this function from
    any SIGINT handler which needs to access GDB data structures or
    escape via longjmp.  If IMMEDIATE_P is set, this triggers either
diff --git a/gdb/remote.c b/gdb/remote.c
index 068d079..2b9debc 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5280,6 +5280,20 @@ async_handle_remote_sigint_twice (int sig)
   gdb_call_async_signal_handler (async_sigint_remote_twice_token, 0);
 }
 
+/* Implementation of to_check_pending_interrupt.  */
+
+static void
+remote_check_pending_interrupt (struct target_ops *self)
+{
+  struct async_signal_handler *token = async_sigint_remote_twice_token;
+
+  if (async_signal_handler_is_marked (token))
+    {
+      clear_async_signal_handler (token);
+      call_async_signal_handler (token);
+    }
+}
+
 /* Perform the real interruption of the target execution, in response
    to a ^C.  */
 static void
@@ -5452,24 +5466,29 @@ remote_interrupt (struct target_ops *self, ptid_t ptid)
 static void
 interrupt_query (void)
 {
+  struct remote_state *rs = get_remote_state ();
+  struct cleanup *old_chain;
+
+  old_chain = make_cleanup_restore_target_terminal ();
   target_terminal_ours ();
 
-  if (target_is_async_p ())
-    {
-      signal (SIGINT, handle_sigint);
-      quit ();
-    }
-  else
+  if (rs->waiting_for_stop_reply && rs->ctrlc_pending_p)
     {
-      if (query (_("Interrupted while waiting for the program.\n\
-Give up (and stop debugging it)? ")))
+      if (query (_("The target is not responding to interrupt requests.\n"
+		   "Stop debugging it? ")))
 	{
 	  remote_unpush_target ();
-	  quit ();
+	  throw_error (TARGET_CLOSE_ERROR, _("Disconnected from target."));
 	}
     }
+  else
+    {
+      if (query (_("Interrupted while waiting for the program.\n"
+		   "Give up waiting? ")))
+	quit ();
+    }
 
-  target_terminal_inferior ();
+  do_cleanups (old_chain);
 }
 
 /* Enable/disable target terminal ownership.  Most targets can use
@@ -12511,6 +12530,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_get_ada_task_ptid = remote_get_ada_task_ptid;
   remote_ops.to_stop = remote_stop;
   remote_ops.to_interrupt = remote_interrupt;
+  remote_ops.to_check_pending_interrupt = remote_check_pending_interrupt;
   remote_ops.to_xfer_partial = remote_xfer_partial;
   remote_ops.to_rcmd = remote_rcmd;
   remote_ops.to_pid_to_exec_file = remote_pid_to_exec_file;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 892cf9d..ddcad94 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1561,6 +1561,28 @@ debug_interrupt (struct target_ops *self, ptid_t arg1)
 }
 
 static void
+delegate_check_pending_interrupt (struct target_ops *self)
+{
+  self = self->beneath;
+  self->to_check_pending_interrupt (self);
+}
+
+static void
+tdefault_check_pending_interrupt (struct target_ops *self)
+{
+}
+
+static void
+debug_check_pending_interrupt (struct target_ops *self)
+{
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_check_pending_interrupt (...)\n", debug_target.to_shortname);
+  debug_target.to_check_pending_interrupt (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_check_pending_interrupt (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (")\n", gdb_stdlog);
+}
+
+static void
 delegate_rcmd (struct target_ops *self, const char *arg1, struct ui_file *arg2)
 {
   self = self->beneath;
@@ -4042,6 +4064,8 @@ install_delegators (struct target_ops *ops)
     ops->to_stop = delegate_stop;
   if (ops->to_interrupt == NULL)
     ops->to_interrupt = delegate_interrupt;
+  if (ops->to_check_pending_interrupt == NULL)
+    ops->to_check_pending_interrupt = delegate_check_pending_interrupt;
   if (ops->to_rcmd == NULL)
     ops->to_rcmd = delegate_rcmd;
   if (ops->to_pid_to_exec_file == NULL)
@@ -4280,6 +4304,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_thread_name = tdefault_thread_name;
   ops->to_stop = tdefault_stop;
   ops->to_interrupt = tdefault_interrupt;
+  ops->to_check_pending_interrupt = tdefault_check_pending_interrupt;
   ops->to_rcmd = default_rcmd;
   ops->to_pid_to_exec_file = tdefault_pid_to_exec_file;
   ops->to_log_command = tdefault_log_command;
@@ -4430,6 +4455,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_thread_name = debug_thread_name;
   ops->to_stop = debug_stop;
   ops->to_interrupt = debug_interrupt;
+  ops->to_check_pending_interrupt = debug_check_pending_interrupt;
   ops->to_rcmd = debug_rcmd;
   ops->to_pid_to_exec_file = debug_pid_to_exec_file;
   ops->to_log_command = debug_log_command;
diff --git a/gdb/target.c b/gdb/target.c
index e41a338..0861d24 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3285,6 +3285,14 @@ target_interrupt (ptid_t ptid)
   (*current_target.to_interrupt) (&current_target, ptid);
 }
 
+/* See target.h.  */
+
+void
+target_check_pending_interrupt (void)
+{
+  (*current_target.to_check_pending_interrupt) (&current_target);
+}
+
 /* See target/target.h.  */
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index e283c86..27612a0 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -642,6 +642,8 @@ struct target_ops
       TARGET_DEFAULT_IGNORE ();
     void (*to_interrupt) (struct target_ops *, ptid_t)
       TARGET_DEFAULT_IGNORE ();
+    void (*to_check_pending_interrupt) (struct target_ops *)
+      TARGET_DEFAULT_IGNORE ();
     void (*to_rcmd) (struct target_ops *,
 		     const char *command, struct ui_file *output)
       TARGET_DEFAULT_FUNC (default_rcmd);
@@ -1681,6 +1683,14 @@ extern void target_stop (ptid_t ptid);
 
 extern void target_interrupt (ptid_t ptid);
 
+/* Some targets install their own SIGINT handler while the target is
+   running.  This method is called from the QUIT macro to give such
+   targets a chance to process a Ctrl-C.  The target may e.g., choose
+   to interrupt the (potentially) long running operation, or give up
+   waiting and disconnect.  */
+
+extern void target_check_pending_interrupt (void);
+
 /* Send the specified COMMAND to the target's monitor
    (shell,interpreter) for execution.  The result of the query is
    placed in OUTBUF.  */
diff --git a/gdb/utils.c b/gdb/utils.c
index e5ad195..2b3ec72 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1041,6 +1041,18 @@ quit (void)
 #endif
 }
 
+/* See defs.h.  */
+
+void
+maybe_quit (void)
+{
+  if (check_quit_flag () || sync_quit_force_run)
+    quit ();
+  if (deprecated_interactive_hook)
+    deprecated_interactive_hook ();
+  target_check_pending_interrupt ();
+}
+
 \f
 /* Called when a memory allocation fails, with the number of bytes of
    memory requested in SIZE.  */
-- 
1.9.3

Re: [PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")

[Sandra Loosemore]

On 08/21/2015 08:52 AM, Pedro Alves wrote:

> But please try the new patch below.

In combination with the two patches Gary just checked in, this is 
working fabulously.  I see:

(gdb) c
Continuing.
Reading 
/scratch/sandra/nios2-linux-trunk/install/opt/codesourcery/nios2-linux-gnu/libc/lib/libc.so.6 
from remote target...
^C^CInterrupted while waiting for the program.
Give up waiting? (y or n) y
Quit
(gdb) bt
#0  __GI__dl_debug_state () at dl-debug.c:74
#1  0x2aaab1d0 in dl_main (phdr=<optimized out>, phnum=<optimized out>,
     user_entry=<optimized out>, auxv=<optimized out>) at rtld.c:2172
#2  0x2aabd284 in _dl_sysdep_start (start_argptr=<optimized out>,
     dl_main=0x2aaa93a4 <dl_main>) at ../elf/dl-sysdep.c:249
#3  0x2aaac34c in _dl_start_final (arg=0x7ffefa80, info=<optimized out>)
     at rtld.c:308
#4  0x2aaac644 in _dl_start (arg=0x7ffefa80) at rtld.c:418
#5  0x2aaa8ce8 in _start ()
    from 
target:/scratch/sandra/nios2-linux-trunk/install/opt/codesourcery/nios2-linux-gnu/libc/lib/ld-linux-nios2.so.1
(gdb) c
Continuing.

Breakpoint 1, main () at /home/sandra/examples/croak.c:10
10	  n = sizeof (s) / sizeof (const char *);
(gdb) info sharedlibrary
 From        To          Syms Read   Shared Object Library
0x2aaa89e4  0x2aac1780  Yes 
target:/scratch/sandra/nios2-linux-trunk/install/opt/codesourcery/nios2-linux-gnu/libc/lib/ld-linux-nios2.so.1
0x2aaece80  0x2abf64c8  No 
target:/scratch/sandra/nios2-linux-trunk/install/opt/codesourcery/nios2-linux-gnu/libc/lib/libc.so.6
(gdb)

I think this addresses all my concerns with the change in the default 
behavior.  Pedro and Gary, thanks very much for your patience and hard 
work in getting this resolved!  Between the speedup in reading the 
libraries, the messages to explain what is going on, and making 
transfers interruptible, this is a big improvement in usability. :-D

-Sandra

Re: [PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")

[Pedro Alves]

On 08/21/2015 06:10 PM, Sandra Loosemore wrote:
> On 08/21/2015 08:52 AM, Pedro Alves wrote:
> 
>> But please try the new patch below.
> 
> In combination with the two patches Gary just checked in, this is 
> working fabulously.  I see:

Ribbit!  :-)

> I think this addresses all my concerns with the change in the default 
> behavior.  Pedro and Gary, thanks very much for your patience and hard 
> work in getting this resolved!  Between the speedup in reading the 
> libraries, the messages to explain what is going on, and making 
> transfers interruptible, this is a big improvement in usability. :-D

Thank you for your patience pushing us all in the right direction!

Does anyone want to comment on the patch, or shall I go ahead with it?

Thanks,
Pedro Alves

Re: [PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")

[Gary Benson]

Sandra Loosemore wrote:
> I think this addresses all my concerns with the change in the
> default behavior.  Pedro and Gary, thanks very much for your
> patience and hard work in getting this resolved!  Between the
> speedup in reading the libraries, the messages to explain what is
> going on, and making transfers interruptible, this is a big
> improvement in usability. :-D

Nice :)  Sorry for being a grouch at times!

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")

[Pedro Alves]

On 08/21/2015 06:27 PM, Pedro Alves wrote:

> Does anyone want to comment on the patch, or shall I go ahead with it?

I'm going to push this in in a couple hours, in order to see if the
buildbots complain, rather than wait too long and rush this in
just before Joel decides to cut the release.

Thanks,
Pedro Alves

Re: [PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")

[Pedro Alves]

On 08/25/2015 11:57 AM, Pedro Alves wrote:
> On 08/21/2015 06:27 PM, Pedro Alves wrote:
> 
>> Does anyone want to comment on the patch, or shall I go ahead with it?
> 
> I'm going to push this in in a couple hours, in order to see if the
> buildbots complain, rather than wait too long and rush this in
> just before Joel decides to cut the release.

Now pushed.

Thanks,
Pedro Alves

GDB 7.10 release tentative date: Fri Aug 28 (was: "Re: [PATCH] remote: allow aborting long operations (e.g., file transfers) (Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:")")

[Joel Brobecker]

> >> Does anyone want to comment on the patch, or shall I go ahead with it?
> > 
> > I'm going to push this in in a couple hours, in order to see if the
> > buildbots complain, rather than wait too long and rush this in
> > just before Joel decides to cut the release.
> 
> Now pushed.

Thanks, guys. Let's indeed give ourselves a couple of days, just in
case, and plan for a release on Friday.

-- 
Joel