Thread: Re: Add AioUringCompletion in wait_event_names.txt and a safeguard in generate-wait_event_types.pl

On Mon, May 26, 2025 at 07:49:34AM +0000, Bertrand Drouvot wrote:
> c325a7633fcb forgot to add AioUringCompletion in wait_event_names.txt: please
> find attached v1-0001 fixing it (please double check the wait event description
> as I'm not that familiar with io_uring).

+AioUringCompletion "Waiting for another process to complete IO via io_uring."

"completion_lock" is described with similar words around the top of
method_io_uring.c.  In more exact words, we are waiting for a backend
to process IO completions with io_method=io_uring.  So perhaps:
"Waiting for another process to process IO completions with io_uring"

Adding Andres in CC for input, as I suspect that he'll think about a
better wording for the docs than what I could come up with.

> That makes me think that it is easy to miss adding a new LWLock in
> wait_event_names.txt and generate-wait_event_types.pl does not detect that.

Agreed, adding a check is a good idea seeing that the same mistake has
been repeated two times in the last couple of months by two different
committers.  This check is only in the docs, but the CI would detect
that.  This is not something strictly required for v18.  Let's tackle
that in v19~.

+   if (exists $hashwe{'WaitEventLWLock'})

So, the only reason why this is included in
generate-wait_event_types.pl is that we need the data from %hashwe,
reused in the new function you have added to perform the validation.

The implementation of your validation check is not consistent with the
existing generate-wait_event_types.pl, adding knowledge about lwlock.h
lwlocklist.h which are passed as extra arguments for the function
validate_lwlock_count().  Perhaps it would be better if split into its
own script, with the code in charge of building %hashwe (aka parsing
the .txt file given in input argument) moved to a .pm "Parsing"
module shared by both scripts, the one generating the data and the one
cross-checking the lwlock numbers?
--
Michael

On Tue, May 27, 2025 at 08:30:56AM +0900, Michael Paquier wrote:
> +AioUringCompletion "Waiting for another process to complete IO via io_uring."
>
> "completion_lock" is described with similar words around the top of
> method_io_uring.c.  In more exact words, we are waiting for a backend
> to process IO completions with io_method=io_uring.  So perhaps:
> "Waiting for another process to process IO completions with io_uring"

Hearing nothing, I've applied this one reusing the exact wording
suggested by Bertrand which is the most consistent choice with the
other wait events related to the same area of the code.
--
Michael