Re: [1/1] powerpc/xmon: Paged output for paca display

2015-08-19 Thread Sam Bobroff
On Tue, Aug 18, 2015 at 04:26:32PM +1000, Michael Ellerman wrote:
 On Fri, 2015-14-08 at 02:55:14 UTC, Sam bobroff wrote:
  The paca display is already more than 24 lines, which can be problematic
  if you have an old school 80x24 terminal, or more likely you are on a
  virtual terminal which does not scroll for whatever reason.
  
  This adds an optional letter to the dp and dpa xmon commands
  (dpp and dppa), which will enable a per-page display (with 16
  line pages): the first page  will be displayed and if there was data
  that didn't fit, it will display a message indicating that the user can
  use enter to display the next page. The intent is that this feels
  similar to the way the memory display functions work.
  
  This is implemented by running over the entire output both for the
  initial command and for each subsequent page: the visible part is
  clipped out by checking line numbers. Handling the empty command as
  more is done by writing a special command into a static buffer that
  indicates where to move the sliding visibility window. This is similar
  to the approach used for the memory dump commands except that the
  state data is encoded into the last_cmd string, rather than a set of
  static variables. The memory dump commands could probably be rewritten
  to make use of the same buffer and remove their other static
  variables.
  
  Sample output:
  
  0:mon dpp1
  paca for cpu 0x1 @ cfdc0480:
   possible = yes
   present  = yes
   online   = yes
   lock_token   = 0x8000  (0x8)
   paca_index   = 0x1 (0xa)
   kernel_toc   = 0xc0eb2400  (0x10)
   kernelbase   = 0xc000  (0x18)
   kernel_msr   = 0xb0001032  (0x20)
   emergency_sp = 0xc0003ffe8000  (0x28)
   mc_emergency_sp  = 0xc0003ffe4000  (0x2e0)
   in_mce   = 0x0 (0x2e8)
   data_offset  = 0x7f17  (0x30)
   hw_cpu_id= 0x8 (0x38)
   cpu_start= 0x1 (0x3a)
   kexec_state  = 0x0 (0x3b)
  [Enter for next page]
  0:mon
   __current= 0xc0007e696620  (0x290)
   kstack   = 0xc0007e6ebe30  (0x298)
   stab_rr  = 0xb (0x2a0)
   saved_r1 = 0xc0007ef37860  (0x2a8)
   trap_save= 0x0 (0x2b8)
   soft_enabled = 0x0 (0x2ba)
   irq_happened = 0x1 (0x2bb)
   io_sync  = 0x0 (0x2bc)
   irq_work_pending = 0x0 (0x2bd)
   nap_state_lost   = 0x0 (0x2be)
  0:mon
  
  (Based on a similar patch by Michael Ellerman m...@ellerman.id.au
  [v2] powerpc/xmon: Allow limiting the size of the paca display.
  This patch is an alternative and cannot coexist with the original.)
 
 
 So this is nice, but ... the diff is twice the size of my version, plus 128
 bytes of BSS, so I'm not sure the added benefit is sufficient to justify the
 added code complexity.
 
 But you can convince me otherwise if you feel strongly about it.
 
 cheers

I do think the output is a lot better paged like this :-)

The 128 byte buffer is a lot more than it needs for this particular command; it
could quite comfortably be lowered to about 32 (I was leaving space for other
commands to use it but there aren't any so far). I'll do this and repost.

Also, because the last_cmd_buf system is not specific to the paca display, it
could be used by the other paged commands (like the memory dumps). If we did
this we could (probably) remove ndump, nidump and ncsum which are all longs,
although I haven't worked out how much buffer space would be needed in
last_cmd_buf to support these (they have their own paging code but the
positional information could be stored in the string buffer). It's probably not
much work but might be a bit tricky. Do you think it's worth doing?

Since we're looking at memory usage, it looks like tmpstr[128] could be
removed without much work, saving 128 bytes and removing an unnecessary global
variable. If it actually turns out to be easy to do I'll post a separate patch.

Thanks for the revew,
Sam.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [1/1] powerpc/xmon: Paged output for paca display

2015-08-19 Thread Michael Ellerman
On Thu, 2015-08-20 at 10:42 +1000, Sam Bobroff wrote:
 On Tue, Aug 18, 2015 at 04:26:32PM +1000, Michael Ellerman wrote:
  On Fri, 2015-14-08 at 02:55:14 UTC, Sam bobroff wrote:
   The paca display is already more than 24 lines, which can be problematic
   if you have an old school 80x24 terminal, or more likely you are on a
   virtual terminal which does not scroll for whatever reason.
...
   
   (Based on a similar patch by Michael Ellerman m...@ellerman.id.au
   [v2] powerpc/xmon: Allow limiting the size of the paca display.
   This patch is an alternative and cannot coexist with the original.)
  
  
  So this is nice, but ... the diff is twice the size of my version, plus 128
  bytes of BSS, so I'm not sure the added benefit is sufficient to justify the
  added code complexity.
  
  But you can convince me otherwise if you feel strongly about it.
 
 I do think the output is a lot better paged like this :-)

