All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Straub <lukasstraub2@web.de>
To: "axboe@kernel.dk" <axboe@kernel.dk>
Cc: "fio@vger.kernel.org" <fio@vger.kernel.org>
Subject: [PATCH 7/8] blktrace.c: Make thread-safe by removing local static variables
Date: Wed, 19 Jan 2022 21:14:36 +0000	[thread overview]
Message-ID: <b805bb3f6acf6c5b4d8811872c62af939aac62a7.1642626314.git.lukasstraub2@web.de> (raw)
In-Reply-To: <cover.1642626314.git.lukasstraub2@web.de>

[-- Attachment #1: Type: text/plain, Size: 6944 bytes --]

Local static variables are not thread-safe. Make the functions in
blktrace.c safe by replacing them.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 blktrace.c | 63 ++++++++++++++++++++++++++++++++----------------------
 fio.h      |  1 +
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/blktrace.c b/blktrace.c
index 1faa83bf..e1804765 100644
--- a/blktrace.c
+++ b/blktrace.c
@@ -13,6 +13,12 @@
 #include "blktrace_api.h"
 #include "oslib/linux-dev-lookup.h"
 
+struct file_cache {
+	unsigned int maj;
+	unsigned int min;
+	unsigned int fileno;
+};
+
 /*
  * Just discard the pdu by seeking past it.
  */
@@ -87,28 +93,28 @@ static void trace_add_open_close_event(struct thread_data *td, int fileno, enum
 	flist_add_tail(&ipo->list, &td->io_log_list);
 }
 
-static int trace_add_file(struct thread_data *td, __u32 device)
+static int trace_add_file(struct thread_data *td, __u32 device,
+			  struct file_cache *cache)
 {
-	static unsigned int last_maj, last_min, last_fileno;
 	unsigned int maj = FMAJOR(device);
 	unsigned int min = FMINOR(device);
 	struct fio_file *f;
 	char dev[256];
 	unsigned int i;
 
-	if (last_maj == maj && last_min == min)
-		return last_fileno;
+	if (cache->maj == maj && cache->min == min)
+		return cache->fileno;
 
-	last_maj = maj;
-	last_min = min;
+	cache->maj = maj;
+	cache->min = min;
 
 	/*
 	 * check for this file in our list
 	 */
 	for_each_file(td, f, i)
 		if (f->major == maj && f->minor == min) {
-			last_fileno = f->fileno;
-			return last_fileno;
+			cache->fileno = f->fileno;
+			return cache->fileno;
 		}
 
 	strcpy(dev, "/dev");
@@ -128,10 +134,10 @@ static int trace_add_file(struct thread_data *td, __u32 device)
 		td->files[fileno]->major = maj;
 		td->files[fileno]->minor = min;
 		trace_add_open_close_event(td, fileno, FIO_LOG_OPEN_FILE);
-		last_fileno = fileno;
+		cache->fileno = fileno;
 	}
 
-	return last_fileno;
+	return cache->fileno;
 }
 
 static void t_bytes_align(struct thread_options *o, struct blk_io_trace *t)
