[sr-dev] Fw: 32bit vs 64bit build problems
Daniel-Constantin Mierla
miconda at gmail.com
Thu Mar 31 18:48:59 CEST 2011
Hi Paul,
I committed the fix to return long value in get_stat_val(), soon I will
backport to 3.1 branch. I will add clear statistics mi command as well.
Thanks,
Daniel
On 3/29/11 3:59 PM, Paul Pankhurst wrote:
> 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 <mailto:miconda at gmail.com>
> *Sent:* Tuesday, March 29, 2011 11:59 AM
> *To:* Paul Pankhurst <mailto:paul at crocodile-rcs.com>
> *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 <mailto:miconda at gmail.com>
>> *Sent:* Tuesday, March 29, 2011 11:36 AM
>> *To:* Development mailing list of the sip-router project
>> <mailto:sr-dev at lists.sip-router.org>
>> *Cc:* Paul Pankhurst <mailto:paul at crocodile-rcs.com>
>> *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
>
>
> _______________________________________________
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20110331/0b835283/attachment-0001.htm>
More information about the sr-dev
mailing list