Thread (26 messages) flat view 26 messages, 5 authors, 14d ago

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

From: Junio C Hamano <hidden>
Date: 2026-08-19 17:18:05

Karthik Nayak [off-list ref] writes:
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.

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.
As long as nobody tries to open a nested or concurrent addition on
the same 'struct reftable_stack', this should be safe, but do we
give enough tools to help the API users avoid doing so?

I may be misreading the code completely, but when a caller already
holds a lock after calling reftable_stack_init_addition() on an
instance of reftable_stack, and then adds another reftable_addition
on the same reftable_stack, flock_acquire(add->stack->list_lock)
would fail because the lock is per stack now, unlike the original
code where the lock was per reftable_addition.  We jump to the
done: label and call reftable_addition_close(), which would release
the lock, which is now shared with other reftable_addition
instances that work on the same stack, which in turn would get the
holders of the lock into trouble, no?

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help