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