Re: Make pg_stat_io view count IOs as bytes instead of blocks

Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> writes:
> On Tue, 14 Jan 2025 at 06:18, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> And I've somewhat managed to fat-finger the business with
>> pgstat_count_io_op() with an incorrect rebase. Will remove in a
>> minute..

> Thank you!

Commit f92c854cf has caused some of the buildfarm members to
spout a new warning, eg at [1]:

ccache clang -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Werror=unguarded-availability-new -Wendif-labels -Wmissing-format-attribute -Wformat-security -Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -O2 -I. -I. -I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o pgstat_io.o pgstat_io.c
pgstat_io.c:81:9: warning: comparison of constant 8 with expression of type 'IOOp' (aka 'enum IOOp') is always true [-Wtautological-constant-out-of-range-compare]
Assert(pgstat_is_ioop_tracked_in_bytes(io_op) || bytes == 0);
~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pgstat_io.c:32:11: note: expanded from macro 'pgstat_is_ioop_tracked_in_bytes'
((io_op) < IOOP_NUM_TYPES && (io_op) >= IOOP_EXTEND)
^ ~~~~~~~~~~~~~~
../../../../src/include/c.h:829:9: note: expanded from macro 'Assert'
if (!(condition)) \\
^~~~~~~~~
1 warning generated.

I don't see a reasonable way to alter that check to suppress this;
for instance, "(io_op) <= IOOP_WRITE" would probably still draw the
same warning. I think most likely we have to remove that check, ie

#define pgstat_is_ioop_tracked_in_bytes(io_op) \
- ((io_op) < IOOP_NUM_TYPES && (io_op) >= IOOP_EXTEND)
+ ((io_op) >= IOOP_EXTEND)

I suppose one alternative is to re-order the enum so that the
upper-limit check in this macro *isn't* tautological ... but
that seems a bit silly.

regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=demoiselle&dt=2025-01-15%2005%3A20%3A59&stg=build