All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <Niklas.Cassel@wdc.com>
To: "axboe@kernel.dk" <axboe@kernel.dk>
Cc: "fio@vger.kernel.org" <fio@vger.kernel.org>,
	Niklas Cassel <Niklas.Cassel@wdc.com>,
	Shane Moore <Shane.Moore@wdc.com>,
	Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
Subject: [PATCH] filesetup: create zbd_info before jumping to done label
Date: Thu, 2 Dec 2021 09:42:06 +0000	[thread overview]
Message-ID: <20211202094153.8381-1-Niklas.Cassel@wdc.com> (raw)

From: Niklas Cassel <niklas.cassel@wdc.com>

For a thread that has zonemode == ZONE_MODE_ZBD set, the zbd code requires
that each file (for that thread) has a valid f->zbd_info pointer.

This intent was further clarified by commit 5ddf46d0b2df ("zbd: change some
f->zbd_info conditionals to asserts").

The zbd info pointer is set by zbd_init_files(), either by creating a new
zbd_info struct, or by increasing the refcount of an existing zbd_info.

A zbd_info struct contains the in memory state of the zones, including e.g.
each zone's wp and zone capacity.

Normally, zbd_init_files() is always called, even for read only workloads.
However, in the case where a read iolog was supplied, setup_files()
currently jumps to the done label before zbd_init_files() has been called.

Even for a read only workload, zbd_adjust_block() will do things as
checking if the read I/O is below the wp (unless td->o.read_beyond_wp is
enabled). In order to be able to do this comparison, we need a valid
zbd_info.

There is no reason why the zbd code should treat a read only workload
different from a read iolog workload. (E.g. the wp for the zones might
have changed since the read iolog was recorded.)

If the user for some reason wants to disregard the wp check during a read
iolog workload, the td->o.read_beyond_wp option can be used, just like in
the regular read only workload case.

Move the read iolog check and the matching "goto done" after the call to
zbd_init_files(). This way, we treat a read iolog workload simlar to a
regular read only workload, while avoiding an assertion failure in
zbd_setup_files() (which is called after the done label).

Reported-by: Shane Moore <shane.moore@wdc.com>
Suggested-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Tested-by: Shane Moore <shane.moore@wdc.com>
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
---
 filesetup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/filesetup.c b/filesetup.c
index 228e4fff..fb556d84 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -1119,9 +1119,6 @@ int setup_files(struct thread_data *td)
 	if (err)
 		goto err_out;
 
-	if (o->read_iolog_file)
-		goto done;
-
 	if (td->o.zone_mode == ZONE_MODE_ZBD) {
 		err = zbd_init_files(td);
 		if (err)
@@ -1129,6 +1126,9 @@ int setup_files(struct thread_data *td)
 	}
 	zbd_recalc_options_with_zone_granularity(td);
 
+	if (o->read_iolog_file)
+		goto done;
+
 	/*
 	 * check sizes. if the files/devices do not exist and the size
 	 * isn't passed to fio, abort.
-- 
2.33.1

             reply	other threads:[~2021-12-02  9:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-02  9:42 Niklas Cassel [this message]
2021-12-03  0:53 ` [PATCH] filesetup: create zbd_info before jumping to done label Damien Le Moal
2021-12-03  0:54 ` 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=20211202094153.8381-1-Niklas.Cassel@wdc.com \
    --to=niklas.cassel@wdc.com \
    --cc=Dmitry.Fomichev@wdc.com \
    --cc=Shane.Moore@wdc.com \
    --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.