From 0ccc7c3102676eb6e96047dd7eeaf1d55fcbfcd9 Mon Sep 17 00:00:00 2001 From: Raghavendra Bhat Date: Wed, 13 Jun 2012 12:12:18 +0530 Subject: debug/io-stats: do not store the string allocated from stack into dict * Unlock if some error happens after the lock is held * White space cleanup Change-Id: If90d9a9ae91c485bb21b1ad222af445981edb77b BUG: 769826 Signed-off-by: Raghavendra Bhat Reviewed-on: http://review.gluster.com/3565 Tested-by: Gluster Build System Reviewed-by: Jeff Darcy Reviewed-by: Anand Avati --- xlators/debug/io-stats/src/io-stats.c | 104 ++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 49 deletions(-) (limited to 'xlators/debug') diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c index be4656202..397b7dc53 100644 --- a/xlators/debug/io-stats/src/io-stats.c +++ b/xlators/debug/io-stats/src/io-stats.c @@ -293,43 +293,43 @@ is_fop_latency_started (call_frame_t *frame) } \ UNLOCK (&iosstat->lock); \ ios_stat_add_to_list (&conf->list[type], \ - value, iosstat); \ + value, iosstat); \ \ } while (0) #define BUMP_THROUGHPUT(iosstat, type) \ do { \ - struct ios_conf *conf = NULL; \ - double elapsed; \ - struct timeval *begin, *end; \ - double throughput; \ + struct ios_conf *conf = NULL; \ + double elapsed; \ + struct timeval *begin, *end; \ + double throughput; \ int flag = 0; \ - \ - begin = &frame->begin; \ - end = &frame->end; \ - \ - elapsed = (end->tv_sec - begin->tv_sec) * 1e6 \ - + (end->tv_usec - begin->tv_usec); \ - throughput = op_ret / elapsed; \ - \ - conf = this->private; \ - LOCK(&iosstat->lock); \ - { \ - if (iosstat->thru_counters[type].throughput \ + \ + begin = &frame->begin; \ + end = &frame->end; \ + \ + elapsed = (end->tv_sec - begin->tv_sec) * 1e6 \ + + (end->tv_usec - begin->tv_usec); \ + throughput = op_ret / elapsed; \ + \ + conf = this->private; \ + LOCK(&iosstat->lock); \ + { \ + if (iosstat->thru_counters[type].throughput \ <= throughput) { \ - iosstat->thru_counters[type].throughput = \ + iosstat->thru_counters[type].throughput = \ throughput; \ - gettimeofday (&iosstat-> \ + gettimeofday (&iosstat-> \ thru_counters[type].time, NULL); \ flag = 1; \ } \ } \ - UNLOCK (&iosstat->lock); \ + UNLOCK (&iosstat->lock); \ if (flag) \ ios_stat_add_to_list (&conf->thru_list[type], \ throughput, iosstat); \ - } while (0) + } while (0) int ios_fd_ctx_get (fd_t *fd, xlator_t *this, struct ios_fd **iosfd) @@ -484,12 +484,12 @@ ios_stat_add_to_list (struct ios_stat_head *list_head, uint64_t value, new = GF_CALLOC (1, sizeof (*new), gf_io_stats_mt_ios_stat_list); new->iosstat = iosstat; - new->value = value; + new->value = value; ios_stat_ref (iosstat); - list_add_tail (&new->list, &tmp->list); + list_add_tail (&new->list, &tmp->list); stat = last->iosstat; last->iosstat = NULL; - ios_stat_unref (stat); + ios_stat_unref (stat); list_del (&last->list); GF_FREE (last); if (reposition == MAX_LIST_MEMBERS) @@ -511,7 +511,7 @@ ios_stat_add_to_list (struct ios_stat_head *list_head, uint64_t value, list_head->members++; if (list_head->min_cnt > value) list_head->min_cnt = value; - } + } } out: UNLOCK (&list_head->lock); @@ -735,13 +735,13 @@ io_stats_dump_global_to_logfp (xlator_t *this, struct ios_global_stats *stats, ios_log (this, logfp, "\nTIMESTAMP \t\t\t THROUGHPUT(KBPS)" "\tFILE NAME"); list_head = &conf->thru_list[IOS_STATS_THRU_READ]; - ios_dump_throughput_stats(list_head, this, logfp, IOS_STATS_THRU_READ); + ios_dump_throughput_stats(list_head, this, logfp, IOS_STATS_THRU_READ); ios_log (this, logfp, "\n======Write Throughput File Stats======"); ios_log (this, logfp, "\nTIMESTAMP \t\t\t THROUGHPUT(KBPS)" "\tFILE NAME"); list_head = &conf->thru_list[IOS_STATS_THRU_WRITE]; - ios_dump_throughput_stats (list_head, this, logfp, IOS_STATS_THRU_WRITE); + ios_dump_throughput_stats (list_head, this, logfp, IOS_STATS_THRU_WRITE); } return 0; } @@ -1082,18 +1082,19 @@ io_stats_dump_stats_to_dict (xlator_t *this, dict_t *resp, ios_stats_thru_t index = IOS_STATS_THRU_MAX; struct tm *tm = NULL; char timestr[256] = {0, }; + char *dict_timestr = NULL; conf = this->private; switch (flags) { - case IOS_STATS_TYPE_OPEN: + case IOS_STATS_TYPE_OPEN: list_head = &conf->list[IOS_STATS_TYPE_OPEN]; LOCK (&conf->lock); { ret = dict_set_uint64 (resp, "current-open", conf->cumulative.nr_opens); if (ret) - goto out; + goto unlock; ret = dict_set_uint64 (resp, "max-open", conf->cumulative.max_nr_opens); @@ -1103,11 +1104,15 @@ io_stats_dump_stats_to_dict (xlator_t *this, dict_t *resp, ".%"GF_PRI_SUSECONDS, conf->cumulative.max_openfd_time.tv_usec); - ret = dict_set_str (resp, "max-openfd-time", - timestr); + dict_timestr = gf_strdup (timestr); + if (!dict_timestr) + goto unlock; + ret = dict_set_dynstr (resp, "max-openfd-time", + dict_timestr); if (ret) - goto out; + goto unlock; } + unlock: UNLOCK (&conf->lock); break; @@ -1125,7 +1130,7 @@ io_stats_dump_stats_to_dict (xlator_t *this, dict_t *resp, break; case IOS_STATS_TYPE_READ_THROUGHPUT: list_head = &conf->thru_list[IOS_STATS_THRU_READ]; - index = IOS_STATS_THRU_READ; + index = IOS_STATS_THRU_READ; break; case IOS_STATS_TYPE_WRITE_THROUGHPUT: list_head = &conf->thru_list[IOS_STATS_THRU_WRITE]; @@ -1146,28 +1151,29 @@ io_stats_dump_stats_to_dict (xlator_t *this, dict_t *resp, snprintf (key, 256, "%s-%d", "filename", cnt); ret = dict_set_str (resp, key, entry->iosstat->filename); if (ret) - goto out; + goto unlock_list_head; snprintf (key, 256, "%s-%d", "value",cnt); ret = dict_set_uint64 (resp, key, entry->value); if (ret) - goto out; + goto unlock_list_head; if (index != IOS_STATS_THRU_MAX) { snprintf (key, 256, "%s-%d", "time-sec", cnt); - ret = dict_set_int32 (resp, key, + ret = dict_set_int32 (resp, key, entry->iosstat->thru_counters[index].time.tv_sec); if (ret) - goto out; + goto unlock_list_head; snprintf (key, 256, "%s-%d", "time-usec", cnt); - ret = dict_set_int32 (resp, key, + ret = dict_set_int32 (resp, key, entry->iosstat->thru_counters[index].time.tv_usec); if (ret) - goto out; + goto unlock_list_head; } if (cnt == list_cnt) break; } } +unlock_list_head: UNLOCK (&list_head->lock); ret = dict_set_int32 (resp, "members", cnt); @@ -1246,7 +1252,7 @@ io_stats_open_cbk (call_frame_t *frame, void *cookie, xlator_t *this, struct ios_stat *iosstat = NULL; struct ios_conf *conf = NULL; - conf = this->private; + conf = this->private; path = frame->local; frame->local = NULL; @@ -1362,7 +1368,7 @@ io_stats_writev_cbk (call_frame_t *frame, void *cookie, xlator_t *this, ios_inode_ctx_get (inode, this, &iosstat); if (iosstat) { BUMP_STATS (iosstat, IOS_STATS_TYPE_WRITE); - BUMP_THROUGHPUT (iosstat, IOS_STATS_THRU_WRITE); + BUMP_THROUGHPUT (iosstat, IOS_STATS_THRU_WRITE); inode = NULL; iosstat = NULL; } @@ -2418,7 +2424,7 @@ io_stats_release (xlator_t *this, fd_t *fd) LOCK (&conf->lock); { - conf->cumulative.nr_opens--; + conf->cumulative.nr_opens--; } UNLOCK (&conf->lock); @@ -2471,16 +2477,16 @@ ios_init_top_stats (struct ios_conf *conf) LOCK_INIT (&conf->list[i].lock); } - for (i = 0; i < IOS_STATS_THRU_MAX; i ++) { - conf->thru_list[i].iosstats = GF_CALLOC (1, - sizeof (*conf->thru_list[i].iosstats), - gf_io_stats_mt_ios_stat); + for (i = 0; i < IOS_STATS_THRU_MAX; i ++) { + conf->thru_list[i].iosstats = GF_CALLOC (1, + sizeof (*conf->thru_list[i].iosstats), + gf_io_stats_mt_ios_stat); - if (!conf->thru_list[i].iosstats) + if (!conf->thru_list[i].iosstats) return -1; - INIT_LIST_HEAD(&conf->thru_list[i].iosstats->list); - LOCK_INIT (&conf->thru_list[i].lock); + INIT_LIST_HEAD(&conf->thru_list[i].iosstats->list); + LOCK_INIT (&conf->thru_list[i].lock); } return 0; -- cgit