Re: [PATCH 1/3] reftable/stack: remove `REFTABLE_STACK_NEW_ADDITION_RELOAD`

Karthik Nayak <[email protected]> Thu, 20 Aug 2026 14:15:58 -0700

Newsgroups	org.kernel.vger.git
Message-ID	<CAOLa=ZRa9Dfdoj4XCpjhDJRn_mqyZ+91LNi5KTocyi6OrbsHbQ@mail.gmail.com>

Justin Tobler <[email protected]> writes:

> On 26/08/19 03:19PM, Karthik Nayak wrote:
>> In 80e7342ea8 (reftable/stack: allow locking of outdated stacks,
>> 2024-09-24), the `REFTABLE_STACK_NEW_ADDITION_RELOAD` was introduced so
>> that callers of `reftable_stack_init_addition()` can also reload the
>> stack if there was a concurrent update made before the lock was
>> obtained.
>>
>> Then 16684b6fae (refs/reftable: always reload stacks when creating
>> lock, 2025-08-12) updated all of the remaining call-sites to propagate
>> this flag to ensure that we always reload the stack whenever there was a
>> concurrent update.
>
> Ok, if all call sites already wire this flag, then we probably don't
> need if anymore.
>
>> As all calls to `reftable_stack_init_addition()` inevitably propagate
>> the flag, it is safe to remove the flag and its associated code and make
>> the reloading of the stack the default flow. This makes it easier to
>> follow the flow and simplifies the logic.
>
> Makes sense.
>
>> The only exceptions are:
>>
>>   1. Unit tests, where we explicitly do not propagate the flag. These
>>      tests are now modified with the new status quo.
>
> I assume this means we no longer need to test for the case where we
> don't reload.

There is no longer a 'don't reload' flow.

>
>>   2. `reftable_stack_clean_locked()`, which was propagating 0 to
>
> Did you mean `reftable_stack_clean()`?
>

Good catch, `reftable_stack_clean()` calls
`reftable_stack_clean_locked()`, but I should have mentioned
`reftable_stack_clean()`.

>>      `reftable_stack_new_addition()` but was then manually reloading the
>>      stack after. Here the new flow will achieve the same, while also
>>      allowing us to remove the manual reload.
>
> Out of curiousity, was this call site just forgotten previously? Or was
> there any reason a manual reload was useful?
>

My understanding was we added the flag at first for a few sites and then
expanded it. I guess reftable_stack_clean() sending in 0 was the
blocker, but we do indeed reload the stack manually there.

[snip]

signature.asc (application/pgp-signature, 690 B)