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.

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

block/035: Test shared queue fairness #135

Merged
merged 1 commit into from Apr 9, 2024
Merged

Conversation

bvanassche
Copy link
Contributor

No description provided.

@bvanassche
Copy link
Contributor Author

@kawasaki Do you get notified automatically if a pull request is created or do we perhaps have to ping you?

@kawasaki
Copy link
Collaborator

@kawasaki Do you get notified automatically if a pull request is created or do we perhaps have to ping you?

@bvanassche Sorry, I missed this pull request in last four days since I was busy on zbd/010 and nbd/002 failures. I made it my daily routine to check my GitHub notification page, but this is the "pull" style then I tend to miss it. I have set up GitHub notification to come to my e-mail reader so that I can receive the notifications in "push" style. I hope it will reduce the chance to miss.

Will look into your pull requests today. Thanks.

@kawasaki
Copy link
Collaborator

I noticed that this PR adds the test case to confirm no regression by the patch proposed to linux-block.

Copy link
Collaborator

@kawasaki kawasaki left a comment

Choose a reason for hiding this comment

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

This test case looks interesting and valuable. Please find my comments. Also, could you add a brief comment in the commit message? Thanks.

tests/block/035 Outdated Show resolved Hide resolved
tests/block/035 Show resolved Hide resolved
tests/block/035 Outdated Show resolved Hide resolved
tests/block/035 Outdated Show resolved Hide resolved
tests/block/035 Outdated Show resolved Hide resolved
tests/block/035 Show resolved Hide resolved
tests/block/035 Outdated Show resolved Hide resolved
@kawasaki
Copy link
Collaborator

@bvanassche Thank you for updating the commit. I did trial runs and results look good. I would like to clarify one point before merge. Please find the question I made.

Test whether both requests queues process I/O if the tag set is shared
and if the completion times of the two request queues differ
significantly.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
@bvanassche bvanassche requested a review from kawasaki April 8, 2024 21:53
@kawasaki kawasaki merged commit f073dac into osandov:master Apr 9, 2024
5 checks passed
@kawasaki
Copy link
Collaborator

kawasaki commented Apr 9, 2024

Thank you @bvanassche for the update. This PR has got merged.

After that, I came up with an idea to print the IOPS numbers so that we can check them easier.

diff --git a/tests/block/035 b/tests/block/035
index 7669633..f9267dd 100755
--- a/tests/block/035
+++ b/tests/block/035
@@ -73,6 +73,8 @@ test() {
        local iops1 iops2 rest
        read -r iops1 iops2 rest \
             <<<"$(sed -n 's/.*IOPS=\([0-9]*\).*/\1/p' <"${fio_output}" | xargs)"
+       TEST_RUN["  1 ms completion IOPS"]=$iops1
+       TEST_RUN["100 ms completion IOPS"]=$iops2
        if [ -z "$iops1" ] || [ -z "$iops2" ] ||
                        [ "$iops1" -lt "$iops2" ]; then
                echo "Error: IOPS too low ($iops1; $iops2)"

The two lines change will makes the blktests run output as follows:

block/035 (shared tag set fairness)                          [passed]
        100 ms completion IOPS  318      ...  318
          1 ms completion IOPS  27000    ...  27194
        runtime                 31.076s  ...  31.063s

I may post this patch later when I have time to afford.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants