This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
[PATCH] Fix regression in statistic operations
- From: Anton Vorontsov <avorontsov at ru dot mvista dot com>
- To: Wenji Huang <wenji dot huang at oracle dot com>, "Frank Ch. Eigler" <fche at redhat dot com>
- Cc: "systemtap at sources dot redhat dot com" <systemtap at sources dot redhat dot com>
- Date: Fri, 4 Dec 2009 07:37:56 +0300
- Subject: [PATCH] Fix regression in statistic operations
- References: <20091202192710.GJ10331@redhat.com> <20091203154442046.00000005292@ETSD-Ora-lap2>
- Reply-to: avorontsov at ru dot mvista dot com
In commit 98c783852039061db8c1611742660aaded0eab77 ("Use proper types
for do_div") I imprudently changed some variables to an unsigned type
while in some places the code actually relies on a sign.
So, let's be a bit smarter now and use temporary variables.
Reported-by: Wenji Huang <wenji.huang@oracle.com>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
On Thu, Dec 03, 2009 at 03:44:42PM +0800, Wenji Huang wrote:
> Hi,
>
> There's one regression on statistics operation. For example,
>
> Running ./systemtap.maps/ix.exp ...
> executing: stap ./systemtap.maps/ix.stp
> FAIL: ./systemtap.maps/ix.stp
> line 1: expected "foo[0]: count:3 sum:98 avg:32 min:-2 max:100"
> Got "foo[0]: count:3 sum:98 avg:32 min:0 max:-2"
>
> It seems this is introduced by the commit.
>
> commit 98c783852039061db8c1611742660aaded0eab77
> Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> Date: Sat Nov 28 01:33:47 2009 +0300
>
> Use proper types for do_div
>
> The test case works fine once I reverted the commit.
Oops. :-/ This patch should fix the issue, thanks for noticing and
sorry for the inconvenience.
This time I checked (on x86_64) that no new regressions introduced.
Before all my patches:
=== systemtap Summary ===
# of expected passes 439
# of unexpected failures 275
# of expected failures 214
# of known failures 3
# of untested testcases 202
# of unsupported tests 4
After all my patches:
=== systemtap Summary ===
# of expected passes 439
# of unexpected failures 275
# of expected failures 214
# of known failures 3
# of untested testcases 202
# of unsupported tests 4
And that was before this particular fix:
=== systemtap Summary ===
# of expected passes 428
# of unexpected failures 286
# of expected failures 214
# of known failures 3
# of untested testcases 202
# of unsupported tests 4
runtime/stat-common.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/runtime/stat-common.c b/runtime/stat-common.c
index f970304..fabe840 100644
--- a/runtime/stat-common.c
+++ b/runtime/stat-common.c
@@ -34,9 +34,10 @@ static int _stp_stat_calc_buckets(int stop, int start, int interval)
return buckets;
}
-static int needed_space(uint64_t v)
+static int needed_space(int64_t v)
{
int space = 0;
+ uint64_t tmp;
if (v == 0)
return 1;
@@ -45,9 +46,10 @@ static int needed_space(uint64_t v)
space++;
v = -v;
}
- while (v) {
+ tmp = v;
+ while (tmp) {
/* v /= 10; */
- do_div (v, 10);
+ do_div(tmp, 10);
space++;
}
return space;
@@ -134,7 +136,8 @@ static void _stp_stat_print_histogram_buf(char *buf, size_t size, Hist st, stat
{
int scale, i, j, val_space, cnt_space;
int low_bucket = -1, high_bucket = 0, over = 0, under = 0;
- uint64_t val, v, valmax = 0;
+ int64_t val, valmax = 0;
+ uint64_t v;
int eliding = 0;
char *cur_buf = buf, *fake = buf;
char **bufptr = (buf == NULL ? &fake : &cur_buf);
@@ -282,7 +285,7 @@ static void _stp_stat_print_histogram(Hist st, stat *sd)
_stp_print_flush();
}
-static void __stp_stat_add (Hist st, stat *sd, uint64_t val)
+static void __stp_stat_add(Hist st, stat *sd, int64_t val)
{
int n;
if (sd->count == 0) {
@@ -310,7 +313,10 @@ static void __stp_stat_add (Hist st, stat *sd, uint64_t val)
if (val < 0)
val = 0;
else {
- do_div (val, st->interval);
+ uint64_t tmp = val;
+
+ do_div(tmp, st->interval);
+ val = tmp;
val++;
}
--
1.6.3.3