Re: [patch] thread info - automatic skip frames

[Michael Snyder <msnyder@redhat.com>]

Jim Houston wrote:
> Hi Everyone,
> 
> I developed this patch to support using kgdb on the 
> x86-64 linux kernel.
> 
> The patch adds an option to gdb to skip over boring frames
> in the "thread info" results.  This is useful when most of
> the threads are stopped in the context switch code.  It lets
> you see the frame which shows why the the thread blocked.
> 
> Use:
> 	set skip-frame thread_return,schedule_timeout

Wow -- interesting idea!  I'd like to see some discussion about
the user interface.

I think it's clever to let the user decide what functions
are "un-interesting" -- and if I were the user, I'd probably
think of more un-interesting functions as I went along.
I'd like to be able to add to the list, rather than enter
the whole thing again from scratch.  And that implies a
separate command to clear it.

I also wonder whether the same idea might be useful for
backtrace and "frame".  When I debug, I often have to
go up the stack a ways before finding something interesting.
That can be added separately, but it would be nice to get
the common functionality right from the beginning.

Have you thought about other ways to distinguish "uninteresting"
functions automatically, without imposing on the user to list
them?  Perhaps (under some circumstances, anyway), any function
from a shared library might be uninteresting?  Or any function
lacking debug information?



> 
> --- old/gdb-6.0/gdb/thread.c	2004-01-06 12:34:14.786496352 -0500
> +++ new/gdb-6.0/gdb/thread.c	2004-01-06 12:34:28.804365312 -0500
> @@ -404,6 +404,43 @@
>      }
>  }
>  
> +/*
> + * When using gdb as a kernel debugger, its really boring to
> + * see every thread is blocked in schedule.  By setting a
> + * list of functions with "set skip-frame schedule,thread_return" 
> + * we can display the frame that called into the scheduler.
> + */
> +static char *skip_frame_string;
> +
> +int
> +skip_frame(struct frame_info *fi)
> +{
> +  struct minimal_symbol *msym;
> +  CORE_ADDR pc;
> +  char *name;
> +  char *s, *r;
> +  int n;
> +
> +  pc = get_frame_pc (fi);
> +  msym = lookup_minimal_symbol_by_pc_section(pc, NULL);
> +  if (!msym)
> +    return 0;
> +  name = SYMBOL_LINKAGE_NAME(msym);
> +
> +  for (s = skip_frame_string; s && *s ; ) {
> +    if ((r = strchr(s, ',')))
> +      n = r - s - 1;
> +    else
> +      n = strlen(s);
> +    if (n && strncmp(s, name, n) == 0)
> +      return 1;
> +    if (!r)
> +      break;
> +    s = r + 1;
> +  }
> +  return 0;
> +}
> +
>  /* Print information about currently known threads 
>  
>   * Note: this has the drawback that it _really_ switches
> @@ -416,7 +453,7 @@
>  {
>    struct thread_info *tp;
>    ptid_t current_ptid;
> -  struct frame_info *cur_frame;
> +  struct frame_info *cur_frame, *fi;
>    int saved_frame_level = frame_relative_level (get_selected_frame ());
>    int counter;
>    char *extra_info;
> @@ -448,6 +485,18 @@
>        puts_filtered ("  ");
>  
>        switch_to_thread (tp->ptid);
> +
> +      if (skip_frame_string) {
> +	/* skip up the stack to an interesting frame. */
> +        fi = get_selected_frame ();
> +        while (fi) {
> +          if (!skip_frame(fi))
> +            break;
> +          fi = get_prev_frame(fi);
> +  	  if (fi)
> +            select_frame(fi);
> +        }
> +      }
>        print_stack_frame (get_selected_frame (), -1, 0);
>      }
>  
> @@ -740,4 +789,12 @@
>  
>    if (!xdb_commands)
>      add_com_alias ("t", "thread", class_run, 1);
> +
> +  add_show_from_set (add_set_cmd ("skip-frame", class_obscure,
> +    var_string_noescape, (char *)&skip_frame_string, "\
> +Set list of functions to skip when choosing the frame to display\n\
> +for a info-thread command.  When gdb is used for kernel debug this option\n\
> +allows the frame which calls the scheduler to be displayed rather than\n\
> +having all blocked threads showing the same function in the scheduler.",
> +    &setlist), &showlist);
>  }
>