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

Check for the unutilized credit and use them before the pacer period and within the threshold limit #37

Open
wants to merge 1 commit into
base: tuner
Choose a base branch
from

Conversation

gobinathsr
Copy link

Effective checking on the available credit

Root Cause:
As of now credit availability is checked only after the pacer period expiry. Hence in case if there is left our credit available will not be consumed till the next pacer period. This will cause unnecessary holding of the request till the next pacer period.

Fix info:
Added the Function is_credit_available() to check the credit available before expiry of the pacer period.
This is to consume available credit if not consumed during the previous pacer period based on the threshold limit.

(1) Added below macros to configure the credit checking
/*

  • Configures credit availability to be calculated along with the
  • bytes in flight within in the allowed margin offset
    */
   #define SPDK_NVMF_RDMA_IO_PACER_CHECK_CREDITS_ONLY_AT_PACER_PERIOD  1
   #define SPDK_NVMF_RDMA_IO_PACER_ALLOW_WITHIN_CREDIT_ONLY            1
   #define SPDK_NVMF_RDMA_IO_PACER_DEFAULT_MARGIN_ABOVE_CREDIT         1

(2) Added the below macros to configure the tuner instead of hardcoded values:

   #define SPDK_NVMF_RDMA_IO_PACER_TUNER_TYPE_01  01
   #define SPDK_NVMF_RDMA_IO_PACER_TUNER_TYPE_02  02

[Features/Functional areas and/or component interfaces affected]
Request submission within the pacer period

[Whether the change affects how hardware is programmed]
Yes - Will not hold the request if the credit available within the pacer period.

Local testing
Performed the testing with SPDK_NVMF_RDMA_IO_PACER_CHECK_CREDITS_ONLY_AT_PACER_PERIOD turned to '0' and below are the results.

   gobinaths@clx-ssp-025 io_pacing]$ SETUP=5 ./test.sh test_14
   test_14
   CPU mask , num cores 4, IO pacer period 5600, adjusted period 22400
   ./test.sh: line 339: 43714 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 177.9      |  191.0      | 189.3288   | 3017.4          | 7.6        | 99.3            | 32.6 (4.0)           | 22.9
   | 2048       | 171.5      |  190.9      | 157.3464   | 25042.2         | 8.3        | 97.0            | 36.0 (4.5)           | 23.2
   CPU mask , num cores 4, IO pacer period 5650, adjusted period 22600
   ./test.sh: line 339: 44445 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 179.6      |  190.3      | 146.3115   | 2989.0          | 6.0        | 99.4            | 37.0 (4.6)           | 22.9
   | 2048       | 169.3      |  190.1      | 135.8801   | 25358.7         | 8.5        | 97.1            | 65.0 (8.1)           | 23.2
   CPU mask , num cores 4, IO pacer period 5700, adjusted period 22800
   ./test.sh: line 339: 45177 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 177.9      |  189.5      | 186.3718   | 3016.9          | 6.0        | 98.8            | 32.0 (4.0)           | 23.2
   | 2048       | 175.6      |  189.5      | 186.3559   | 24469.5         | 5.3        | 98.7            | 53.3 (6.6)           | 23.4
   CPU mask , num cores 4, IO pacer period 5750, adjusted period 23000
   ./test.sh: line 339: 45909 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 170.3      |  188.8      | 184.2591   | 3152.5          | 8.2        | 98.9            | 37.0 (4.6)           | 23.5
   | 2048       | 176.5      |  188.6      | 184.7163   | 24332.3         | 5.3        | 99.0            | 39.3 (4.9)           | 23.6
   CPU mask , num cores 4, IO pacer period 5800, adjusted period 23200
   ./test.sh: line 339: 47221 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 164.5      |  187.9      | 145.8121   | 3262.9          | 8.3        | 99.2            | 30.3 (3.7)           | 23.8
   | 2048       | 176.4      |  187.9      | 183.0275   | 24350.6         | 4.3        | 99.1            | 55.6 (6.9)           | 23.8
   CPU mask , num cores 4, IO pacer period 6000, adjusted period 24000
   ./test.sh: line 339: 48341 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 163.5      |  184.8      | 154.4534   | 3282.1          | 7.2        | 99.4            | 59.0 (7.3)           | 24.5
   | 2048       | 170.2      |  194.7      | 175.3624   | 25237.4         | 5.6        | 99.1            | 41.0 (5.1)           | 24.9

Effective checking on the available credit

