This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Linux kernel thread runtime support


Hi Peter
Hi everybody else

well it had to come this way. I am also working on an kernel debugging
target quite similar to yours and planed to send my code next week or
the one after...

On the bright side, till now i worked on core dumps on S390 and also
have some features (especially module support with kernel virtual
memory handling) you are still missing. So not all the work is in vain
we just need to find a common basis. Please give me some days so i can
finalize my code and send it to the list. Then we can discuss it in
detail.

Nevertheless i found some issues in your basic thread handling logic
(from kernel point of view) you should have a look at. See comments
below.

Philipp


From linux-kthread.c:

[...]

> /* asm/generic/percpu.h
>  * per_cpu_offset() is the offset that has to be added to a
>  * percpu variable to get to the instance for a certain processor.
>  * Most arches use the __per_cpu_offset array for those offsets but
>  * some arches have their own ways of determining the offset (x86_64,
> s390). */

Yes, S390 is special in this case. You need to add an arch specific
hook. I made the hook optional, i.e. if not set the default method via
__per_cpu_offset array is used.

> DECLARE_ADDR (__per_cpu_offset);
> DECLARE_ADDR (per_cpu__process_counts);
> DECLARE_ADDR (process_counts);
> DECLARE_ADDR (per_cpu__runqueues);
> DECLARE_ADDR (runqueues);
> 
> #define CORE_INVAL (-1)

CORE_INVAL is defined a second time in linux-kthread.h.

[...]

> void
> lkthread_get_per_cpu_offsets(int numcores)
> {
>   enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
>   int length = TYPE_LENGTH (builtin_type (target_gdbarch
> ())->builtin_data_ptr); CORE_ADDR curr_addr = ADDR (__per_cpu_offset);
>   int core;
> 
> 
>   if (!HAS_ADDR (__per_cpu_offset))
>     {
>       if (debug_linuxkthread_threads)
> 	fprintf_unfiltered (gdb_stdlog, "Assuming non-SMP kernel.\n");
> 
>       return;
>     }
> 
>   for (core=0; core < numcores; core++)

This iteration does not work. As CPUs can be added/removed the logical
CPU-ids needn't be continuous, e.g. with two CPUs the ids can be 0 and
2. You have to walk the cpu_online_mask to see which CPUs are used at
the moment. The iteration appears several times in the code.

>   {
>       if (!lkthread_h->per_cpu_offset[core])
> 	lkthread_h->per_cpu_offset[core] =
> 	  read_memory_unsigned_integer (curr_addr, length,
> byte_order);
> 
>       curr_addr += (CORE_ADDR) length;
> 
>       if (!lkthread_h->per_cpu_offset[core])
> 	{
> 	  warning ("Suspicious null per-cpu offsets,"
> 		   " or wrong number of detected cores:\n"
> 		   "ADDR (__per_cpu_offset) = %s\nmax_cores = %d",
> 		   phex (ADDR (__per_cpu_offset),4), max_cores);
> 
> 	  break;
> 	}
>     }
> 
>   if (debug_linuxkthread_threads)
>     fprintf_unfiltered (gdb_stdlog, "SMP kernel. %d cores detected\n",
> 			/numcores);
> }

[...]

> static void
> lkthread_init (void)
> {
>   struct thread_info *th = NULL;
>   struct cleanup *cleanup;
>   int size =
>     TYPE_LENGTH (builtin_type (target_gdbarch
> ())->builtin_unsigned_long);
> 
>   /* Ensure thread list from beneath target is up to date.  */
>   cleanup = make_cleanup_restore_integer (&print_thread_events);
>   print_thread_events = 0;
>   update_thread_list ();
>   do_cleanups (cleanup);
> 
>   /* Count the h/w threads.  */
>   max_cores = thread_count ();

The number of CPUs does not have to be constant. Especially when you
are running in a VM you can add/remove CPUs. Thus max_cores has to be
determined dynamically or set to the max number of CPUs, e.g. by taking
the size (in bits) of cpu_online_mask. 

>   gdb_assert (max_cores);
> 
>   if (debug_linuxkthread_threads)
>     {
>       fprintf_unfiltered (gdb_stdlog, "lkthread_init() cores(%d) GDB"
> 			  "HW threads\n", max_cores);
>       iterate_over_threads (thread_print_info, NULL);
>     }
> 
>   /* Allocate per cpu data.  */
>   lkthread_alloc_percpu_data(max_cores);

Together with the comment above, allocating the percpu_data to a fixed
size can cause problems when the number of CPUs is increased.

>   lkthread_get_per_cpu_offsets(max_cores);
> 
>   if (!lkthread_get_runqueues_addr () && (max_cores > 1))
>     fprintf_unfiltered (gdb_stdlog, "Could not find the address of
> CPU" " runqueues current context information maybe "
> 			"less precise\n.");
> 
>   /* Invalidate the linux-kthread cached list.  */
>   lkthread_invalidate_threadlist ();
> }

[...]

