[RFC PATCH v4 1/5] server: Allow calling async_handoff() with status code STATUS_ALERTED.

[Zebediah Figura]

On 1/29/22 01:47, Jinoh Kang wrote:
> If the server detects that an I/O request could be completed immediately
> (e.g. the socket to read from already has incoming data), it can now
> return STATUS_ALERTED to allow opportunistic synchronous I/O.
> The Unix side will then attempt to perform I/O in nonblocking mode and
> report back the I/O status to the server with signal_wait_async().
> If the operation returns e.g. EAGAIN or EWOULDBLOCK, the client can opt
> to either abandon the request (by specifying an error status) or poll
> for it in the server as usual (by waiting on the wait handle).
> 
> Without such mechanism in place, the client cannot safely perform
> immediately satiable I/O operations synchronously, since it can
> potentially conflict with other pending I/O operations that have already
> been queued.
> 
> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
> ---
> 
> Notes:
>      v2 -> v3: set async->alerted = 1
>      v3 -> v4: unchanged
> 
>   server/async.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/server/async.c b/server/async.c
> index 7aef28355f0..e169bb23225 100644
> --- a/server/async.c
> +++ b/server/async.c
> @@ -338,6 +338,30 @@ obj_handle_t async_handoff( struct async *async, data_size_t *result, int force_
>           return async->wait_handle;
>       }
>   
> +    if (!async->pending && get_error() == STATUS_ALERTED)

Why "!async->pending"?

> +    {
> +        /* Give the client opportunity to complete synchronously.
> +         * If it turns out that the I/O request is not actually immediately satiable,
> +         * the client may then choose to re-queue the async (with STATUS_PENDING).

I'd explicitly mention *how* the async should be requeued here.

> +         */
> +        async->unknown_status = 1;
> +
> +        /* Deactivate async so that it can be safely queued.
> +         * Otherwise, the async will be erroneously alerted the next time
> +         * someone calls async_wake_up() on the queue.

Not to put too much weight on my own perspective, but I think it's 
clearer to explain *why* to do something, rather than why not. In this 
case, what we're doing is mimicking a normal "alert" case, except that 
we're not using an APC.

> +         *
> +         * Don't use async_terminate() here since we'd like to leave the IOSB
> +         * status as STATUS_PENDING.  

Wait, why is this important?

> Also we certainly don't want APC_ASYNC_IO
> +         * to fire in any circumstances.
> +         */
> +        async->terminated = 1;
> +        async->alerted = 1;
> +
> +        async_reselect( async );
> +
> +        return async->wait_handle;
> +    }
> +
>       async->initial_status = get_error();
>   
>       if (!async->pending && NT_ERROR( get_error() ))