Am 14.01.2015 um 14:49 hat Paolo Bonzini geschrieben:
On 14/01/2015 14:38, Kevin Wolf wrote:
Well, what do you want to use it for? I thought it would only be for a
one-time check where we usually end up rather than something that would
be enabled in production, but maybe I misunderstood.
No, you didn't. Though I guess we could limit the checks to the yield
points. If we have BDS recursion, as in the backing file case, yield
points should not be far from the deepest part of the stack.
Another possibility (which cannot be enabled in production) is to fill
the stack with a known 64-bit value, and do a binary search when the
coroutine is destroyed.
Maybe that's the easiest one, yes.
I tried gathering warning from GCC's -Wstack-usage=1023 option and the
block layer does not seem to have functions with huge stacks in the I/O
path.
So, assuming a maximum stack depth of 50 (already pretty generous since
there shouldn't be any recursive calls) a 100K stack should be pretty
much okay for coroutines and thread-pool threads.
The potential problem in the block layer is long backing file chains.
Perhaps we need to do something to solve that iteratively instead of
recursively.
Basically first read stuff from the current BDS, and then "fill in the
blanks" with a tail call on bs->backing_file? That would be quite a
change, and we'd need a stopgap measure like Alex's patch in the meanwhile.
Basically block.c would do something like get_block_status() first and
only then call the read/write functions of the individual drivers. But
yes, that's more a theoretical consideration at this point.
I think with the 50 recursions that you calculated we should be fine in
practice for now. I would however strongly recommend finally implementing
a guard page for coroutine stacks before we make that change.