Root Cause:
As of now credit availability is checked only after the pacer period expiry. Hence in case if there is left our credit available will not be consumed till the next pacer period. This will cause unnecessary holding of the request till the next pacer period.

Fix info:
Added the Function is_credit_available() to check the credit available before expiry of the pacer period.
This is to consume available credit if not consumed during the previous pacer period based on the threshold limit.

(1) Added below macros to configure the credit checking
/*
 * Configures credit availability to be calculated along with the
  * bytes in flight within in the allowed margin offset
   */
   #define SPDK_NVMF_RDMA_IO_PACER_CHECK_CREDITS_ONLY_AT_PACER_PERIOD  1
   #define SPDK_NVMF_RDMA_IO_PACER_ALLOW_WITHIN_CREDIT_ONLY            1
   #define SPDK_NVMF_RDMA_IO_PACER_DEFAULT_MARGIN_ABOVE_CREDIT         1

   (2) Added the below macros to configure the tuner instead of hardcoded values:
   #define SPDK_NVMF_RDMA_IO_PACER_TUNER_TYPE_01  01
   #define SPDK_NVMF_RDMA_IO_PACER_TUNER_TYPE_02  02

   [Features/Functional areas and/or component interfaces affected]
   Request submission within the pacer period

   [Whether the change affects how hardware is programmed]
   Yes - Will not hold the request if the credit available within the pacer period.

   Local testing
   Performed the testing with SPDK_NVMF_RDMA_IO_PACER_CHECK_CREDITS_ONLY_AT_PACER_PERIOD turned to '0' and below are the results.
   gobinaths@clx-ssp-025 io_pacing]$ SETUP=5 ./test.sh test_14
   test_14
   CPU mask , num cores 4, IO pacer period 5600, adjusted period 22400
   ./test.sh: line 339: 43714 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 177.9      |  191.0      | 189.3288   | 3017.4          | 7.6        | 99.3            | 32.6 (4.0)           | 22.9
   | 2048       | 171.5      |  190.9      | 157.3464   | 25042.2         | 8.3        | 97.0            | 36.0 (4.5)           | 23.2
   CPU mask , num cores 4, IO pacer period 5650, adjusted period 22600
   ./test.sh: line 339: 44445 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 179.6      |  190.3      | 146.3115   | 2989.0          | 6.0        | 99.4            | 37.0 (4.6)           | 22.9
   | 2048       | 169.3      |  190.1      | 135.8801   | 25358.7         | 8.5        | 97.1            | 65.0 (8.1)           | 23.2
   CPU mask , num cores 4, IO pacer period 5700, adjusted period 22800
   ./test.sh: line 339: 45177 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 177.9      |  189.5      | 186.3718   | 3016.9          | 6.0        | 98.8            | 32.0 (4.0)           | 23.2
   | 2048       | 175.6      |  189.5      | 186.3559   | 24469.5         | 5.3        | 98.7            | 53.3 (6.6)           | 23.4
   CPU mask , num cores 4, IO pacer period 5750, adjusted period 23000
   ./test.sh: line 339: 45909 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 170.3      |  188.8      | 184.2591   | 3152.5          | 8.2        | 98.9            | 37.0 (4.6)           | 23.5
   | 2048       | 176.5      |  188.6      | 184.7163   | 24332.3         | 5.3        | 99.0            | 39.3 (4.9)           | 23.6
   CPU mask , num cores 4, IO pacer period 5800, adjusted period 23200
   ./test.sh: line 339: 47221 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 164.5      |  187.9      | 145.8121   | 3262.9          | 8.3        | 99.2            | 30.3 (3.7)           | 23.8
   | 2048       | 176.4      |  187.9      | 183.0275   | 24350.6         | 4.3        | 99.1            | 55.6 (6.9)           | 23.8
   CPU mask , num cores 4, IO pacer period 6000, adjusted period 24000
   ./test.sh: line 339: 48341 Terminated              tail -f > rpc_pipe
   | QD         | BW         |  BW Max     | WIRE BW    | AVG LAT, us     | BW STDDEV  | L3 Hit Rate     | Bufs in-flight (MiB) | Pacer period, us
   | 256        | 163.5      |  184.8      | 154.4534   | 3282.1          | 7.2        | 99.4            | 59.0 (7.3)           | 24.5
   | 2048       | 170.2      |  194.7      | 175.3624   | 25237.4         | 5.6        | 99.1            | 41.0 (5.1)           | 24.9
@gobinathsr gobinathsr requested a review from nv-shri February 17, 2021 05:04
@swx-snap
Copy link

Can one of the admins verify this patch?

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.

3 participants