OK.

 The 128 byte buffer is a lot more than it needs for this particular command; 
 it
 could quite comfortably be lowered to about 32 (I was leaving space for other
 commands to use it but there aren't any so far). I'll do this and repost.
 
 Also, because the last_cmd_buf system is not specific to the paca display, it
 could be used by the other paged commands (like the memory dumps). If we did
 this we could (probably) remove ndump, nidump and ncsum which are all longs,
 although I haven't worked out how much buffer space would be needed in
 last_cmd_buf to support these (they have their own paging code but the
 positional information could be stored in the string buffer). It's probably 
 not
 much work but might be a bit tricky. Do you think it's worth doing?

Not sure. The xmon code is in general incredibly crufty, so any clean ups are
always welcome. Certainly if we could come up with a general paging system that
would be great.

 Since we're looking at memory usage, it looks like tmpstr[128] could be
 removed without much work, saving 128 bytes and removing an unnecessary global
 variable. If it actually turns out to be easy to do I'll post a separate 
 patch.

Cool. Obviously in absolute terms 128 bytes is a non-issue, it was only meant
as a comparison between the two approaches.

cheers



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [1/1] powerpc/xmon: Paged output for paca display

2015-08-18 Thread Michael Ellerman
On Fri, 2015-14-08 at 02:55:14 UTC, Sam bobroff wrote:
 The paca display is already more than 24 lines, which can be problematic
 if you have an old school 80x24 terminal, or more likely you are on a
 virtual terminal which does not scroll for whatever reason.
 
 This adds an optional letter to the dp and dpa xmon commands
 (dpp and dppa), which will enable a per-page display (with 16
 line pages): the first page  will be displayed and if there was data
 that didn't fit, it will display a message indicating that the user can
 use enter to display the next page. The intent is that this feels
 similar to the way the memory display functions work.
 
 This is implemented by running over the entire output both for the
 initial command and for each subsequent page: the visible part is
 clipped out by checking line numbers. Handling the empty command as
 more is done by writing a special command into a static buffer that
 indicates where to move the sliding visibility window. This is similar
 to the approach used for the memory dump commands except that the
 state data is encoded into the last_cmd string, rather than a set of
 static variables. The memory dump commands could probably be rewritten
 to make use of the same buffer and remove their other static
 variables.
 
 Sample output:
 
 0:mon dpp1
 paca for cpu 0x1 @ cfdc0480:
  possible = yes
  present  = yes
  online   = yes
  lock_token   = 0x8000(0x8)
  paca_index   = 0x1   (0xa)
  kernel_toc   = 0xc0eb2400(0x10)
  kernelbase   = 0xc000(0x18)
  kernel_msr   = 0xb0001032(0x20)
  emergency_sp = 0xc0003ffe8000(0x28)
  mc_emergency_sp  = 0xc0003ffe4000(0x2e0)
  in_mce   = 0x0   (0x2e8)
  data_offset  = 0x7f17(0x30)
  hw_cpu_id= 0x8   (0x38)
  cpu_start= 0x1   (0x3a)
  kexec_state  = 0x0   (0x3b)
 [Enter for next page]
 0:mon
  __current= 0xc0007e696620(0x290)
  kstack   = 0xc0007e6ebe30(0x298)
  stab_rr  = 0xb   (0x2a0)
  saved_r1 = 0xc0007ef37860(0x2a8)
  trap_save= 0x0   (0x2b8)
  soft_enabled = 0x0   (0x2ba)
  irq_happened = 0x1   (0x2bb)
  io_sync  = 0x0   (0x2bc)
  irq_work_pending = 0x0   (0x2bd)
  nap_state_lost   = 0x0   (0x2be)
 0:mon
 
 (Based on a similar patch by Michael Ellerman m...@ellerman.id.au
 [v2] powerpc/xmon: Allow limiting the size of the paca display.
 This patch is an alternative and cannot coexist with the original.)


So this is nice, but ... the diff is twice the size of my version, plus 128
bytes of BSS, so I'm not sure the added benefit is sufficient to justify the
added code complexity.

But you can convince me otherwise if you feel strongly about it.

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev