From: Ammar Faizi <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Ammar Faizi <[email protected]>,
Alviro Iskandar Setiawan <[email protected]>,
Niklas Cassel <[email protected]>,
fio Mailing List <[email protected]>,
GNU/Weeb Mailing List <[email protected]>
Subject: [PATCH v1 4/8] blktrace: Fix broken error handling in `merge_blktrace_iologs()`
Date: Fri, 29 Apr 2022 07:47:01 +0700 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
From: Ammar Faizi <[email protected]>
There are several problems in this functions:
1) The error is handled inconsistently. (e.g. we have `ret = -errno`,
but we don't alter the `ret` when memory allocation fails).
2) Missing ENOMEM handling when allocating @bcs.
3) Wrong `fclose()` on cleanup. We have:
for (i = 0; i < nr_logs; i++)
fclose(bcs[i].f);
but at that point, the @nr_logs is already zero. Recall that
@nr_logs is zeroed inside the `while (nr_logs)` loop. The
merge_finish_file() decrements @nr_logs every loop cycle.
Fix and refactor them all.
Signed-off-by: Ammar Faizi <[email protected]>
---
blktrace.c | 73 +++++++++++++++++++++++++++++++++---------------------
1 file changed, 45 insertions(+), 28 deletions(-)
diff --git a/blktrace.c b/blktrace.c
index 619121c7..776bd4ab 100644
--- a/blktrace.c
+++ b/blktrace.c
@@ -700,21 +700,25 @@ static int write_trace(FILE *fp, struct blk_io_trace *t)
int merge_blktrace_iologs(struct thread_data *td)
{
+ int nr_logs_iter;
int nr_logs = get_max_str_idx(td->o.read_iolog_file);
- struct blktrace_cursor *bcs = malloc(sizeof(struct blktrace_cursor) *
- nr_logs);
+ struct blktrace_cursor *bcs;
struct blktrace_cursor *bc;
FILE *merge_fp;
char *str, *ptr, *name, *merge_buf;
int i, ret;
+ bcs = calloc(nr_logs, sizeof(struct blktrace_cursor));
+ if (!bcs)
+ return -ENOMEM;
+
ret = init_merge_param_list(td->o.merge_blktrace_scalars, bcs, nr_logs,
100, offsetof(struct blktrace_cursor,
scalar));
if (ret) {
log_err("fio: merge_blktrace_scalars(%d) != nr_logs(%d)\n",
ret, nr_logs);
- goto err_param;
+ goto out_free_bcs;
}
ret = init_merge_param_list(td->o.merge_blktrace_iters, bcs, nr_logs,
@@ -723,83 +727,96 @@ int merge_blktrace_iologs(struct thread_data *td)
if (ret) {
log_err("fio: merge_blktrace_iters(%d) != nr_logs(%d)\n",
ret, nr_logs);
- goto err_param;
+ goto out_free_bcs;
}
/* setup output file */
merge_fp = fopen(td->o.merge_blktrace_file, "w");
+ if (!merge_fp) {
+ ret = -errno;
+ goto out_free_bcs;
+ }
+
merge_buf = malloc(128 * 1024);
- if (!merge_buf)
- goto err_out_file;
+ if (!merge_buf) {
+ ret = -ENOMEM;
+ goto out_close_merge_fp;
+ }
+
ret = setvbuf(merge_fp, merge_buf, _IOFBF, 128 * 1024);
- if (ret)
- goto err_merge_buf;
+ if (ret) {
+ ret = -errno;
+ goto out_free_merge_buf;
+ }
/* setup input files */
str = ptr = strdup(td->o.read_iolog_file);
+ if (!str) {
+ ret = -ENOMEM;
+ goto out_free_merge_buf;
+ }
+
nr_logs = 0;
for (i = 0; (name = get_next_str(&ptr)) != NULL; i++) {
bcs[i].f = fopen(name, "rb");
if (!bcs[i].f) {
log_err("fio: could not open file: %s\n", name);
ret = -errno;
- free(str);
- goto err_file;
+ goto out_close_bcs_files;
}
nr_logs++;
if (!is_blktrace(name, &bcs[i].swap)) {
log_err("fio: file is not a blktrace: %s\n", name);
- free(str);
- goto err_file;
+ goto out_close_bcs_files;
}
ret = read_trace(td, &bcs[i]);
if (ret < 0) {
- free(str);
- goto err_file;
+ goto out_close_bcs_files;
} else if (!ret) {
merge_finish_file(bcs, i, &nr_logs);
i--;
}
}
- free(str);
/* merge files */
- while (nr_logs) {
- i = find_earliest_io(bcs, nr_logs);
+ nr_logs_iter = nr_logs;
+ while (nr_logs_iter) {
+ i = find_earliest_io(bcs, nr_logs_iter);
bc = &bcs[i];
/* skip over the pdu */
ret = discard_pdu(bc->f, &bc->t);
if (ret < 0) {
td_verror(td, -ret, "blktrace lseek");
- goto err_file;
+ goto out_close_bcs_files;
}
ret = write_trace(merge_fp, &bc->t);
ret = read_trace(td, bc);
if (ret < 0)
- goto err_file;
+ goto out_close_bcs_files;
else if (!ret)
- merge_finish_file(bcs, i, &nr_logs);
+ merge_finish_file(bcs, i, &nr_logs_iter);
}
/* set iolog file to read from the newly merged file */
td->o.read_iolog_file = td->o.merge_blktrace_file;
ret = 0;
-err_file:
- /* cleanup */
- for (i = 0; i < nr_logs; i++) {
+out_close_bcs_files:
+ for (i = 0; i < nr_logs; i++)
fclose(bcs[i].f);
- }
-err_merge_buf:
+ free(str);
+
+out_free_merge_buf:
free(merge_buf);
-err_out_file:
+
+out_close_merge_fp:
fflush(merge_fp);
fclose(merge_fp);
-err_param:
- free(bcs);
+out_free_bcs:
+ free(bcs);
return ret;
}
--
Ammar Faizi
next prev parent reply other threads:[~2022-04-29 0:47 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-29 0:46 [PATCH v1 0/8] fio error handling fixes Ammar Faizi
2022-04-29 0:46 ` [PATCH v1 1/8] cgroup: Add ENOMEM handling on a `malloc()` call Ammar Faizi
2022-04-29 18:20 ` Vincent Fu
2022-04-30 3:25 ` Alviro Iskandar Setiawan
2022-04-29 0:46 ` [PATCH v1 2/8] stat: Add ENOMEM handling on `malloc()` / `calloc()` calls Ammar Faizi
2022-04-29 0:47 ` [PATCH v1 3/8] engines/net: Add ENOMEM handling on a `malloc()` call Ammar Faizi
2022-04-29 18:20 ` Vincent Fu
2022-04-29 0:47 ` Ammar Faizi [this message]
2022-04-29 0:47 ` [PATCH v1 5/8] blktrace: Add ENOMEM handling when allocating @ipo Ammar Faizi
2022-04-29 0:47 ` [PATCH v1 6/8] blktrace: Add ENOMEM handling in `trace_add_open_close_event()` and its callers Ammar Faizi
2022-04-29 0:47 ` [PATCH v1 7/8] client: Add ENOMEM handling on `realloc()` calls Ammar Faizi
2022-04-29 0:47 ` [PATCH v1 8/8] client: Add ENOMEM handling on `malloc()`, `calloc()` and `strdup()` calls Ammar Faizi
2022-04-30 13:08 ` Jens Axboe
2022-04-29 18:21 ` [PATCH v1 0/8] fio error handling fixes Jens Axboe
2022-04-29 20:15 ` Ammar Faizi
2022-04-29 20:37 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox