Hi Rory,
On 18/5/2020 11:39 AM, Sexton, Rory wrote:
> This modification allows for the branch ratio threshold to be set
> per core rather than system wide. This gives greater flexibility to
> the branch ration monitoring allowing it to manage different
> workloads with different characteristics on the same system.
>
> Signed-off-by: Rory Sexton <rory.sexton@intel.com>
> ---
> examples/vm_power_manager/main.c | 40 +++++++++++---------
> examples/vm_power_manager/oob_monitor_x86.c | 2 +-
> examples/vm_power_manager/parse.c | 42 ++++++++++++++++++++-
> examples/vm_power_manager/parse.h | 3 ++
> examples/vm_power_manager/power_manager.c | 2 +-
> examples/vm_power_manager/power_manager.h | 2 +-
> 6 files changed, 68 insertions(+), 23 deletions(-)
This change is a good enhancement, as there are usually different types
of workload running on the cores in a system, each requiring a different
ratio.
Also, it's good that multiple groups of cores can be specified by using
the parameter multiple times, e.g. -b 21-23:0.5 -b 24,25:0.4
LGTM.
Reviewed-by: David Hunt <david.hunt@intel.com>
@@ -165,15 +165,14 @@ parse_args(int argc, char **argv)
static struct option lgopts[] = {
{ "mac-updating", no_argument, 0, 1},
{ "no-mac-updating", no_argument, 0, 0},
- { "core-list", optional_argument, 0, 'l'},
+ { "core-branch-ratio", optional_argument, 0, 'b'},
{ "port-list", optional_argument, 0, 'p'},
- { "branch-ratio", optional_argument, 0, 'b'},
{NULL, 0, 0, 0}
};
argvopt = argv;
ci = get_core_info();
- while ((opt = getopt_long(argc, argvopt, "l:p:q:T:b:",
+ while ((opt = getopt_long(argc, argvopt, "p:q:T:b:",
lgopts, &option_index)) != EOF) {
switch (opt) {
@@ -185,7 +184,8 @@ parse_args(int argc, char **argv)
return -1;
}
break;
- case 'l':
+ case 'b':
+ branch_ratio = BRANCH_RATIO_THRESHOLD;
oob_enable = malloc(ci->core_count * sizeof(uint16_t));
if (oob_enable == NULL) {
printf("Error - Unable to allocate memory\n");
@@ -193,32 +193,37 @@ parse_args(int argc, char **argv)
}
cnt = parse_set(optarg, oob_enable, ci->core_count);
if (cnt < 0) {
- printf("Invalid core-list - [%s]\n",
+ printf("Invalid core-list section in "
+ "core-branch-ratio matrix - [%s]\n",
+ optarg);
+ free(oob_enable);
+ break;
+ }
+ cnt = parse_branch_ratio(optarg, &branch_ratio);
+ if (cnt < 0) {
+ printf("Invalid branch-ratio section in "
+ "core-branch-ratio matrix - [%s]\n",
optarg);
free(oob_enable);
break;
}
+ if (branch_ratio <= 0.0) {
+ printf("invalid branch ratio specified\n");
+ return -1;
+ }
for (i = 0; i < ci->core_count; i++) {
if (oob_enable[i]) {
- printf("***Using core %d\n", i);
+ printf("***Using core %d "
+ "with branch ratio %f\n",
+ i, branch_ratio);
ci->cd[i].oob_enabled = 1;
ci->cd[i].global_enabled_cpus = 1;
+ ci->cd[i].branch_ratio_threshold =
+ branch_ratio;
}
}
free(oob_enable);
break;
- case 'b':
- branch_ratio = 0.0;
- if (strlen(optarg))
- branch_ratio = atof(optarg);
- if (branch_ratio <= 0.0) {
- printf("invalid branch ratio specified\n");
- return -1;
- }
- ci->branch_ratio_threshold = branch_ratio;
- printf("***Setting branch ratio to %f\n",
- branch_ratio);
- break;
/* long options */
case 0:
break;
@@ -109,7 +109,7 @@ apply_policy(int core)
* down. Each core_details struct has it's own array.
*/
freq_window_idx = ci->cd[core].freq_window_idx;
- if (ratio > ci->branch_ratio_threshold)
+ if (ratio > ci->cd[core].branch_ratio_threshold)
ci->cd[core].freq_directions[freq_window_idx] = 1;
else
ci->cd[core].freq_directions[freq_window_idx] = 0;
@@ -60,7 +60,7 @@ parse_set(const char *input, uint16_t set[], unsigned int num)
min = idx;
else /* avoid continuous '-' */
return -1;
- } else if ((*end == ',') || (*end == '\0')) {
+ } else if ((*end == ',') || (*end == ':') || (*end == '\0')) {
max = idx;
if (min == num)
@@ -75,7 +75,45 @@ parse_set(const char *input, uint16_t set[], unsigned int num)
return -1;
str = end + 1;
- } while (*end != '\0');
+ } while ((*end != '\0') && (*end != ':'));
+
+ return str - input;
+}
+
+int
+parse_branch_ratio(const char *input, float *branch_ratio)
+{
+ const char *str = input;
+ char *end = NULL;
+
+ while (isblank(*str))
+ str++;
+
+ if (*str == '\0')
+ return -1;
+
+ /* Go straight to the ':' separator if present */
+ while ((*str != '\0') && (*str != ':'))
+ str++;
+
+ /* Branch ratio not specified in args so leave it at default setting */
+ if (*str == '\0')
+ return 0;
+
+ /* Confirm ':' separator present */
+ if (*str != ':')
+ return -1;
+
+ str++;
+ errno = 0;
+ *branch_ratio = strtof(str, &end);
+ if (errno || end == NULL)
+ return -1;
+
+ if (*end != '\0')
+ return -1;
+
+ str = end + 1;
return str - input;
}
@@ -12,6 +12,9 @@ extern "C" {
int
parse_set(const char *, uint16_t [], unsigned int);
+int
+parse_branch_ratio(const char *input, float *branch_ratio);
+
#ifdef __cplusplus
}
#endif
@@ -60,7 +60,6 @@ core_info_init(void)
ci = get_core_info();
ci->core_count = get_nprocs_conf();
- ci->branch_ratio_threshold = BRANCH_RATIO_THRESHOLD;
ci->cd = malloc(ci->core_count * sizeof(struct core_details));
memset(ci->cd, 0, ci->core_count * sizeof(struct core_details));
if (!ci->cd) {
@@ -69,6 +68,7 @@ core_info_init(void)
}
for (i = 0; i < ci->core_count; i++) {
ci->cd[i].global_enabled_cpus = 1;
+ ci->cd[i].branch_ratio_threshold = BRANCH_RATIO_THRESHOLD;
}
printf("%d cores in system\n", ci->core_count);
return 0;
@@ -26,12 +26,12 @@ struct core_details {
uint16_t freq_directions[FREQ_WINDOW_SIZE];
uint16_t freq_window_idx;
uint16_t freq_state;
+ float branch_ratio_threshold;
};
struct core_info {
uint16_t core_count;
struct core_details *cd;
- float branch_ratio_threshold;
};
#define BRANCH_RATIO_THRESHOLD 0.1