[ovs-dev] [PATCH] latch-unix: Make the latch read buffer shared

[Ben Pfaff]

On Wed, Jul 14, 2021 at 05:36:36PM +0100, anton.ivanov at cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> 
> There is no point to add 512 bytes on the stack
> every time latch is polled. Alignment, cache line thrashing,
> etc - you name it.

Do you have evidence this is a real problem?

> The result of the read is discarded anyway so the buffer
> can be shared by all latches.
> 
> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>

> +/* All writes to latch are zero sized. Even 16 bytes are an overkill */
> +static char latch_buffer[16];

This comment is wrong.  Writes to a latch are 1 byte.

latch_poll() is supposed to fully clear any buffered data.  It shouldn't
cause behavioral problems if it doesn't, and I imagine that it's rare
that there'd be more than 16 queued notifications, but it seems
regressive to just clear some of them.

It's silly to use static data for 16 bytes.  If you're going to reduce
the size, just keep it as local.

>  /* Initializes 'latch' as initially unset. */
>  void
>  latch_init(struct latch *latch)
> @@ -43,9 +46,7 @@ latch_destroy(struct latch *latch)
>  bool
>  latch_poll(struct latch *latch)
>  {
> -    char buffer[_POSIX_PIPE_BUF];
> -
> -    return read(latch->fds[0], buffer, sizeof buffer) > 0;
> +    return read(latch->fds[0], &latch_buffer, sizeof latch_buffer) > 0;
>  }
>  
>  /* Sets 'latch'.
> -- 
> 2.20.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev