On Mon, Aug 3, 2026 at 6:45 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> For master, I actually think what we should do is save the IOContext
> in the ReadBuffersOperation instead of the BufferAccessStrategy. I
> think it is cleaner since you only need the IOContext when completing
> the IO and you can pass the BufferAccessStrategy directly to
> StartReadBuffers() without saving it in the ReadBuffersOperation.
>
> And, when it comes to this patch and being able to "deactivate" the
> buffer access strategy, we need a way to do so for future IOs without
> affecting in-progress IOs, and this structure seems like the best way
> to do that.
Andres pointed out off-list that we had seen regressions when adding
another parameter to StartReadBuffersImpl() because it currently has 6
arguments and the sysv-x86-64 ABI can only accommodate 6 arguments in
registers before spilling to stack. As such, most of my "elegant"
solutions don't work. One thing we could do is add another
StartReadBuffersImpl() flag for "use strategy". I tried that and
didn't love how it looked since we then have to check that flag before
using the strategy in 5 places and that can only grow in the future.
Instead, I propose we just take a small hit to the accounting and
simply clear the ReadBuffersOperations->strategy even of in-progress
IOs. It should only misattribute a bit of wait time -- and only for a
limited number of IOs once -- when we enter failsafe mode. It makes
for a simple, backpatchable solution. I've attached here what I intend
to commit later today.
- Melanie
