diff mbox series

[blktests,v4,05/11] nvme{032,040}: Use runtime fio background jobs

Message ID 20230511140953.17609-6-dwagner@suse.de
State New
Headers
Series nvme testsuite runtime optimization |

Commit Message

Daniel Wagner May 11, 2023, 2:09 p.m. UTC 
  The fio jobs are supposed to run long in background during the test.
Instead relying on a job size use explicit runtime for this.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 tests/nvme/032 | 4 ++--
 tests/nvme/040 | 9 +++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

Comments

Chaitanya Kulkarni May 17, 2023, 4:40 a.m. UTC | #1 
On 5/11/23 07:09, Daniel Wagner wrote:
> The fio jobs are supposed to run long in background during the test.
> Instead relying on a job size use explicit runtime for this.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>

Is there any issue with the exiting approach for this change ?

-ck
Daniel Wagner May 18, 2023, 7:52 a.m. UTC | #2 
On Wed, May 17, 2023 at 04:40:52AM +0000, Chaitanya Kulkarni wrote:
> On 5/11/23 07:09, Daniel Wagner wrote:
> > The fio jobs are supposed to run long in background during the test.
> > Instead relying on a job size use explicit runtime for this.
> >
> > Signed-off-by: Daniel Wagner <dwagner@suse.de>
> 
> Is there any issue with the exiting approach for this change ?

The expectation of the test here is that there is a background job running.
Depending on the job size is an indirect way to express run at least for x
seconds. This gives a variable runtime as it depends the how fast fio jobs gets
executed. Explicitly telling the runtime is my opinion more robust and documents
the indention better.
Chaitanya Kulkarni May 18, 2023, 8:38 a.m. UTC | #3 
On 5/18/23 00:52, Daniel Wagner wrote:
> On Wed, May 17, 2023 at 04:40:52AM +0000, Chaitanya Kulkarni wrote:
>> On 5/11/23 07:09, Daniel Wagner wrote:
>>> The fio jobs are supposed to run long in background during the test.
>>> Instead relying on a job size use explicit runtime for this.
>>>
>>> Signed-off-by: Daniel Wagner <dwagner@suse.de>
>> Is there any issue with the exiting approach for this change ?
> The expectation of the test here is that there is a background job running.
> Depending on the job size is an indirect way to express run at least for x
> seconds. This gives a variable runtime as it depends the how fast fio jobs gets
> executed. Explicitly telling the runtime is my opinion more robust and documents
> the indention better.

agree, it is better to kill on rely on the variable while test is 
running ...

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck
diff mbox series

Patch

diff --git a/tests/nvme/032 b/tests/nvme/032
index 017d4a339971..93f53ac22a0b 100755
--- a/tests/nvme/032
+++ b/tests/nvme/032
@@ -38,8 +38,8 @@  test_device() {
 	sysfs="/sys/bus/pci/devices/${pdev}"
 
 	# start fio job
-	_run_fio_rand_io --filename="$TEST_DEV" --size=1g \
-		--group_reporting  &> /dev/null &
+	_run_fio_rand_io --filename="$TEST_DEV" \
+		--group_reporting --time_based --runtime=1d &> /dev/null &
 
 	sleep 5
 
diff --git a/tests/nvme/040 b/tests/nvme/040
index 04bd726cd309..10f924082f34 100755
--- a/tests/nvme/040
+++ b/tests/nvme/040
@@ -21,6 +21,7 @@  test() {
 	local port
 	local loop_dev
 	local nvmedev
+	local fio_pid
 
 	echo "Running ${TEST_NAME}"
 
@@ -37,8 +38,10 @@  test() {
 
 	# start fio job
 	echo "starting background fio"
-	_run_fio_rand_io --filename="/dev/${nvmedev}n1" --size=1g \
-		--group_reporting --ramp_time=5  &> /dev/null &
+	_run_fio_rand_io --filename="/dev/${nvmedev}n1" \
+		--group_reporting --ramp_time=5 \
+		--time_based --runtime=1d &> /dev/null &
+	fio_pid=$!
 	sleep 5
 
 	# do reset/remove operation
@@ -48,6 +51,8 @@  test() {
 	echo "deleting controller"
 	_nvme_delete_ctrl "${nvmedev}"
 
+	{ kill "${fio_pid}"; wait; } &> /dev/null
+
 	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
 	_remove_nvmet_subsystem "${subsys}"
 	_remove_nvmet_port "${port}"