Re: [PATCH 2/3] reftable/stack: move list lock to `struct reftable_stack`

On 26/08/19 03:19PM, Karthik Nayak wrote:
> The struct `reftable_addition` is used to modify a given stack, as such,
> it also includes a `struct reftable_flock` used to obtain the lock to
> the list file. While the scope of the field lies within this struct, it
> doesn't allow for optimizations to be made on `struct reftable_stack`
> itself.

Hmmm IIUC, there can only be a single lock for the reftable stack
correct? If that is the case, it sounds like `struct reftable_stack` may
conceptually be the better place for the field regardless.

> Move the field to `struct reftable_stack`, allowing us to make a simple
> optimization around avoiding a stack reload when we have already
> obtained a lock. While this is currently possible in the write path, the
> write path also contains multiple branches to reads which only work
> on top of `struct reftable_stack`, and we would miss the optimization in
> such paths.

Ok, so if we know the reftable stack is alreay locked, there is no need
to reload it since it can't change. Makes sense.

> While here, remove an unused header file from 'reftable/stack.h'.
> 
> Signed-off-by: Karthik Nayak <[email protected]>
> ---
[snip]
>  struct reftable_stack {
> @@ -18,6 +17,12 @@ struct reftable_stack {
>  	char *list_file;
>  	int list_fd;
>  
> +	/*
> +	 * Set while an addition holds the stack locked. Used by
> +	 * stack_uptodate() to skip reload checks while locked.
> +	 */
> +	struct reftable_flock list_lock;
> +

As mentioned in the log message, the lock is now tracked in `struct
reftable_stack` and the rest of this patch just wires it accordingly.
Looks good.

-Justin