@@ -195,7 +201,8 @@ static bool handle_trace_notify(struct blk_io_trace *t)
 static bool handle_trace_discard(struct thread_data *td,
 				 struct blk_io_trace *t,
 				 unsigned long long ttime,
-				 unsigned long *ios, unsigned long long *bs)
+				 unsigned long *ios, unsigned long long *bs,
+				 struct file_cache *cache)
 {
 	struct io_piece *ipo;
 	int fileno;
@@ -205,7 +212,7 @@ static bool handle_trace_discard(struct thread_data *td,
 
 	ipo = calloc(1, sizeof(*ipo));
 	init_ipo(ipo);
-	fileno = trace_add_file(td, t->device);
+	fileno = trace_add_file(td, t->device, cache);
 
 	ios[DDIR_TRIM]++;
 	if (t->bytes > bs[DDIR_TRIM])
@@ -238,12 +245,12 @@ static void dump_trace(struct blk_io_trace *t)
 
 static bool handle_trace_fs(struct thread_data *td, struct blk_io_trace *t,
 			    unsigned long long ttime, unsigned long *ios,
-			    unsigned long long *bs)
+			    unsigned long long *bs, struct file_cache *cache)
 {
 	int rw;
 	int fileno;
 
-	fileno = trace_add_file(td, t->device);
+	fileno = trace_add_file(td, t->device, cache);
 
 	rw = (t->action & BLK_TC_ACT(BLK_TC_WRITE)) != 0;
 
@@ -271,7 +278,8 @@ static bool handle_trace_fs(struct thread_data *td, struct blk_io_trace *t,
 }
 
 static bool handle_trace_flush(struct thread_data *td, struct blk_io_trace *t,
-			       unsigned long long ttime, unsigned long *ios)
+			       unsigned long long ttime, unsigned long *ios,
+			       struct file_cache *cache)
 {
 	struct io_piece *ipo;
 	int fileno;
@@ -281,7 +289,7 @@ static bool handle_trace_flush(struct thread_data *td, struct blk_io_trace *t,
 
 	ipo = calloc(1, sizeof(*ipo));
 	init_ipo(ipo);
-	fileno = trace_add_file(td, t->device);
+	fileno = trace_add_file(td, t->device, cache);
 
 	ipo->delay = ttime / 1000;
 	ipo->ddir = DDIR_SYNC;
@@ -298,28 +306,29 @@ static bool handle_trace_flush(struct thread_data *td, struct blk_io_trace *t,
  * due to internal workings of the block layer.
  */
 static bool queue_trace(struct thread_data *td, struct blk_io_trace *t,
-			 unsigned long *ios, unsigned long long *bs)
+			 unsigned long *ios, unsigned long long *bs,
+			 struct file_cache *cache)
 {
-	static unsigned long long last_ttime;
+	unsigned long long *last_ttime = &td->io_log_blktrace_last_ttime;
 	unsigned long long delay = 0;
 
 	if ((t->action & 0xffff) != __BLK_TA_QUEUE)
 		return false;
 
 	if (!(t->action & BLK_TC_ACT(BLK_TC_NOTIFY))) {
-		if (!last_ttime || td->o.no_stall || t->time < last_ttime)
+		if (!*last_ttime || td->o.no_stall || t->time < *last_ttime)
 			delay = 0;
 		else if (td->o.replay_time_scale == 100)
-			delay = t->time - last_ttime;
+			delay = t->time - *last_ttime;
 		else {
-			double tmp = t->time - last_ttime;
+			double tmp = t->time - *last_ttime;
 			double scale;
 
 			scale = (double) 100.0 / (double) td->o.replay_time_scale;
 			tmp *= scale;
 			delay = tmp;
 		}
-		last_ttime = t->time;
+		*last_ttime = t->time;
 	}
 
 	t_bytes_align(&td->o, t);
@@ -327,11 +336,11 @@ static bool queue_trace(struct thread_data *td, struct blk_io_trace *t,
 	if (t->action & BLK_TC_ACT(BLK_TC_NOTIFY))
 		return handle_trace_notify(t);
 	else if (t->action & BLK_TC_ACT(BLK_TC_DISCARD))
-		return handle_trace_discard(td, t, delay, ios, bs);
+		return handle_trace_discard(td, t, delay, ios, bs, cache);
 	else if (t->action & BLK_TC_ACT(BLK_TC_FLUSH))
-		return handle_trace_flush(td, t, delay, ios);
+		return handle_trace_flush(td, t, delay, ios, cache);
 	else
-		return handle_trace_fs(td, t, delay, ios, bs);
+		return handle_trace_fs(td, t, delay, ios, bs, cache);
 }
 
 static void byteswap_trace(struct blk_io_trace *t)
@@ -409,6 +418,7 @@ bool init_blktrace_read(struct thread_data *td, const char *filename, int need_s
 		goto err;
 	}
 	td->io_log_blktrace_swap = need_swap;
+	td->io_log_blktrace_last_ttime = 0;
 	td->o.size = 0;
 
 	free_release_files(td);
@@ -439,6 +449,7 @@ err:
 bool read_blktrace(struct thread_data* td)
 {
 	struct blk_io_trace t;
+	struct file_cache cache = { };
 	unsigned long ios[DDIR_RWDIR_SYNC_CNT] = { };
 	unsigned long long rw_bs[DDIR_RWDIR_CNT] = { };
 	unsigned long skipped_writes;
@@ -502,7 +513,7 @@ bool read_blktrace(struct thread_data* td)
 			}
 		}
 
-		if (!queue_trace(td, &t, ios, rw_bs))
+		if (!queue_trace(td, &t, ios, rw_bs, &cache))
 			continue;
 
 		if (td->o.read_iolog_chunked) {
diff --git a/fio.h b/fio.h
index 5c68ad80..1ea3d064 100644
--- a/fio.h
+++ b/fio.h
@@ -429,6 +429,7 @@ struct thread_data {
 	FILE *io_log_rfile;
 	unsigned int io_log_blktrace;
 	unsigned int io_log_blktrace_swap;
+	unsigned long long io_log_blktrace_last_ttime;
 	unsigned int io_log_current;
 	unsigned int io_log_checkmark;
 	unsigned int io_log_highmark;
-- 
2.34.1


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2022-01-19 21:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-19 21:14 [PATCH 0/8] blktrace: Add support for read_iolog_chunked and fixes Lukas Straub
2022-01-19 21:14 ` [PATCH 1/8] blktrace.c: Use file stream interface instead of fifo Lukas Straub
2022-01-19 21:14 ` [PATCH 2/8] iolog.c: Make iolog_items_to_fetch public Lukas Straub
2022-01-19 21:14 ` [PATCH 3/8] blktrace.c: Add support for read_iolog_chunked Lukas Straub
2022-01-19 21:14 ` [PATCH 4/8] linux-dev-lookup.c: Put the check for replay_redirect in the beginning Lukas Straub
2022-01-19 21:14 ` [PATCH 5/8] blktrace.c: Don't hardcode direct-io Lukas Straub
2022-01-19 21:14 ` [PATCH 6/8] blktrace.c: Don't sleep indefinitely if there is a wrong timestamp Lukas Straub
2022-01-19 21:14 ` Lukas Straub [this message]
2022-01-19 21:14 ` [PATCH 8/8] iolog.c: Fix memory leak for blkparse case Lukas Straub
2022-01-20 18:41 ` [PATCH 0/8] blktrace: Add support for read_iolog_chunked and fixes 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 \
    --in-reply-to=b805bb3f6acf6c5b4d8811872c62af939aac62a7.1642626314.git.lukasstraub2@web.de \
    --to=lukasstraub2@web.de \
    --cc=axboe@kernel.dk \
    --cc=fio@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.