public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: linux-kselftest@vger.kernel.org, "Shuah Khan" <shuah@kernel.org>,
	"Babu Moger" <babu.moger@amd.com>,
	"Maciej Wieczór-Retman" <maciej.wieczor-retman@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"Fenghua Yu" <fenghua.yu@intel.com>,
	"Shuah Khan" <skhan@linuxfoundation.org>
Subject: Re: [PATCH v4 02/16] selftests/resctrl: Calculate resctrl FS derived mem bw over sleep(1) only
Date: Fri, 24 May 2024 10:57:16 +0300 (EEST)	[thread overview]
Message-ID: <ea0c86b9-ae77-c2d9-b52b-239ae42603e8@linux.intel.com> (raw)
In-Reply-To: <04d0a5d6-82fa-4cc7-bd80-ee5cbd35f0c3@intel.com>

[-- Attachment #1: Type: text/plain, Size: 3038 bytes --]

On Thu, 23 May 2024, Reinette Chatre wrote:
> On 5/20/24 5:30 AM, Ilpo Järvinen wrote:
> > For MBM/MBA tests, measure_vals() calls get_mem_bw_imc() that performs
> > the measurement over a duration of sleep(1) call. The memory bandwidth
> > numbers from IMC are derived over this duration. The resctrl FS derived
> > memory bandwidth, however, is calculated inside measure_vals() and only
> > takes delta between the previous value and the current one which
> > besides the actual test, also samples inter-test noise.
> > 
> > Rework the logic in measure_vals() and get_mem_bw_imc() such that the
> > resctrl FS memory bandwidth section covers much shorter duration
> > closely matching that of the IMC perf counters to improve measurement
> > accuracy. Open two the resctrl mem bw files twice to avoid opening
> > after the test during measurement period (reading the same file twice
> > returns the same value so two files are needed).
> 
> I think this is only because of how the current reading is done, resctrl
> surely supports keeping a file open and reading from it multiple times.
> 
> There seems to be two things that prevent current code from doing this
> correctly:
> (a) the fscanf() code does not take into account that resctrl also
>     prints a "\n" ... (this seems to be the part that may cause the same
>     value to be returned).
>     So:
> 	if (fscanf(fp, "%lu", mbm_total) <= 0) {
>     should be:
> 	if (fscanf(fp, "%lu\n", mbm_total) <= 0) {
> (b) the current reading does not reset the file position so a second
>     read will attempt to read past the beginning. A "rewind(fp)"
>     should help here.

(b) cannot be the cause for returning the same value again. It would 
not be able to reread the number at all if file position is not moved.

I certainly tried with fseek() and it is when I got same value on the 
second read which is when I just went to two files solution.

> A small program like below worked for me by showing different values
> on every read:
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> 
> const char *mbm_total_path =
> "/sys/fs/resctrl/mon_data/mon_L3_00/mbm_total_bytes";
> 
> int main(void)
> {
> 	unsigned long mbm_total;
> 	FILE *fp;
> 	int count;
> 
> 	fp = fopen(mbm_total_path, "r");
> 	if (!fp) {
> 		perror("Opening data file\n");
> 		exit(1);
> 	}
> 	for (count = 0; count < 100; count++) {
> 		if (fscanf(fp, "%lu\n", &mbm_total) <= 0) {
> 			perror("Unable to read from data file\n");
> 			exit(1);
> 		}
> 		printf("Read %d: %lu\n",count ,mbm_total );
> 		sleep(1);
> 		rewind(fp);
> 	}
> 	fclose(fp);
> 	return 0;
> }

Okay, so perhaps it's your explanation (a) but can libc be trusted to not 
do buffering/caching for FILE *? So to be on the safe side, it would 
need to use syscalls directly to guarantee it's read the file twice.

If I convert it into fds, fscanf() cannot be used which would complicate 
the string processing by adding extra steps.

-- 
 i.

  reply	other threads:[~2024-05-24  7:57 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-20 12:30 [PATCH v4 00/16] selftests/resctrl: resctrl_val() related cleanups & improvements Ilpo Järvinen
2024-05-20 12:30 ` [PATCH v4 01/16] selftests/resctrl: Fix closing IMC fds on error and open-code R+W instead of loops Ilpo Järvinen
2024-05-29 17:42   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 02/16] selftests/resctrl: Calculate resctrl FS derived mem bw over sleep(1) only Ilpo Järvinen
2024-05-24  0:10   ` Reinette Chatre
2024-05-24  7:57     ` Ilpo Järvinen [this message]
2024-05-24 15:14       ` Reinette Chatre
2024-05-24 15:26         ` Ilpo Järvinen
2024-05-28 10:19           ` Ilpo Järvinen
2024-05-28 15:15             ` Reinette Chatre
2024-05-30 11:11               ` Ilpo Järvinen
2024-05-30 15:08                 ` Reinette Chatre
2024-05-31 12:51                   ` Ilpo Järvinen
2024-05-20 12:30 ` [PATCH v4 03/16] selftests/resctrl: Make "bandwidth" consistent in comments & prints Ilpo Järvinen
2024-05-29 17:43   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 04/16] selftests/resctrl: Consolidate get_domain_id() into resctrl_val() Ilpo Järvinen
2024-05-29 17:43   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 05/16] selftests/resctrl: Use correct type for pids Ilpo Järvinen
2024-05-29 17:44   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 06/16] selftests/resctrl: Cleanup bm_pid and ppid usage & limit scope Ilpo Järvinen
2024-05-29 17:44   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 07/16] selftests/resctrl: Rename measure_vals() to measure_mem_bw_vals() & document Ilpo Järvinen
2024-05-29 17:44   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 08/16] selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM tests Ilpo Järvinen
2024-05-29 17:45   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 09/16] selftests/resctrl: Add ->measure() callback to resctrl_val_param Ilpo Järvinen
2024-05-29 17:46   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 10/16] selftests/resctrl: Add ->init() callback into resctrl_val_param Ilpo Järvinen
2024-05-29 17:48   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 11/16] selftests/resctrl: Simplify bandwidth report type handling Ilpo Järvinen
2024-05-29 17:46   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 12/16] selftests/resctrl: Make some strings passed to resctrlfs functions const Ilpo Järvinen
2024-05-29 17:47   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 13/16] selftests/resctrl: Convert ctrlgrp & mongrp to pointers Ilpo Järvinen
2024-05-29 17:48   ` Reinette Chatre
2024-05-30 11:44     ` Ilpo Järvinen
2024-05-20 12:30 ` [PATCH v4 14/16] selftests/resctrl: Remove mongrp from MBA test Ilpo Järvinen
2024-05-29 17:49   ` Reinette Chatre
2024-05-30 11:56     ` Ilpo Järvinen
2024-05-30 15:09       ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 15/16] selftests/resctrl: Remove mongrp from CMT test Ilpo Järvinen
2024-05-29 17:49   ` Reinette Chatre
2024-05-20 12:30 ` [PATCH v4 16/16] selftests/resctrl: Remove test name comparing from write_bm_pid_to_resctrl() Ilpo Järvinen
2024-05-29 17:52   ` Reinette Chatre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ea0c86b9-ae77-c2d9-b52b-239ae42603e8@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=babu.moger@amd.com \
    --cc=fenghua.yu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=reinette.chatre@intel.com \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox