Hi,
On 6/7/2023 5:13 AM, Alexei Starovoitov wrote:
> On Mon, Jun 5, 2023 at 8:20 PM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>> +static void htab_mem_read_mem_cgrp_file(const char *name, unsigned long *value)
>> +{
>> + char buf[32];
>> + int fd;
>> +
>> + fd = openat(ctx.fd, name, O_RDONLY);
>> + if (fd < 0) {
>> + fprintf(stderr, "no %s\n", name);
>> + *value = 0;
>> + return;
>> + }
>> +
>> + buf[sizeof(buf) - 1] = 0;
>> + read(fd, buf, sizeof(buf) - 1);
> Please BPF CI. It's complaining about:
I don't know that RFC patch will go through BPF CI. Will check other
checks and tests in CI.
>
> benchs/bench_htab_mem.c: In function ‘htab_mem_read_mem_cgrp_file’:
> benchs/bench_htab_mem.c:290:2: error: ignoring return value of ‘read’,
> declared with attribute warn_unused_result [-Werror=unused-result]
> 290 | read(fd, buf, sizeof(buf) - 1);
Will fix in v5.
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
>> + *value = strtoull(buf, NULL, 0);
>> +
>> + close(fd);
>> +}
>> +
>> +static void htab_mem_measure(struct bench_res *res)
>> +{
>> + res->hits = atomic_swap(&ctx.skel->bss->loop_cnt, 0);
> This is missing:
> res->hits /= env.producer_cnt;
>
> Doubling the number of producers should double the perf metric.
> Like -p 4 should be half the speed of -p 8.
> In an ideal situation, of course.
> Without this normalization -p 1 vs -p 2 numbers are meaningless.
> Runs with different numbers of producers cannot be compared.
It is a good idea to compare the metric numbers between different
producers. Will do it in v5.
> .
