[sr-dev] Fw: 32bit vs 64bit build problems
Paul Pankhurst
paul at crocodile-rcs.com
Tue Mar 29 15:59:24 CEST 2011
Hi Daniel,
Here is the svn diff output against the 3.1.2 version we had in our local svn repo for the clear_stats command.
Note I added this as a new command rather than enhancement to the reset one to maintain backwards compatibility.
This version outputs details of the values after resetting them, with the old values in brackets so that you can
a) see if a variable was read only and thus not reset
b) see the final counts at the point of resetting them all
Also it makes a call to the bodge function get_stat_val_long in this code fragment.
Regards
Paul
Index: core_stats.c
===================================================================
--- core_stats.c (revision 8)
+++ core_stats.c (working copy)
@@ -95,10 +95,12 @@
static struct mi_root *mi_get_stats(struct mi_root *cmd, void *param);
static struct mi_root *mi_reset_stats(struct mi_root *cmd, void *param);
+static struct mi_root *mi_clear_stats(struct mi_root *cmd, void *param);
static mi_export_t mi_stat_cmds[] = {
{ "get_statistics", mi_get_stats, 0 , 0, 0 },
{ "reset_statistics", mi_reset_stats, 0 , 0, 0 },
+ { "clear_statistics", mi_clear_stats, 0 , 0, 0 },
{ 0, 0, 0, 0, 0}
};
@@ -207,15 +209,44 @@
node = addf_mi_node_child(rpl, 0, 0, 0, "%s:%s = %lu",
ZSW(get_stat_module(stat)),
ZSW(get_stat_name(stat)),
- get_stat_val(stat) );
+ get_stat_val_long(stat) );
if (node==0)
return -1;
return 0;
}
+inline static int mi_reset_and_add_stat(struct mi_node *rpl, stat_var *stat)
+{
+ struct mi_node *node;
+ long old_val, new_val;
+ if (stats_support()==0) return -1;
+ old_val=get_stat_val_long(stat);
+ reset_stat(stat);
+ new_val=get_stat_val_long(stat);
+
+ if (old_val==new_val)
+ {
+ node = addf_mi_node_child(rpl, 0, 0, 0, "%s:%s = %lu",
+ ZSW(get_stat_module(stat)),
+ ZSW(get_stat_name(stat)),
+ new_val);
+ }
+ else
+ {
+ node = addf_mi_node_child(rpl, 0, 0, 0, "%s:%s = %lu (%lu)",
+ ZSW(get_stat_module(stat)),
+ ZSW(get_stat_name(stat)),
+ new_val, old_val );
+ }
+
+ if (node==0)
+ return -1;
+ return 0;
+}
+
/* callback for counter_iterate_grp_vars. */
static void mi_add_grp_vars_cbk(void* r, str* g, str* n, counter_handle_t h)
{
@@ -227,13 +258,43 @@
g->len, g->s, n->len, n->s, counter_get_val(h));
}
+/* callback for counter_iterate_grp_vars to reset counters */
+static void mi_add_grp_vars_cbk2(void* r, str* g, str* n, counter_handle_t h)
+{
+ struct mi_node *rpl;
+ struct mi_node *node;
+ counter_val_t old_val, new_val;
+
+ rpl = r;
+ old_val = counter_get_val(h);
+ counter_reset(h);
+ new_val = counter_get_val(h);
+ if (old_val==new_val)
+ {
+ node = addf_mi_node_child(rpl, 0, 0, 0, "%.*s:%.*s = %lu",
+ g->len, g->s, n->len, n->s, new_val);
+ }
+ else
+ {
+ node = addf_mi_node_child(rpl, 0, 0, 0, "%.*s:%.*s = %lu (%lu)",
+ g->len, g->s, n->len, n->s, new_val, old_val);
+ }
+}
+
+
/* callback for counter_iterate_grp_names */
static void mi_add_all_grps_cbk(void* p, str* g)
{
counter_iterate_grp_vars(g->s, mi_add_grp_vars_cbk, p);
}
+/* callback for counter_iterate_grp_names to reset counters */
+static void mi_add_all_grps_cbk2(void* p, str* g)
+{
+ counter_iterate_grp_vars(g->s, mi_add_grp_vars_cbk2, p);
+}
+
static struct mi_root *mi_get_stats(struct mi_root *cmd, void *param)
{
struct mi_root *rpl_tree;
@@ -293,7 +354,66 @@
}
+static struct mi_root *mi_clear_stats(struct mi_root *cmd, void *param)
+{
+ struct mi_root *rpl_tree;
+ struct mi_node *rpl;
+ struct mi_node *arg;
+ stat_var *stat;
+ str val;
+
+ if(stats_support()==0)
+ return init_mi_tree( 404, "Statistics Not Found", 20);
+
+ if (cmd->node.kids==NULL)
+ return init_mi_tree( 400, MI_MISSING_PARM_S, MI_MISSING_PARM_LEN );
+
+ rpl_tree = init_mi_tree( 200, MI_OK_S, MI_OK_LEN);
+ if (rpl_tree==0)
+ return 0;
+ rpl = &rpl_tree->node;
+
+ for( arg=cmd->node.kids ; arg ; arg=arg->next) {
+ if (arg->value.len==0)
+ continue;
+
+ val = arg->value;
+
+ if ( val.len==3 && memcmp(val.s,"all",3)==0) {
+ /* add all statistic variables */
+ /* use direct counters access for that */
+ counter_iterate_grp_names(mi_add_all_grps_cbk2, rpl);
+ } else if ( val.len>1 && val.s[val.len-1]==':') {
+ /* add module statistics */
+ val.len--;
+ val.s[val.len]=0; /* zero term. */
+ /* use direct counters access for that */
+ counter_iterate_grp_vars(val.s, mi_add_grp_vars_cbk2, rpl);
+ val.s[val.len]=':' /* restore */;
+ } else {
+ /* reset & return only one statistic */
+ stat = get_stat( &val );
+
+ if (stat==0)
+ continue;
+ if (mi_reset_and_add_stat(rpl,stat)!=0)
+ goto error;
+ }
+ }
+
+ if (rpl->kids==0) {
+ free_mi_tree(rpl_tree);
+ return init_mi_tree( 404, "Statistics Not Found", 20);
+ }
+
+ return rpl_tree;
+error:
+ free_mi_tree(rpl_tree);
+ return 0;
+}
+
+
static struct mi_root *mi_reset_stats(struct mi_root *cmd, void *param)
{
struct mi_root *rpl_tree;
From: Daniel-Constantin Mierla
Sent: Tuesday, March 29, 2011 11:59 AM
To: Paul Pankhurst
Subject: Re: [sr-dev] Fw: 32bit vs 64bit build problems
Hi Paul,
the return of int instead of long is a bug and I will try to fix asap.
The new command to reset statistics is an addition and we can add it whenever you want. You don't need to wait for Peter, you can send the patch to sr-dev and will be added into the master branch (devel version). Over the time, if you plan to contribute more code and come with new patches I can just grant you commit access to git, so you can commit directly there, either on master branch or your own one and then merge to master.
Thanks,
Daniel
On 3/29/11 12:53 PM, Paul Pankhurst wrote:
Hi Daniel,
Anything that calls get_stat_val is potentially a problem – I did a ‘bodge’ fix by adding get_stat_val_long function and calling that from kex/core_stats.c instead of get_stat_val. I wasn’t sure if modifying get_stat_val was going to break something else that relied on the return NOT being a long from this function!
Incidentally I have also extended core_stats.c by adding a clear_statistics mi command that takes the same params as get_statistics, thus allowing users to reset a group or all stats at the same time. My intention is to submit this back to the communits(possibly packaged with Peters presence changes unless you’d prefer them separate)
Paul
From: Daniel-Constantin Mierla
Sent: Tuesday, March 29, 2011 11:36 AM
To: Development mailing list of the sip-router project
Cc: Paul Pankhurst
Subject: Re: [sr-dev] Fw: 32bit vs 64bit build problems
Hi Paul,
indeed, some of these stats are not consistent. I will go through them. General idea is that the old stats framework will be replaced by a new one (present already in the code) that does not need any kind of locking when updating the counters, thus is faster.
If you have specific examples of current stats that go nuts in 64b, please report them here so I will start with them then I will go through the rest of sources.
Thanks,
Daniel
On 3/29/11 12:04 PM, Paul Pankhurst wrote:
I have noticed that 'kamctl monitor' displays corrupt statistics when kamailio is built for a 64 bit platform.
Further investigation has revealed that the reason is the underlying datatype is a long, whilst some of the wrapper functions (e.g. get_stat_val) return unsigned int. On a 32bit machine where a long and int are both the same size it is not a problem, but on a 64 bit machine they are different and hence the problem.
Some code uses the wrapper function, other does not, in the case of mi_get_stats it uses both, so depending on paramaters passed to the MI fifo it may work or produce garbage results.
Paul
_______________________________________________
sr-dev mailing list
sr-dev at lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla
http://www.asipto.com
--
Daniel-Constantin Mierla
http://www.asipto.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20110329/95475dfd/attachment-0001.htm>
More information about the sr-dev
mailing list