[v2,17/26] selftests/resctrl: Replace file write with volatile variable
Commit Message
The fill_buf code prevents compiler optimizating the entire read loop
away by writing the final value of the variable into a file. While it
achieves the goal, writing into a file requires significant amount of
work within the innermost test loop and also error handling.
A simpler approach is to take advantage of volatile. Writing to a
variable through a volatile pointer is enough to prevent compiler from
optimizing the write away, and therefore compiler cannot remove the
read loop either.
Add a volatile 'value_sink' into resctrl_tests.c and make fill_buf to
write into it. As a result, the error handling in fill_buf.c can be
simplified.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
tools/testing/selftests/resctrl/fill_buf.c | 26 ++++---------------
tools/testing/selftests/resctrl/resctrl.h | 7 +++++
.../testing/selftests/resctrl/resctrl_tests.c | 4 +++
3 files changed, 16 insertions(+), 21 deletions(-)
Comments
Hi Ilpo,
On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
> The fill_buf code prevents compiler optimizating the entire read loop
> away by writing the final value of the variable into a file. While it
> achieves the goal, writing into a file requires significant amount of
> work within the innermost test loop and also error handling.
>
> A simpler approach is to take advantage of volatile. Writing to a
> variable through a volatile pointer is enough to prevent compiler from
> optimizing the write away, and therefore compiler cannot remove the
> read loop either.
>
> Add a volatile 'value_sink' into resctrl_tests.c and make fill_buf to
> write into it. As a result, the error handling in fill_buf.c can be
> simplified.
>
The subject and changelog describes the need for a volatile variable.
The patch introduces two volatile variables. Could you please elaborate
why two volatile variables are needed?
Reinette
On Tue, 28 Nov 2023, Reinette Chatre wrote:
> On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
> > The fill_buf code prevents compiler optimizating the entire read loop
> > away by writing the final value of the variable into a file. While it
> > achieves the goal, writing into a file requires significant amount of
> > work within the innermost test loop and also error handling.
> >
> > A simpler approach is to take advantage of volatile. Writing to a
> > variable through a volatile pointer is enough to prevent compiler from
> > optimizing the write away, and therefore compiler cannot remove the
> > read loop either.
> >
> > Add a volatile 'value_sink' into resctrl_tests.c and make fill_buf to
> > write into it. As a result, the error handling in fill_buf.c can be
> > simplified.
> >
>
> The subject and changelog describes the need for a volatile variable.
> The patch introduces two volatile variables. Could you please elaborate
> why two volatile variables are needed?
Well, the other "volatile variable" is a pointer to a volatile variable.
I've seen gcc to kill a static volatile int so I prefer to not take
change with its optimizer. Thus, I placed the sink into different
compilation unit and just present a pointer to the actual "volatile"
variable.
I guess the sink could be marked as plain int instead but this being
trickery to begin with I don't see much value either way. It's still
a trick.
I'll alter the changelog's wording though, "a volatile variable" is
not accurate as it's "a pointer to a volatile variable".
@@ -78,10 +78,9 @@ static void fill_one_span_write(unsigned char *buf, size_t buf_size)
}
}
-static int fill_cache_read(unsigned char *buf, size_t buf_size, bool once)
+static void fill_cache_read(unsigned char *buf, size_t buf_size, bool once)
{
int ret = 0;
- FILE *fp;
while (1) {
ret = fill_one_span_read(buf, buf_size);
@@ -90,26 +89,16 @@ static int fill_cache_read(unsigned char *buf, size_t buf_size, bool once)
}
/* Consume read result so that reading memory is not optimized out. */
- fp = fopen("/dev/null", "w");
- if (!fp) {
- perror("Unable to write to /dev/null");
- return -1;
- }
- fprintf(fp, "Sum: %d ", ret);
- fclose(fp);
-
- return 0;
+ *value_sink = ret;
}
-static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once)
+static void fill_cache_write(unsigned char *buf, size_t buf_size, bool once)
{
while (1) {
fill_one_span_write(buf, buf_size);
if (once)
break;
}
-
- return 0;
}
static unsigned char *alloc_buffer(size_t buf_size, int memflush)
@@ -143,21 +132,16 @@ static unsigned char *alloc_buffer(size_t buf_size, int memflush)
int run_fill_buf(size_t buf_size, int memflush, int op, bool once)
{
unsigned char *buf;
- int ret;
buf = alloc_buffer(buf_size, memflush);
if (!buf)
return -1;
if (op == 0)
- ret = fill_cache_read(buf, buf_size, once);
+ fill_cache_read(buf, buf_size, once);
else
- ret = fill_cache_write(buf, buf_size, once);
+ fill_cache_write(buf, buf_size, once);
free(buf);
- if (ret) {
- printf("\n Error in fill cache\n");
- return -1;
- }
return 0;
}
@@ -79,6 +79,13 @@ struct perf_event_read {
#define CMT_STR "cmt"
#define CAT_STR "cat"
+/*
+ * Memory location that consumes values compiler must not optimize away.
+ * Volatile ensures writes to this location cannot be optimized away by
+ * compiler.
+ */
+extern volatile int *value_sink;
+
extern pid_t bm_pid, ppid;
extern char llc_occup_path[1024];
@@ -10,6 +10,10 @@
*/
#include "resctrl.h"
+/* Volatile memory sink to prevent compiler optimizations */
+static volatile int sink_target;
+volatile int *value_sink = &sink_target;
+
static int detect_vendor(void)
{
FILE *inf = fopen("/proc/cpuinfo", "r");