Re: [PATCH 6/9] block/accounting: Emit BLOCK_IO_DELAY event

[Hanna Czenczek <hreitz@redhat.com>]

On 21.09.26 22:48, Stefan Hajnoczi wrote:
> On Wed, Sep 16, 2026 at 02:04:58PM +0200, Hanna Czenczek wrote:
>> On 16.09.26 10:14, Hanna Czenczek wrote:
>>> On 03.09.26 16:23, Stefan Hajnoczi wrote:
>>>> On Mon, Aug 31, 2026 at 03:52:02PM +0200, Hanna Czenczek wrote:
>>>>> When a request finishes with a higher latency than a predefined
>>>>> threshold, emit the BLOCK_IO_DELAY event.
>>>>>
>>>>> Note there would be an alternative, more precise solution: We
>>>>> could keep
>>>>> all active cookies per BlockBackend in a list and repeatedly iterate
>>>>> over it in a background coroutine (woken on a timer so it would wake
>>>>> always exactly when the next request would time out, so it generally
>>>>> stays asleep until there is actually a timeout). This way, we
>>>>> could emit
>>>>> the event exactly when a request crosses the delay threshold, while it
>>>>> is still running; and we could hypothetically even take actions like
>>>>> pausing the VM until the request is done so the guest operating system
>>>>> is shielded from extreme latency spikes.
>>>>>
>>>>> In practice, this is very complicated because latency cookies are
>>>>> created and finalized all over the place, so it is very hard to
>>>>> guarantee that every `block_acct_start()` is matched by the right
>>>>> finalization to ensure that cookies are properly removed from the list
>>>>> when they are done. Even if we fix all non-matching places now,
>>>>> there is
>>>>> hardly a guarantee this will be kept in order in the future.
>>>> I think this patch series already couples the accounting so closely with
>>>> BlockBackend (i.e. adding the offset field into the cookie struct and
>>>> adding a BB pointer into the stats struct) that we might as well fully
>>>> integrate the two. Then callers don't need to manually manage cookies
>>>> because BlockBackend does that internally and the concerns about
>>>> lifetimes go away.
>>> I don’t follow how integrating them into BlockBackend automatically
>>> solves the problem.
>>>
>>> Are you suggesting that blk_* I/O functions should do the accounting
>>> instead of the device emulation code?
> Yes.

OK, I see.

>> The thing is, AFAIU, doing that would cause changes in behavior, because the
>> hardware device requests don’t always line up with the BB requests.
> Did you find a fundamental incompatibility that rules out letting the
> block layer handles stats for blk_aio_*()?

I mean, a simple thing is that reset requests are currently just not 
accounted, but they would be accounted then. Or repeated requests after 
VM stop/cont.

There is also the fact that rerror/werror set to IGNORE means errors are 
currently accounted as 'done', not 'failed'. That makes… a little sense 
in the device models, but not really much sense in BB, I feel like.

So I am not sure what you mean by “fundamental incompatibility”. I’m 
sure if we make it sufficiently ugly, we could reproduce all current 
peculiarities. We could use the chance to make it cleaner, but that 
would change the interface, and “cleaner” is always in the eye of the 
beholder. (Besides the fact that I feel like you are trying to have me 
open a can of worms, I feel like. :) )

>> What I could do would be to abandon the cookie-based approach altogether, of
>> course; instead creating a completely different tracking object in those BB
>> functions and put those into a list. And then we could decide at a later
>> point if we want to integrate cookies with that. The only downside I see is
>> that this would mean the latency reporting would not line up with the
>> requests reported in the stats, or the histogram.
> The tracking object could be a timer! :)

The problem is not timer or sleep, the problem is the lifecycle.

Hanna