public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/6] [Linux] Read vDSO range from /proc/PID/task/PID/maps instead of /proc/PID/maps
Date: Thu, 19 May 2016 14:48:00 -0000	[thread overview]
Message-ID: <1463669290-30415-3-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1463669290-30415-1-git-send-email-palves@redhat.com>

... as it's _much_ faster.

Hacking the gdb.threads/attach-many-short-lived-threads.exp test to
spawn thousands of threads instead of dozens to stress and debug
timeout problems with gdb.threads/attach-many-short-lived-threads.exp,
I saw that GDB would spend several seconds just reading the
/proc/PID/smaps file, to determine the vDSO mapping range.  GDB opens
and reads the whole file just once, and caches the result, but even
that is too slow.  For example, with almost 8000 threads:

 $ ls /proc/3518/task/ | wc -l
 7906

reading the /proc/PID/smaps file grepping for "vdso" takes over 15
seconds :

 $ time cat /proc/3518/smaps | grep vdso
 7ffdbafee000-7ffdbaff0000 r-xp 00000000 00:00 0                          [vdso]

 real    0m15.371s
 user    0m0.008s
 sys     0m15.017s

Looking around the web for hints, I found a nice description of the
issue here:

 http://backtrace.io/blog/blog/2014/11/12/large-thread-counts-and-slow-process-maps/

The problem is that /proc/PID/smaps wants to show the mappings as
being thread stack, and that has the kernel iterating over all threads
in the thread group, for each mapping.

The fix is to use the "map" file under /proc/PID/task/PID/ instead of
the /proc/PID/ one, as the former doesn't mark thread stacks for all
threads.

That alone drops the timing to the millisecond range on my machine:

 $ time cat /proc/3518/task/3518/smaps | grep vdso
 7ffdbafee000-7ffdbaff0000 r-xp 00000000 00:00 0                          [vdso]

 real    0m0.150s
 user    0m0.009s
 sys     0m0.084s

And since we only need the vdso mapping's address range, we can use
"maps" file instead of "smaps", and it's even cheaper:

/proc/PID/task/PID/maps :

 $ time cat /proc/3518/task/3518/maps | grep vdso
 7ffdbafee000-7ffdbaff0000 r-xp 00000000 00:00 0                          [vdso]

 real    0m0.027s
 user    0m0.000s
 sys     0m0.017s

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	PR gdb/19828
	* linux-tdep.c (find_mapping_size): Delete.
	(linux_vsyscall_range_raw): Rewrite reading from
	/proc/PID/task/PID/maps directly instead of using
	gdbarch_find_memory_regions.
---
 gdb/linux-tdep.c | 77 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 54 insertions(+), 23 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index b8d063f..ab110b0 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2277,39 +2277,70 @@ linux_gdb_signal_to_target (struct gdbarch *gdbarch,
   return -1;
 }
 
-/* Rummage through mappings to find a mapping's size.  */
-
-static int
-find_mapping_size (CORE_ADDR vaddr, unsigned long size,
-		   int read, int write, int exec, int modified,
-		   void *data)
-{
-  struct mem_range *range = (struct mem_range *) data;
-
-  if (vaddr == range->start)
-    {
-      range->length = size;
-      return 1;
-    }
-  return 0;
-}
-
 /* Helper for linux_vsyscall_range that does the real work of finding
    the vsyscall's address range.  */
 
 static int
 linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
 {
+  char filename[100];
+  long pid;
+  char *data;
+
+  /* Can't access /proc if debugging a core file.  */
+  if (!target_has_execution)
+    return 0;
+
+  /* We need to know the real target PID to access /proc.  */
+  if (current_inferior ()->fake_pid_p)
+    return 0;
+
   if (target_auxv_search (&current_target, AT_SYSINFO_EHDR, &range->start) <= 0)
     return 0;
 
-  /* This is installed by linux_init_abi below, so should always be
-     available.  */
-  gdb_assert (gdbarch_find_memory_regions_p (target_gdbarch ()));
+  pid = current_inferior ()->pid;
 
-  range->length = 0;
-  gdbarch_find_memory_regions (gdbarch, find_mapping_size, range);
-  return 1;
+  /* Note that reading /proc/PID/task/PID/maps (1) is much faster than
+     reading /proc/PID/maps (2).  The later identifies thread stacks
+     in the output, which requires scanning every thread in the thread
+     group to check whether a VMA is actually a thread's stack.  With
+     Linux 4.4 on an Intel i7-4810MQ @ 2.80GHz, with an inferior with
+     a few thousand threads, (1) takes a few miliseconds, while (2)
+     takes several seconds.  Also note that "smaps", what we read for
+     determining core dump mappings, is even slower than "maps".  */
+  xsnprintf (filename, sizeof filename, "/proc/%ld/task/%ld/maps", pid, pid);
+  data = target_fileio_read_stralloc (NULL, filename);
+  if (data != NULL)
+    {
+      struct cleanup *cleanup = make_cleanup (xfree, data);
+      char *line;
+      char *saveptr = NULL;
+
+      for (line = strtok_r (data, "\n", &saveptr);
+	   line != NULL;
+	   line = strtok_r (NULL, "\n", &saveptr))
+	{
+	  ULONGEST addr, endaddr;
+	  const char *p = line;
+
+	  addr = strtoulst (p, &p, 16);
+	  if (addr == range->start)
+	    {
+	      if (*p == '-')
+		p++;
+	      endaddr = strtoulst (p, &p, 16);
+	      range->length = endaddr - addr;
+	      do_cleanups (cleanup);
+	      return 1;
+	    }
+	}
+
+      do_cleanups (cleanup);
+    }
+  else
+    warning (_("unable to open /proc file '%s'"), filename);
+
+  return 0;
 }
 
 /* Implementation of the "vsyscall_range" gdbarch hook.  Handles
-- 
2.5.5

  reply	other threads:[~2016-05-19 14:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-19 14:48 [PATCH 0/6] Fix PR gdb/19828 (attach -> internal error) and attach optimizations Pedro Alves
2016-05-19 14:48 ` Pedro Alves [this message]
2016-05-23 11:08   ` [PATCH 2/6] [Linux] Read vDSO range from /proc/PID/task/PID/maps instead of /proc/PID/maps Yao Qi
2016-05-19 14:48 ` [PATCH 3/6] [Linux] Avoid refetching core-of-thread if thread hasn't run Pedro Alves
2016-05-23 12:45   ` Yao Qi
2016-05-19 14:48 ` [PATCH 1/6] Linux native thread create/exit events support Pedro Alves
2016-05-23 10:49   ` Yao Qi
2016-05-23 13:00     ` Pedro Alves
2016-05-19 14:56 ` [PATCH 5/6] Make gdb/linux-nat.c consider a waitstatus pending on the infrun side Pedro Alves
2016-05-19 14:57 ` [PATCH 4/6] [Linux] Optimize PID -> struct lwp_info lookup Pedro Alves
2016-05-23 13:12   ` Yao Qi
2016-05-23 18:10     ` [PATCH v2/htab " Pedro Alves
2016-05-24  9:33       ` Yao Qi
2016-05-24 13:47         ` Pedro Alves
2016-05-19 14:57 ` [PATCH 6/6] Fix PR gdb/19828: gdb -p <process from a container>: internal error Pedro Alves
2016-05-23 10:14   ` Yao Qi
2016-05-25 17:43   ` [pushed/7.11.1] " Pedro Alves
2016-05-19 15:11 ` [PATCH 0/6] Fix PR gdb/19828 (attach -> internal error) and attach optimizations Pedro Alves
2016-05-24 13:57 ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1463669290-30415-3-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).