Re: Buffer locking is special (hints, checksums, AIO writes)

Hi,

On 2026-01-15 18:02:34 -0500, Tom Lane wrote:
> In file included from ../pgsql/src/include/pgstat.h:24,
> from ../pgsql/src/backend/storage/buffer/bufmgr.c:52:
> In function \342\200\230pgstat_report_wait_start\342\200\231,
> inlined from \342\200\230BufferLockAcquire\342\200\231 at ../pgsql/src/backend/storage/buffer/bufmgr.c:5833:3:
> ../pgsql/src/include/utils/wait_event.h:75:49: warning: \342\200\230wait_event\342\200\231 may be used uninitialized [-Wmaybe-uninitialized]
> 75 | *(volatile uint32 *) my_wait_event_info = wait_event_info;
> [...]
> Apparently they do not find the coding in this switch persuasive:
>
> switch (mode)
> {
> case BUFFER_LOCK_EXCLUSIVE:
> wait_event = WAIT_EVENT_BUFFER_EXCLUSIVE;
> break;
> case BUFFER_LOCK_SHARE_EXCLUSIVE:
> wait_event = WAIT_EVENT_BUFFER_SHARE_EXCLUSIVE;
> break;
> case BUFFER_LOCK_SHARE:
> wait_event = WAIT_EVENT_BUFFER_SHARED;
> break;
> case BUFFER_LOCK_UNLOCK:
> pg_unreachable();
>
> }
>
> It's not clear to me whether that's more about not believing
> pg_unreachable() or more about the lack of a default: case.

> I see that this is a modification of code that existed before
> fcb9c977a and wasn't being complained of, which makes it even
> stranger.

The code before did have a default:, so I guess that could be the
difference...

I can reproduce it here if I build with -Og or -O1, but not with -O2, so I
guess it's "just" the optimizer falling short somehow.

Huh, interesting, the optimization level dependency made me curious. Turns out
the warning goes away if I actually force the inlining of BufferLockAcquire()
with pg_attribute_always_inline. Presumably because then the compiler realizes
that the mode argument is always a constant that it can evaluate.

Can't quite decide between just using always_inline - after all the buffer
locking really is a rather crucial code path - and just initializing
wait_event to 0. Either seems better than using a default:.

Greetings,

Andres Freund