Hi Ilpo,
On 12/11/2023 4:18 AM, Ilpo Järvinen wrote:
> -int get_cbm_mask(char *cache_type, char *cbm_mask)
> +static int get_bit_mask(const char *filename, unsigned long *mask)
> {
> - char cbm_mask_path[1024];
> FILE *fp;
>
> - if (!cbm_mask)
> + if (!filename || !mask)
> return -1;
>
> - sprintf(cbm_mask_path, "%s/%s/cbm_mask", INFO_PATH, cache_type);
> -
> - fp = fopen(cbm_mask_path, "r");
> + fp = fopen(filename, "r");
> if (!fp) {
> - ksft_perror("Failed to open cache level");
> -
> + fprintf(stderr, "Failed to open bit mask file '%s': %s\n",
> + filename, strerror(errno));
> return -1;
> }
> - if (fscanf(fp, "%s", cbm_mask) <= 0) {
> - ksft_perror("Could not get max cbm_mask");
> +
> + if (fscanf(fp, "%lx", mask) <= 0) {
> + fprintf(stderr, "Could not read bit mask file '%s': %s\n",
> + filename, strerror(errno));
> fclose(fp);
>
> return -1;
After seeing the new effort to correct the perror() messages it is
not obvious to me why this patch changes these particular messages to
use fprintf(stderr, ...).
Reinette
On Wed, 13 Dec 2023, Reinette Chatre wrote:
> Hi Ilpo,
>
> On 12/11/2023 4:18 AM, Ilpo Järvinen wrote:
>
> > -int get_cbm_mask(char *cache_type, char *cbm_mask)
> > +static int get_bit_mask(const char *filename, unsigned long *mask)
> > {
> > - char cbm_mask_path[1024];
> > FILE *fp;
> >
> > - if (!cbm_mask)
> > + if (!filename || !mask)
> > return -1;
> >
> > - sprintf(cbm_mask_path, "%s/%s/cbm_mask", INFO_PATH, cache_type);
> > -
> > - fp = fopen(cbm_mask_path, "r");
> > + fp = fopen(filename, "r");
> > if (!fp) {
> > - ksft_perror("Failed to open cache level");
> > -
> > + fprintf(stderr, "Failed to open bit mask file '%s': %s\n",
> > + filename, strerror(errno));
> > return -1;
> > }
> > - if (fscanf(fp, "%s", cbm_mask) <= 0) {
> > - ksft_perror("Could not get max cbm_mask");
> > +
> > + if (fscanf(fp, "%lx", mask) <= 0) {
> > + fprintf(stderr, "Could not read bit mask file '%s': %s\n",
> > + filename, strerror(errno));
> > fclose(fp);
> >
> > return -1;
>
> After seeing the new effort to correct the perror() messages it is
> not obvious to me why this patch changes these particular messages to
> use fprintf(stderr, ...).
Yeah, good point, thanks. Somehow I dismissed the opportunity and thought
there's no need to do anything even if this came up during series conflict
resolution phase.
@@ -93,25 +93,20 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
int ret, pipefd[2], sibling_cpu_no;
unsigned long cache_size = 0;
unsigned long long_mask;
- char cbm_mask[256];
int count_of_bits;
char pipe_message;
size_t span;
- /* Get default cbm mask for L3/L2 cache */
- ret = get_cbm_mask(cache_type, cbm_mask);
+ ret = get_full_cbm(cache_type, &long_mask);
if (ret)
return ret;
- long_mask = strtoul(cbm_mask, NULL, 16);
-
/* Get L3/L2 cache size */
ret = get_cache_size(cpu_no, cache_type, &cache_size);
if (ret)
return ret;
ksft_print_msg("Cache size :%lu\n", cache_size);
- /* Get max number of bits from default-cabm mask */
count_of_bits = count_bits(long_mask);
if (!n)
@@ -75,17 +75,14 @@ int cmt_resctrl_val(int cpu_no, int n, const char * const *benchmark_cmd)
unsigned long cache_size = 0;
unsigned long long_mask;
char *span_str = NULL;
- char cbm_mask[256];
int count_of_bits;
size_t span;
int ret, i;
- ret = get_cbm_mask("L3", cbm_mask);
+ ret = get_full_cbm("L3", &long_mask);
if (ret)
return ret;
- long_mask = strtoul(cbm_mask, NULL, 16);
-
ret = get_cache_size(cpu_no, "L3", &cache_size);
if (ret)
return ret;
@@ -98,7 +98,7 @@ void tests_cleanup(void);
void mbm_test_cleanup(void);
int mba_schemata_change(int cpu_no, const char * const *benchmark_cmd);
void mba_test_cleanup(void);
-int get_cbm_mask(char *cache_type, char *cbm_mask);
+int get_full_cbm(const char *cache_type, unsigned long *mask);
int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
int signal_handler_register(void);
@@ -196,30 +196,29 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size)
#define CORE_SIBLINGS_PATH "/sys/bus/cpu/devices/cpu"
/*
- * get_cbm_mask - Get cbm mask for given cache
- * @cache_type: Cache level L2/L3
- * @cbm_mask: cbm_mask returned as a string
+ * get_bit_mask - Get bit mask from given file
+ * @filename: File containing the mask
+ * @mask: The bit mask returned as unsigned long
*
* Return: = 0 on success, < 0 on failure.
*/
-int get_cbm_mask(char *cache_type, char *cbm_mask)
+static int get_bit_mask(const char *filename, unsigned long *mask)
{
- char cbm_mask_path[1024];
FILE *fp;
- if (!cbm_mask)
+ if (!filename || !mask)
return -1;
- sprintf(cbm_mask_path, "%s/%s/cbm_mask", INFO_PATH, cache_type);
-
- fp = fopen(cbm_mask_path, "r");
+ fp = fopen(filename, "r");
if (!fp) {
- ksft_perror("Failed to open cache level");
-
+ fprintf(stderr, "Failed to open bit mask file '%s': %s\n",
+ filename, strerror(errno));
return -1;
}
- if (fscanf(fp, "%s", cbm_mask) <= 0) {
- ksft_perror("Could not get max cbm_mask");
+
+ if (fscanf(fp, "%lx", mask) <= 0) {
+ fprintf(stderr, "Could not read bit mask file '%s': %s\n",
+ filename, strerror(errno));
fclose(fp);
return -1;
@@ -229,6 +228,32 @@ int get_cbm_mask(char *cache_type, char *cbm_mask)
return 0;
}
+/*
+ * get_full_cbm - Get full Cache Bit Mask (CBM)
+ * @cache_type: Cache type as "L2" or "L3"
+ * @mask: Full cache bit mask representing the maximal portion of cache
+ * available for allocation, returned as unsigned long.
+ *
+ * Return: = 0 on success, < 0 on failure.
+ */
+int get_full_cbm(const char *cache_type, unsigned long *mask)
+{
+ char cbm_path[PATH_MAX];
+ int ret;
+
+ if (!cache_type)
+ return -1;
+
+ snprintf(cbm_path, sizeof(cbm_path), "%s/%s/cbm_mask",
+ INFO_PATH, cache_type);
+
+ ret = get_bit_mask(cbm_path, mask);
+ if (ret || !*mask)
+ return -1;
+
+ return 0;
+}
+
/*
* get_core_sibling - Get sibling core id from the same socket for given CPU
* @cpu_no: CPU number