-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
add fs_mark harness #233
base: master
Are you sure you want to change the base?
add fs_mark harness #233
Conversation
885f77e
to
f63355e
Compare
9755602
to
385db8f
Compare
1a5c59e
to
c0ef729
Compare
c0ef729
to
9acbc20
Compare
666d7b6
to
83b4faf
Compare
e2cd77f
to
377d622
Compare
c7715b6
to
b700069
Compare
20b14c8
to
9c72080
Compare
cef2eb5
to
599365e
Compare
9193843
to
5c61710
Compare
return -2 | ||
else: | ||
return 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's place there sample output from fs mark like it's done for mbedtls - it will be much easier to analyze the harness then, sth like:
FSUse% Count Size Files/sec App Overhead CREAT (Min/Avg/Max) WRITE (Min/Avg/Max) FSYNC (Min/Avg/Max) SYNC (Min/Avg/Max) CLOSE (Min/Avg/Max) UNLINK (Min/Avg/Max)
27 10 0 71.1 19612 10439 11319 12782 327 384 479 0 0 0 0 0 0 372 396 428 0 0 0
27 20 0 66.1 20880 11356 12248 13971 367 389 407 0 0 0 0 0 0 375 400 438 0 0 0
27 30 0 67.4 18939 11368 12152 13179 371 399 503 0 0 0 0 0 0 377 394 422 0 0 0
27 40 0 53.5 26032 11556 15166 28925 372 450 1014 0 0 0 0 0 0 379 462 954 0 0 0
27 50 0 57.2 23061 12960 14265 18243 375 457 967 0 0 0 0 0 0 383 445 836 0 0 0
27 60 0 60.9 19539 12836 13617 15040 373 406 498 0 0 0 0 0 0 382 436 762 0 0 0
27 70 0 52.8 23458 12563 15690 25471 337 439 807 0 0 0 0 0 0 380 447 810 0 0 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to add it like in mbedtls but flake8 complain about unused variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still include a sample output, for example, in the form of:
"""
OUTPUT
"""
fs_mark/fs_mark_clean.c
Outdated
while ((ret = rmdir(dirPath)) < 0) { | ||
if (errno == ENOTEMPTY && try < 5) { | ||
remove_dir_recursive(dirPath); | ||
try++; | ||
} | ||
else if (errno == ENOENT) { | ||
errno = 0; | ||
return 0; | ||
} | ||
else { | ||
return -1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary? We can remove dirs also using remove()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But using rmdir()
is semantically correct.
targets: | ||
value: [armv7a7-imx6ull-evk] | ||
|
||
# TODO: full fs filling tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it already added?
- name: fs_mark_pageSizeFiles_alignedWrite_20_threads | ||
execute: fs_mark -d /fs_mark_test -t 20 -D 20 -N 1 -s 4096 -w 32 -n 20 -v -S 0 -L 5 | ||
|
||
# s=10000 w=32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to remove (?)
# s=10000 w=32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with the other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can remain, this comment show how this tests are organized. Full fs filling tests are not added for now due to difficulty of checking how much space is left on jffs2.
# This yaml contain armv7a7-imx6ull-evk specific fs_mark tests | ||
test: | ||
type: harness | ||
harness: fs_mark.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed I would consider adding some nested cases to provide better coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests are not straight forward to implement, maybe it is better to add it later on.
5c61710
to
f527874
Compare
fs_mark/fs_mark.py
Outdated
loop_time = 1200 | ||
first_loop_done = False | ||
|
||
# Single line of input (wraped to pass linter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[codespell] reported by reviewdog 🐶
wraped ==> wrapped, warped
7271997
to
56501b5
Compare
JIRA: CI-307
56501b5
to
9d59598
Compare
@@ -0,0 +1 @@ | |||
$(eval $(call add_test, fs-mark-clean)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm considering whether it might be better to rename the macro, as it's not actually a test after all.. However, we could discuss this during something lika a repository cleanup/organization, not directly here.
@@ -0,0 +1,159 @@ | |||
import trunner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Group imports (standard library, third-party, and local imports)
|
||
|
||
def harness(dut: Dut, ctx: TestContext, result: TestResult): | ||
target = trunner.ctx.target.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't ctx.target.name
be sufficient? In that case, the import trunner
would be unnecessary
return 0 | ||
|
||
|
||
def harness(dut: Dut, ctx: TestContext, result: TestResult): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could specify what the function should return ->
from trunner.types import TestResult, Status | ||
|
||
|
||
def clean(dut, test_name, ctx: TestContext) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def clean(dut, test_name, ctx: TestContext) -> int: | |
def clean(dut: Dut, test_name: str, ctx: TestContext) -> int: |
if idx == 0: | ||
return 0 | ||
elif idx == 1: | ||
return -1 | ||
elif idx == 2: | ||
return -2 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe something like:
if idx in [0,1,2]:
return -idx
return -2 | ||
else: | ||
return 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still include a sample output, for example, in the form of:
"""
OUTPUT
"""
return 1; | ||
} | ||
/* Clean test directory */ | ||
errno = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is there errno ?
if not t_data.timing_dict[(target, name)][0] <= int(value) <= t_data.timing_dict[(target, name)][1]: | ||
test_status = Status.FAIL | ||
test_msg += ''.join(('\n\t', name, ' exec time - ', value, ' out of range [', | ||
str(t_data.timing_dict[( | ||
target, name)][0]), ' - ', | ||
str(t_data.timing_dict[(target, name)][1]), ']')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this approach will be more readable ?:
if not t_data.timing_dict[(target, name)][0] <= int(value) <= t_data.timing_dict[(target, name)][1]: | |
test_status = Status.FAIL | |
test_msg += ''.join(('\n\t', name, ' exec time - ', value, ' out of range [', | |
str(t_data.timing_dict[( | |
target, name)][0]), ' - ', | |
str(t_data.timing_dict[(target, name)][1]), ']')) | |
min_val, max_val = t_data.timing_dict[(target, name)] | |
if not min_val <= int(value) <= max_val: | |
test_status = Status.FAIL | |
test_msg += f"\n\t{name} exec time - {value} out of range [{min_val} - {max_val}]" |
test_msg += "\n\n\tF_Use%: " + str(f_use) | ||
test_msg += "\n\tCount: " + str(count) | ||
test_msg += "\n\tSize: " + str(size) | ||
test_msg += "\n\tFiles/sec: " + str(files_sec) | ||
test_msg += "\n\tApp overhead: " + str(app_overhead) + "\n\t" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about this? :
test_msg += "\n\n\tF_Use%: " + str(f_use) | |
test_msg += "\n\tCount: " + str(count) | |
test_msg += "\n\tSize: " + str(size) | |
test_msg += "\n\tFiles/sec: " + str(files_sec) | |
test_msg += "\n\tApp overhead: " + str(app_overhead) + "\n\t" | |
test_msg += '\n\t'.join(( | |
f"F_Use%: {f_use}", | |
f"Count: {count}", | |
f"Size: {size}", | |
f"Files/sec: {files_sec}", | |
f"App overhead: {app_overhead}\n\t" | |
)) |
first_loop_done = False | ||
|
||
NAME = r".+?# (?P<name>(/bin/)?fs_mark.+?)\r+\n" | ||
MSG_LINE = r"(?P<line>(\s+\d+){3}.+?)\r+\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify whether there have been any issues with regex on any specific target? From my current testing on ia32-generic-qemu, I've managed to capture regex results into groups smoothly, both under no load and under stress. However, I haven't tested this extensively.
My regex:
TYPES = [
"f_use",
"count",
"size",
"files_sec",
"app_overhead",
"creatMin",
"creatAbg",
"creatMax",
"writeMin",
"writeAbg",
"writeMax",
"fsyncMin",
"fsyncAbg",
"fsyncMax",
"syncMin",
"syncAbg",
"syncMax",
"closeMin",
"closeAbg",
"closeMax",
"unlinkMin",
"unlinkAbg",
"unlinkMax",
]
MSG_LINE = "".join([rf"(?P<{type}>-?\d+\.?\d*)\s+" for type in TYPES]) + r"\r?\n"
Code on which that was tested:
while True:
if loop_start and loop_end:
loop_time = 3 * (loop_end - loop_start)
loop_start = None
loop_end = None
idx = dut.expect([NAME, MSG_LINE, NO_SPC, END, NO_CONT_BLOCK, ERR], timeout=60)
parsed = dut.match.groupdict()
print(parsed)
if idx == 0:
test_name = parsed["name"]
loop_start = time.time()
elif idx == 1:
pass
Output:
�[0J(psh)% /bin/fs_mark -d /fs_mark_test -s 0 -n 10 -v -S 0 -L 10
# /bin/fs_mark -d /fs_mark_test -s 0 -n 10 -v -S 0 -L 10
{'name': '/bin/fs_mark -d /fs_mark_test -s 0 -n 10 -v -S 0 -L 10 '}
# Version 3.3, 1 thread(s) starting at Thu Jan 1 00:00:08 1970
# Sync method: NO SYNC: Test does not issue sync() or fsync() calls.
# Directories: no subdirectories used
# File names: 40 bytes long, (16 initial bytes of time stamp with 24 random bytes at end of name)
# Files info: size 0 bytes, written with an IO size of 16384 bytes per write
# App overhead is time in microseconds spent in the test not doing file writing related system calls.
# All system call times are reported in microseconds.
FSUse% Count Size Files/sec App Overhead CREAT (Min/Avg/Max) WRITE (Min/Avg/Max) FSYNC (Min/Avg/Max) SYNC (Min/Avg/Max) CLOSE (Min/Avg/Max) UNLINK (Min/Avg/Max)
27 10 0 1.7 181866 358108 547925 712653 1601 3120 6763 0 0 0 0 0 0 1404 3829 15385 0 0 0
{'f_use': '27', 'count': '10', 'size': '0', 'files_sec': '1.7', 'app_overhead': '181866', 'creatMin': '358108', 'creatAbg': '547925', 'creatMax': '712653', 'writeMin': '1601', 'writeAbg': '3120', 'writeMax': '6763', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1404', 'closeAbg': '3829', 'closeMax': '15385', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
27 20 0 1.6 237227 419356 590063 842584 1844 3814 7751 0 0 0 0 0 0 1814 4079 9268 0 0 0
27 30 0 1.4 213842 451503 674516 982013 1746 5525 15782 0 0 0 0 0 0 1719 5564 20754 0 0 0
27 40 0 1.5 172467 517018 643414 737076 2138 3036 5709 0 0 0 0 0 0 1981 5020 10241 0 0 0
27 50 0 1.7 156668 501367 577613 700207 2071 3411 6560 0 0 0 0 0 0 1709 2290 3061 0 0 0
28 60 0 1.5 216290 546834 630984 836860 1656 3969 10902 0 0 0 0 0 0 1937 3399 6637 0 0 0
28 70 0 1.5 192122 431973 632832 793497 1250 3366 12533 0 0 0 0 0 0 1438 2688 5080 0 0 0
28 80 0 1.6 139242 479825 622701 775921 1730 4523 10685 0 0 0 0 0 0 1837 3346 9047 0 0 0
{'f_use': '27', 'count': '20', 'size': '0', 'files_sec': '1.6', 'app_overhead': '237227', 'creatMin': '419356', 'creatAbg': '590063', 'creatMax': '842584', 'writeMin': '1844', 'writeAbg': '3814', 'writeMax': '7751', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1814', 'closeAbg': '4079', 'closeMax': '9268', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
{'f_use': '27', 'count': '30', 'size': '0', 'files_sec': '1.4', 'app_overhead': '213842', 'creatMin': '451503', 'creatAbg': '674516', 'creatMax': '982013', 'writeMin': '1746', 'writeAbg': '5525', 'writeMax': '15782', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1719', 'closeAbg': '5564', 'closeMax': '20754', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
{'f_use': '27', 'count': '40', 'size': '0', 'files_sec': '1.5', 'app_overhead': '172467', 'creatMin': '517018', 'creatAbg': '643414', 'creatMax': '737076', 'writeMin': '2138', 'writeAbg': '3036', 'writeMax': '5709', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1981', 'closeAbg': '5020', 'closeMax': '10241', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
{'f_use': '27', 'count': '50', 'size': '0', 'files_sec': '1.7', 'app_overhead': '156668', 'creatMin': '501367', 'creatAbg': '577613', 'creatMax': '700207', 'writeMin': '2071', 'writeAbg': '3411', 'writeMax': '6560', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1709', 'closeAbg': '2290', 'closeMax': '3061', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
{'f_use': '28', 'count': '60', 'size': '0', 'files_sec': '1.5', 'app_overhead': '216290', 'creatMin': '546834', 'creatAbg': '630984', 'creatMax': '836860', 'writeMin': '1656', 'writeAbg': '3969', 'writeMax': '10902', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1937', 'closeAbg': '3399', 'closeMax': '6637', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
{'f_use': '28', 'count': '70', 'size': '0', 'files_sec': '1.5', 'app_overhead': '192122', 'creatMin': '431973', 'creatAbg': '632832', 'creatMax': '793497', 'writeMin': '1250', 'writeAbg': '3366', 'writeMax': '12533', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1438', 'closeAbg': '2688', 'closeMax': '5080', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
{'f_use': '28', 'count': '80', 'size': '0', 'files_sec': '1.6', 'app_overhead': '139242', 'creatMin': '479825', 'creatAbg': '622701', 'creatMax': '775921', 'writeMin': '1730', 'writeAbg': '4523', 'writeMax': '10685', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1837', 'closeAbg': '3346', 'closeMax': '9047', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
28 90 0 1.5 225379 418652 633652 1053823 1616 3947 18033 0 0 0 0 0 0 2179 4394 10715 0 0 0
{'f_use': '28', 'count': '90', 'size': '0', 'files_sec': '1.5', 'app_overhead': '225379', 'creatMin': '418652', 'creatAbg': '633652', 'creatMax': '1053823', 'writeMin': '1616', 'writeAbg': '3947', 'writeMax': '18033', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '2179', 'closeAbg': '4394', 'closeMax': '10715', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
28 100 0 1.2 226891 477213 774066 1200110 1676 3605 8894 0 0 0 0 0 0 1573 3729 10540 0 0 0
{'f_use': '28', 'count': '100', 'size': '0', 'files_sec': '1.2', 'app_overhead': '226891', 'creatMin': '477213', 'creatAbg': '774066', 'creatMax': '1200110', 'writeMin': '1676', 'writeAbg': '3605', 'writeMax': '8894', 'fsyncMin': '0', 'fsyncAbg': '0', 'fsyncMax': '0', 'syncMin': '0', 'syncAbg': '0', 'syncMax': '0', 'closeMin': '1573', 'closeAbg': '3729', 'closeMax': '10540', 'unlinkMin': '0', 'unlinkAbg': '0', 'unlinkMax': '0'}
Average Files/sec: 1.0
{}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The python harness is being run on host - I assume You've been testing the target ia32-generic-qemu
on your laptop - which is more capable than GA runner (either from GitHub or our RPi).
Regex expressions are hard on CPU (if you don't construct them carefully you could even have O(2^n) complexity!) - so a rule of thumb is to avoid them if they're not necessary (and better use simple ones when not). Please note that intensive regex matching might impact the test total time being measured (not much, but still).
Current approach with splitting the line is better CPU-wise, could probably still be optimized in regards to python syntax, eg:
TYPES = [ "f_use", "count", "size", "files_sec", "app_overhead", "creatMin", "creatAbg", "creatMax", "writeMin", "writeAbg", "writeMax", "fsyncMin", "fsyncAbg", "fsyncMax", "syncMin", "syncAbg", "syncMax", "closeMin", "closeAbg", "closeMax", "unlinkMin", "unlinkAbg", "unlinkMax", ]
# NOTE: used `float` as files_sec is a float number, doesn't really harm other stats
line_dict = dict(zip(TYPES, map(float, line.split())))
# if you need partial-only line_dict - use list splitting, eg:
line_dict = dict(zip(TYPES[20:22], map(float, line.split()[20:22])))
JIRA: CI-307
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment