[Crash-utility] [PATCHv2 00/11] Implement percpu handling for crash

Dave Anderson anderson at redhat.com
Wed Oct 23 19:37:42 UTC 2013



----- Original Message -----
> Hi Dave,
> 
> I'm sorry for the last submission. It seems I forgot to refresh the
> patches, so it was completely bogus. Should be fixed now. I'm also
> attaching my changes as one big patch to this message.
> 
> Petr Tesarik

Hi Petr,

I've reviewed the code changes, and have been beating on the
patch, and I can't get it break.  Really nice work...

The only things I'd like to change are trivial/cosmetic.
For clarity's sake I'd like to change the "p" help page
to separate expressions from symbols as arguments, and
then enforce the fact that the :cpuspec only applies to 
symbol arguments.  So instead of this:

  crash> help p
  
  NAME
    p - print the value of an expression
  
  SYNOPSIS
    p [-x|-d][-u] expression[:cpuspec]
  
  DESCRIPTION
    This command passes its arguments on to gdb "print" command for evaluation.
  
      expression   The expression to be evaluated.
         cpuspec  CPU specification for per-cpu variables:
                   :             CPU of the currently selected task.
                   :a[ll]        all CPUs.
                   :#[-#][,...]  CPU list(s), e.g. "1,3,5", "1-3",
                                 or "1,3,5-7,10".
              -x  override default output format with hexadecimal format.
              -d  override default output format with decimal format.
              -u  the expression evaluates to a user address reference.
  ...

I'd change it to this:
  
  crash> help p
  
  NAME
    p - print the value of an expression
  
  SYNOPSIS
    p [-x|-d][-u] [expression | symbol[:cpuspec]]
  
  DESCRIPTION
    This command passes its arguments on to gdb "print" command for evaluation.
  
      expression  an expression to be evaluated.
          symbol  a kernel symbol.
        :cpuspec  when appended to a symbol with a colon, a CPU specification
                  for per-cpu variables:
                    :             CPU of the currently selected task.
                    :a[ll]        all CPUs.
                    :#[-#][,...]  CPU list(s), e.g. "1,3,5", "1-3",
                                  or "1,3,5-7,10".
              -x  override default output format with hexadecimal format.
              -d  override default output format with decimal format.
              -u  the expression evaluates to a user address reference.
  ...

And maybe a minor indenting change for the cpu/address values, 
from this:

  crash> struct desc_ptr b0c8:1,3
    [1]: ffff88021e24b0c8
  struct desc_ptr {
    size = 0,
    address = 0
  }
    [3]: ffff88021e2cb0c8
  struct desc_ptr {
    size = 0,
    address = 0
  }
  crash>

to this:
  
  crash> struct desc_ptr b0c8:1,3
  [1]: ffff88021e24b0c8
  struct desc_ptr {
    size = 0, 
    address = 0
  }
  [3]: ffff88021e2cb0c8
  struct desc_ptr {
    size = 0, 
    address = 0
  }
  crash>

But other than that, it looks good to go.  Do you have anything
more to add?
  
Thanks,
  Dave




More information about the Crash-utility mailing list