nvme/plugins: Add performance stats v2 #2176
Conversation
jinhuahuang2021
force-pushed
the
feature/nvme-performance-stats-v2-1812
branch
16 times, most recently
from
6a3deef to
69c67d5
Compare
Add performance stats v2 Signed-off-by: jinhua.huang <jinhua.huang@memblaze.com>
jinhuahuang2021
force-pushed
the
feature/nvme-performance-stats-v2-1812
branch
from
69c67d5 to
f543444
Compare
| @@ -107,7 +107,8 @@ static __u64 raw_2_u64(const __u8 *buf, size_t len) | |||
| return le64_to_cpu(val); | |||
| } | |||
|
|
|||
| static void get_memblaze_new_smart_info(struct nvme_p4_smart_log *smart, int index, __u8 *nm_val, __u8 *raw_val) | |||
| static void get_memblaze_new_smart_info(struct nvme_p4_smart_log *smart, int index, __u8 *nm_val, | |||
| __u8 *raw_val) | |||
There was a problem hiding this comment.
I'd prefer if you could split out white space changes into a separate patch. Mixing janitor updates and new code is making the review harder that it needs to be.
Anyway, when shortening overlong lines, please align the arguments or use two spaces as indent, e.g.
static void get_memblaze_new_smart_info(struct nvme_p4_smart_log *smart, int index, __u8 *nm_val,
__u8 *raw_val)or
static void get_memblaze_new_smart_info(struct nvme_p4_smart_log *smart, int index, __u8 *nm_val,
__u8 *raw_val)| @@ -144,35 +145,42 @@ static void show_memblaze_smart_log_new(struct nvme_memblaze_smart_log *s, | |||
| "min: ", *(__u16 *)raw, ", max: ", *(__u16 *)(raw+2), ", avg: ", *(__u16 *)(raw+4)); | |||
|
|
|||
| get_memblaze_new_smart_info(smart, RAISIN_SI_VD_E2E_DECTECTION_COUNT, nm, raw); | |||
| printf("%-31s: %3d%% %"PRIu64"\n", "end_to_end_error_detection_count", *nm, int48_to_long(raw)); | |||
| printf("%-31s: %3d%% %"PRIu64"\n", "end_to_end_error_detection_count", *nm, | |||
| int48_to_long(raw)); | |||
There was a problem hiding this comment.
Here the int48_to_long argument could align with the opening '('.
Obviously, this is also true for all cases below.
|
|
||
| get_memblaze_new_smart_info(smart, RAISIN_SI_VD_THERMAL_THROTTLE_STATUS, nm, raw); | ||
| printf("%-32s: %3d%% %u%%%s%"PRIu64"\n", "thermal_throttle_status", *nm, | ||
| *raw, ", cnt: ", int48_to_long(raw+1)); | ||
| *raw, ", cnt: ", int48_to_long(raw+1)); |
There was a problem hiding this comment.
This change is not needed as the line is already properly align.
| static int mb_get_additional_smart_log(int argc, char **argv, struct command *cmd, struct plugin *plugin) | ||
| static int mb_get_additional_smart_log( | ||
| int argc, char **argv, | ||
| struct command *cmd, struct plugin *plugin) |
There was a problem hiding this comment.
Indent these two lines with double tap, so the arguments do not align with the single tab indented variable declearations.
| { | ||
| struct nvme_memblaze_smart_log smart_log; | ||
| char *desc = | ||
| "Get Memblaze vendor specific additional smart log (optionally, for the specified namespace), and show it."; | ||
| char *desc = "Get Memblaze additional smart log, and show it."; |
There was a problem hiding this comment.
Overlong strings are okay to be one line. This is an exception but if the shortened help text is also fine. Either way you like it.
| @@ -667,7 +688,7 @@ struct log_page_high_latency { | |||
| static int find_deadbeef(char *buf) | |||
| { | |||
| if (((*(buf + 0) & 0xff) == 0xef) && ((*(buf + 1) & 0xff) == 0xbe) && | |||
| ((*(buf + 2) & 0xff) == 0xad) && ((*(buf + 3) & 0xff) == 0xde)) | |||
| ((*(buf + 2) & 0xff) == 0xad) && ((*(buf + 3) & 0xff) == 0xde)) | |||
|
|
||
| // Close device | ||
|
|
||
| dev_close(dev); |
There was a problem hiding this comment.
You could use the cleanup helper and then you could just remove the dev_close.
_cleanup_nvme_dev_ struct nvme_dev *dev = NULL;| struct command *cmd, | ||
| struct plugin *plugin) | ||
| { | ||
| // Get the configuration |
There was a problem hiding this comment.
Please avoid non meaningful comments. It's obvious from the code what is happening. This clutters just the code. Some for the rest of the comments in this function.
|
|
||
| static int mb_get_latency_stats(int argc, char **argv, | ||
| struct command *cmd, | ||
| struct plugin *plugin) |
| "%s.%03d", str_time_s, time_ms); | ||
| printf("%-24s", str_time_ms); | ||
|
|
||
| // |
|
The PR need more change, I will update it together. |
|
Do you still work on this? |
|
Yes, we are :) |
|
Okay, not sure what the update from yesterday means. FWIW, there shouldn't be any merges in the PR. Just rebase your changes to the latest upstream version and update the commit from there. Hope this helps. |
|
This PR contains various modifications, which makes it confusing. |
1 similar comment
|
This PR contains various modifications, which makes it confusing. |
Add performance stats v2