Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fio: Adding fio engine #32

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fio: Adding fio engine #32

wants to merge 4 commits into from

Conversation

ErwanAliasr1
Copy link
Collaborator 

This first commit is adding a first cmdline engine_module to execute a
single fio command line.

This commit is not functional yet but set the base of the logic to parse
the engine.

@ErwanAliasr1
Copy link
Collaborator Author

rebasing on top of current main

@Arno500 Arno500 linked an issue Oct 30, 2024 that may be closed by this pull request
Add Fio support in hwbench #31
Open
@ErwanAliasr1 ErwanAliasr1 force-pushed the issue31 branch 11 times, most recently from 0f55da8 to 8d2ffe3 Compare November 5, 2024 13:53
@ErwanAliasr1
engines: Rename test_parse to test_parse_stressng
622a3fd 
As new engines are coming, let's name files so it's easy to remember
what file is doing what.

Signed-off-by: Erwan Velu <[email protected]>
@ErwanAliasr1
benchmark: Link parameter with benchmark
f78fed5 
The parameter has no link to the benchmark preventing engines extracting
the benchmark properties including the current job number.

This will be needed by fio to name its jobs.

Signed-off-by: Erwan Velu <[email protected]>
@ErwanAliasr1
bench/parameter: Adding get_name_with_position()
175db84 
This function is an helper to provide a unique name to benchmarks.
The current job position is padded to the current job name.

This commit:
- add this new function
- replace the existing occurences.
- change the output to report the name with position at runtime

Before :
[full_cpu_load] stressng/cpu/matrixprod(M): 1 stressor on CPU [0] for 60s
[full_cpu_load] stressng/cpu/matrixprod(M): 2 stressor on CPU [0] for 60s
[full_cpu_load] stressng/cpu/matrixprod(M): 4 stressor on CPU [0] for 60s

After:
[full_cpu_load_0] stressng/cpu/matrixprod(M): 1 stressor on CPU [0] for 60s
[full_cpu_load_1] stressng/cpu/matrixprod(M): 2 stressor on CPU [0] for 60s
[full_cpu_load_2] stressng/cpu/matrixprod(M): 4 stressor on CPU [0] for 60s

Signed-off-by: Erwan Velu <[email protected]>
@ErwanAliasr1
hwbench: Adding fio engine
8559885 
This commit is adding a first cmdline engine_module to execute a
single fio command line.

This code has been tested on fio-3.19, defining the minimal release
version.

To enable this mode, engine_module must be set to "cmdline".
The expected command line to forward to fio must be provided in the engine_module_parameter_base.

The command line will be tweaked by hwbench to ensure:
- runtime consistency with other engines : --time_based and --runtime are added
- output consistency: --output-format=json+ is added
- job naming: --name is adjusted to match hwbench's job name
- logs: --write_*_logs are enabled at a 20sec precision
- cache invalidation: each benchmark clears the cache to ensure an
  out-of-cache testing

Please note that :
- Fio's runtime will inherit automatically from hwbench's runtime value.
- --numjobs value will be fed with 'stressor_range' making possible to
  study the scalability of a device with a minimal code.

If one of these values were already present in the
engine_module_parameter_base, hwbench will replace them by the values
that were computed based on the benchmark descrption.

A sample configuration file (configs/fio.conf) is provided as an example, it will:
- test /dev/nvme0n1 in a randread 4k profile
- two benchmarks are automatically created as per the stressor_range
  value ("4,6") :
-- one with numjobs=4
-- one with numjobs=6

The testing suite is added to ensure a proper parsing and benchmarking job creation.

A documentation is also added to detail this first implementation
behavior.

Signed-off-by: Erwan Velu <[email protected]>
anisse
anisse reviewed Nov 28, 2024
View reviewed changes
hwbench/bench/benchmark.py
@@ -19,6 +19,7 @@ def __init__(
self.job_number = job_number
self.enginemodule = enginemodule
self.parameters = parameters
self.parameters.benchmark = self
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of unconditional set/get accessors in general, but since you added a set_benchmark, why not use it?

hwbench/engines/fio.py
# This avoid a case where the user already defined a value we need to control
for arg in args:
if arg.startswith(item[0]):
if arg != f"{item[0]}={item[1]}":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with this check is that it won't properly detect options like --numjobs 2, which are not separated by =

hwbench/engines/fio.py
ret = json.loads(stdout)
except json.decoder.JSONDecodeError:
print(
f"{self.parameters.get_name_with_position()}: Cannot load fio's JSON output"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should print stdout and stderr to debug why fio's JSON output can't be parsed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anisse anisse anisse left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Fio support in hwbench
2 participants
@ErwanAliasr1 @anisse