> static linux_kthread_info_t **
> lkthread_get_threadlist_helper (linux_kthread_info_t ** ps)
> {
>   struct linux_kthread_arch_ops *arch_ops =
>     gdbarch_linux_kthread_ops (target_gdbarch ());
>   CORE_ADDR rq_idle_taskstruct;
>   CORE_ADDR g, t, init_task_addr;
>   int core = 0, i;
> 
>   if (debug_linuxkthread_threads)
>     fprintf_unfiltered (gdb_stdlog,
> "lkthread_get_threadlist_helper\n");
> 
>   init_task_addr = ADDR (init_task);
>   g = init_task_addr;
> 
>   do
>     {
>       t = g;
>       do
>         {
> 
> 	  if (!arch_ops->is_kernel_address(t))
> 	    {
> 	      warning ("parsing of task list stopped because of
> invalid address" "%s", phex (t, 4));
>               break;
> 	    }
> 
>           lkthread_get_task_info (t, ps, core /*zero-based */ );
>           core = CORE_INVAL;
> 
>           if (ptid_get_tid (PTID_OF (*ps)) == 0)
>             {
>               /* This is init_task, let's insert the other cores
> swapper now.  */
>               for (i = 1; i < max_cores; i++)
>                 {
>                   ps = &((*ps)->next);
>                   rq_idle_taskstruct = lkthread_get_rq_idle (i);
>                   lkthread_get_task_info (rq_idle_taskstruct, ps, i);
>                 }
>             }

How does the setting to a core work? For me it looks like only for
init_task and the swapper tasks core != CORE_INVAL. Is that the
intended behavior?

> 	    if (debug_linuxkthread_threads)
> 	      fprintf_unfiltered (gdb_stdlog, "Got task info for %s
> (%li)\n", (*ps)->comm, ptid_get_lwp (PTID_OF (*ps)));
> 
>           ps = &((*ps)->next);
> 
> 	  /* Mark end of chain and remove those threads that
> disappeared from the thread_list to avoid any_thread_of_process() to
> 	     select a ghost.  */
>           if (*ps)
>             (*ps)->valid = 0;
> 
>           t = _next_thread (t);
>         } while (t && (t != g));
> 
>       g = _next_task (g);
>     } while (g && (g != init_task_addr));

Although pretty unlikely, the do {} while can end in an endless loop
when you have a memory corruption compromising one of the list_heads.
Thus there should be more sanity checks than just the one for NULL. I
use list_head->next->prev == list_head.

>   return ps;
> }


On Thu, 22 Dec 2016 17:31:03 +0000
Peter Griffin <peter.griffin@linaro.org> wrote:

> Hi GDB maintainers,
> 
> The following patch implements a Linux kernel thread runtime stratum
> which can be used when using GDB to debug a Linux kernel. For example
> when connecting to a QEMU GDB stub, or OpenOCD which then communicates
> with the target via JTAG or SWD.
> 
> This patch is a refactored version based on the 'Linux kernel
> debugger' GDB plugin written at STMicroelectronics which used to
> be packaged with their JTAG debuggers. There has been some discussion
> previously on the list by myself [1], Kieran [2] and one of the
> original authors at ST Marc Titinger [3].
> 
> This patchset I'm hoping is a lot closer to something which can
> be upstreamed to GDB project after some discussion about structure
> with Yao Qi at Linaro Connect, the code has been refactored to be
> structured much more like the existing upstream thread runtimes (such
> as ravenscar-thread.c and sparc-ravenscar-thread etc).
> 
> Since the original email [1] various helper commands have also been
> migrated into python and merged into the Linux kernel source tree.
> The GDB python extensions, combined with the linux-kthread GDB
> thread runtime implemented in this patchset provide a powerful
> Linux kernel debug solution, and is a working implementation
> of what Andreas talked about on slide 17 and 18 of his talk at GNU
> Cauldron [4].
> 
> I have currently been testing this patchset using mainline GDB
> debugging arm-linux kernels in Qemu and via OpenoCD to real hardware.
> It is straight forward with the current strructure to add new
> architecture support, and I'm looking at adding PowerPC so I can
> easily validate big endian targets.
> 
> What I'm really hoping for now is some patch review on the GDB
> mailing list from the GDB maintainers and community with a view to
> getting this functionality which has been talked about for quite a
> few years finally merged to the upstream GDB project.
> 
> All patch review feedback greatfully received :)
> 
> kind regards,
> 
> Peter.
> 
> [1] https://cygwin.com/ml/gdb/2015-09/msg00032.html
> [2] https://www.sourceware.org/ml/gdb/2016-01/msg00028.html
> [3]
> https://lists.linaro.org/pipermail/linaro-toolchain/2011-November/001754.html
> [4]
> https://gcc.gnu.org/wiki/cauldron2015?action=AttachFile&do=view&target=Andreas+Arnez_+Debugging+Linux+kernel+dumps+with+GDB.pdf
> 
> 
> 
> Peter Griffin (1):
>   Add Linux kernel thread runtime support.
> 
>  gdb/ChangeLog           |   12 +
>  gdb/Makefile.in         |    8 +-
>  gdb/arm-linux-kthread.c |  178 +++++
>  gdb/arm-linux-kthread.h |   27 +
>  gdb/arm-tdep.c          |    4 +
>  gdb/configure.tgt       |    6 +-
>  gdb/gdbarch.c           |   23 +
>  gdb/gdbarch.h           |    5 +
>  gdb/gdbarch.sh          |    3 +
>  gdb/linux-kthread.c     | 1828
> +++++++++++++++++++++++++++++++++++++++++++++++
> gdb/linux-kthread.h     |  223 ++++++ 11 files changed, 2311
> insertions(+), 6 deletions(-) create mode 100644
> gdb/arm-linux-kthread.c create mode 100644 gdb/arm-linux-kthread.h
>  create mode 100644 gdb/linux-kthread.c
>  create mode 100644 gdb/linux-kthread.h
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]