qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 10/12] block.c: add subtree_drains where needed


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH 10/12] block.c: add subtree_drains where needed
Date: Fri, 4 Feb 2022 17:03:03 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

04.02.2022 16:30, Emanuele Giuseppe Esposito wrote:

  From the other thread:
So you mean that backing_hd is modified as its parent is changed, do
I understand correctly?

Yes


As we started to discuss in a thread started with my "[PATCH] block:
bdrv_set_backing_hd(): use drained section", I think draining the whole
subtree when we modify some parent-child relation is too much. In-flight
requests in subtree don't touch these relations, so why to wait/stop
them? Imagine two disks A and B with same backing file C. If we want to
detach A from C, what is the reason to drain in-fligth read request of B
in C?

If I am not mistaken, bdrv_set_backing_hd adds a new node (yes removes
the old backing_hd, but let's not think about this for a moment).
So we have disk B with backing file C, and new disk A that wants to have
backing file C.

I think I understand what you mean, so in theory the operation would be
- create new child
- add child to A->children list
- add child to C->parents list

So in theory we need to
* drain A (without subtree), because it can't happen that child nodes of
    A have in-flight requests that look at A status (children list),
right?
    In other words, if A has another node X, can a request in X inspect
    A->children

It should not happen. In my understanding, IO request never access
parents of the node.

* drain C, as parents can inspect C status (like B). Same assumption
    here, C->children[x]->bs cannot have requests inspecting C->parents
    list?

No, I think we should not drain C. IO requests don't inspect parents
list. And if some _other_ operations do inspect it, drain will not help,
as drain is only about IO requests.

I am still not convinced about this, because of the draining.

While drain can only be called by either main loop or the "home
iothread" (the one running the AioContext), it still means there are 2
threads involved. So while the iothread runs set_backing_hd, main loop
could easily drain one of the children of the nodes. Or the other way
around.
I am not saying that this happens, but it *might* happen.

I agree that it might happen. So, parallel drains are a problem. But drain is 
always a part of graph modification, and any graph modifications running in 
parallel are already a problem, we should forbid it somehow.

When you drain node, you say: please no in-flight requests now at this node. 
But IO requests doesn't do drain themselves, so why to restrict them?

And anyway, I don't see how this help. Ok, assume you drain additional node C 
or even the whole subtree. What protect us from parallel drain from another 
thread?


I am a little bit confused about this, it would be nice to hear opinions
from others as well.

Once we figure this, I will know where exactly to put the assertions,
and consequently what to drain and with which function.

Emanuele



--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]