Re: [PATCH] socket: do not enable POLLOUT for empty out buffer

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

Hi Nikolay,

Thanks for your patch. Is there a particular reason it had to be
changed? The current behavior of the POLLOUT is to avoid a poll() call
when sending a new packet. I think the new behavior will force buffering
+ return to the main loop + poll() + send() instead of send() + main
loop + poll() + main loop again. It can look nasty in debug logs but I
think it's better for latency.

Aris

On 2/08/17 16:12, Nikolay wrote:
> Added new condition in ssh_socket_unbuffered_write() so that POLLOUT
> event for socket is not to be enabled if the whole buffer has been
> successfully written with send()/write().
>
> Signed-off-by: Nikolay N. Karikh <nitro@xxxxxxxxxxx>
> ---
>  src/socket.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/src/socket.c b/src/socket.c
> index 76dc55e..da7c46a 100644
> --- a/src/socket.c
> +++ b/src/socket.c
> @@ -567,11 +567,14 @@ static int
> ssh_socket_unbuffered_write(ssh_socket s, const void *buffer,
>  #else
>    s->last_errno = errno;
>  #endif
> -  s->write_wontblock = 0;
> -  /* Reactive the POLLOUT detector in the poll multiplexer system */
> -  if(s->poll_out){
> -      SSH_LOG(SSH_LOG_PACKET, "Enabling POLLOUT for socket");
> -     
> ssh_poll_set_events(s->poll_out,ssh_poll_get_events(s->poll_out) |
> POLLOUT);
> +  // only enable POLLOUT if unwritten data left in buffer
> +  if (w < len) {
> +    s->write_wontblock = 0;
> +    /* Reactive the POLLOUT detector in the poll multiplexer system */
> +    if(s->poll_out){
> +        SSH_LOG(SSH_LOG_PACKET, "Enabling POLLOUT for socket");
> +     
> ssh_poll_set_events(s->poll_out,ssh_poll_get_events(s->poll_out) |
> POLLOUT);
> +    }
>    }
>    if (w < 0) {
>      s->data_except = 1;