[Crash-utility] ps: improve performance

Dave Anderson anderson at prospeed.net
Wed Aug 20 00:35:28 UTC 2014


> Hello dave,
>
> This patch is used to improve ps command performance, especilly on RHEL 7.
> And the author is Pan Fengyun <panfy.fnst at cn.fujitsu.com>.
>
> Please check.

Hello Qiao,

I am currently on vacation and will not be able to fully review this
patch until I return in September.

A patch that changes such fundamental workings of the task handling
functionality is going to take a significant amount of review and
testing.  And a quick cursory review of the patch shows several issues
that are problematic.

For example, the change to task_to_pid() is just wrong.  That function
is exported in defs.h to any other function or extension module, so it
makes no sense to call hq_entry_exists(), because the hash queue is only
usable during the period of time that a single command runs and has opened
the queue.  So perhaps Pan does not understand that the hash queue
facility can only be used on a per-command basis?

Also, changing the arguments to exported functions such as
get_task_mem_usage() should not be done because it would break
any extension module that uses it.  And related to that, the
ugly changes to vm_area_dump() are just unnacceptable.  Does
he expect every user of get_task_mem_usage() to have to go
through the same hoops instead of just making a single call?

Another potential problem is the use of malloc() for anything
that is not permanent.  If the command is interrupted for whatever
reason, the memory will be leaked.  GETBUF()/FREEBUF() should be
used for memory allocated for the requirements of a single command.

Thanks,
  Dave

>
> --
> Regards
> Qiao Nuohan
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility




More information about the Crash-utility mailing list