Hi Nathan,
It has taken a bit longer than I originally planned it to but I have
made all of the suggested changes to the code that were pointed out.
A list of my changes are below:
Made general improvements with code for the pmda over last time whilst
taking on suggestions made by Nathan Scott. Have improved collection
code for the glock_lock_time trace-point metrics as well as switching to
the provided pmdaCache(3) data structure instead of a home made
linked-list data structure.
Introduced a new gfs2.control.glock_lock_time metric which can be used
to enable/disable the glock_lock_time statistic collection based on its
value, this can be set using pmStore(3)on this metric. On installs
without the support for trace-point based statistics, the metrics are
disabled by default because the necessary config files will not be found.
Split the QA files into two halves, the first 654 handles testing of the
base part of the pmda (glstats, sbstats, glocks metrics). 655 then
handles testing for the trace-point based metrics with not_run escapes
in an event that trace-point based metrics are not available for the
install (i.e. Debian unstable?). A new common.gfs2 script handles common
functions used in both of the tests.
#modified: src/pmdas/gfs2/GNUmakefile
Misc. changes to include the new control based metric files into the pmda.
#modified: src/pmdas/gfs2/Install
Have removed the enabling of the glock_lock_time trace-point from here
and moved it within the pmda itself.
#new file: src/pmdas/gfs2/control.c, new file: src/pmdas/gfs2/control.h
The control based metrics current have the functions to amend the
control.glock_lock_time metric, the plan is to use this as a future
method to finely control which trace-point based metrics are enabled at
any time by switching them on or off using pmStore(3).
#modified: src/pmdas/gfs2/glocks.c, modified: src/pmdas/gfs2/glstats.c
Misc. code and comment cleanup.
#modified: src/pmdas/gfs2/help
Cleaned up the help text to reflect the changes to the glock_lock_time
metrics as well as adding additional entries for the new control metric.
#modified: src/pmdas/gfs2/lock_time.c, modified: src/pmdas/gfs2/lock_time.h
Cleaned up the collection loop for the trace-point data, have moved to a
single loop that removes the dependency on allocation of dynamic memory
to achieve collection of the data. These changes will reduce the memory
requirements of the pmda and speeds up the pmda with less iteration of
the data.
Changed the data structure to hold our collected data, I have gone from
home made linked-list to the available pmdaCache(3) this reduces the
amount of code, simplifies reading and reduces again the amount of
iteration of the locks we have (no more remove duplicate and easier
addition through a well defined hash).
Changed the collection rate by changing the calling
gfs2_refresh_lock_time() from once-per each file-system to once for all
file-systems, this reduces the complexity of data storage. Reducing
potential of lost lock data in-between refreshes.
#modified: src/pmdas/gfs2/pmda.c
Changed the number of calls to gfs2_refresh_lock_time() from once per
each filesystem to now being called once for all file-systems also
removed forgotten testing code that was left within the
gfs2_fetch_refresh function.
Separated instance refresh so that the collection of dev_id is handled
within another function in order to increase code readability.
Introduced the necessary pmStore(3) hooks so that we can use the control
based metrics to handle whether we are collecting statistics from the
trace-point based metrics borrowing from the techniques used in the
linux pmda.
#modified: src/pmdas/gfs2/pmdagfs2.h
Moved dev_id to the top of the structure as suggested.
#modified: src/pmdas/gfs2/pmns
Added new control metric in order to enable/disable collecting of the
glock_lock_time metrics.
#modified: qa/654
Split the test into half, this file now checks for support for gfs2,
attempts to probe the required modules and if they exist and are not
already loaded. We test glstats, sbstats and glock metrics here.
#new file: qa/655
Handles the testing of the current trace-point based metrics and will be
used for any future ones. Checks for support of these types of metrics,
if they do not exist the test is not run.
#modified: qa/GNUmakefile
Changes to the makefile to include the new common.gfs2 script.
#new file: qa/common.gfs2
Contains functions which will be used by both the 654 and 655 tests.
#modified: qa/group
Added entry for the new 655 test.
These changes are available at git://github.com/pauljevans/pcp.git gfs2
I look forward to feedback.
Regards,
Paul.
|