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. Anyway, the thread pool workers aren't affected by any of this, so they would be the obvious first step. Kevin