* [Qemu-devel] Limiting coroutine stack usage
@ 2018-02-20 17:04 Peter Lieven
2018-02-20 21:54 ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
2018-03-06 11:51 ` Stefan Hajnoczi
0 siblings, 2 replies; 19+ messages in thread
From: Peter Lieven @ 2018-02-20 17:04 UTC (permalink / raw)
To: qemu block, qemu-devel
Hi,
I remember we discussed a long time ago to limit the stack usage of all functions that are executed in a coroutine
context to a very low value to be able to safely limit the coroutine stack size as well.
I checked through all functions in block/, migration/ and nbd/ and there are only very few larger or unbound stack
allocations that can easily be fixed.
Now my question: Is there an easy way to add a cflag like -Wstack-usage=2048 to all objects in a given directory only?
I tried to add a llimit to the whole project, but fixing this will be a larger task.
Thanks,
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-20 17:04 [Qemu-devel] Limiting coroutine stack usage Peter Lieven
@ 2018-02-20 21:54 ` Paolo Bonzini
2018-02-22 10:57 ` Kevin Wolf
2018-03-06 11:51 ` Stefan Hajnoczi
1 sibling, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2018-02-20 21:54 UTC (permalink / raw)
To: Peter Lieven, qemu block, qemu-devel
On 20/02/2018 18:04, Peter Lieven wrote:
> Hi,
>
> I remember we discussed a long time ago to limit the stack usage of all
> functions that are executed in a coroutine
> context to a very low value to be able to safely limit the coroutine
> stack size as well.
IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
ide_atapi_cmd_reply_end.
But perhaps it's not an issue, somebody needs to audit the code.
> I checked through all functions in block/, migration/ and nbd/ and there
> are only very few larger or unbound stack
> allocations that can easily be fixed.
Yeah, the really large allocations are very few and mostly have to do
with networking (https://wiki.qemu.org/BiteSizedTasks#Large_frames).
Unfortunately the link to the original list has died together with the
gmane archives.
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-20 21:54 ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
@ 2018-02-22 10:57 ` Kevin Wolf
2018-02-22 11:01 ` Peter Lieven
2018-02-22 17:06 ` John Snow
0 siblings, 2 replies; 19+ messages in thread
From: Kevin Wolf @ 2018-02-22 10:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peter Lieven, qemu block, qemu-devel, jsnow
Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> On 20/02/2018 18:04, Peter Lieven wrote:
> > Hi,
> >
> > I remember we discussed a long time ago to limit the stack usage of all
> > functions that are executed in a coroutine
> > context to a very low value to be able to safely limit the coroutine
> > stack size as well.
>
> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> ide_atapi_cmd_reply_end.
>
> But perhaps it's not an issue, somebody needs to audit the code.
I think John intended to get rid of the recursion sometime, but I doubt
he has had the time so far.
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 10:57 ` Kevin Wolf
@ 2018-02-22 11:01 ` Peter Lieven
2018-02-22 11:32 ` Kevin Wolf
2018-02-22 17:06 ` John Snow
1 sibling, 1 reply; 19+ messages in thread
From: Peter Lieven @ 2018-02-22 11:01 UTC (permalink / raw)
To: Kevin Wolf, Paolo Bonzini; +Cc: qemu block, qemu-devel, jsnow
Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>> On 20/02/2018 18:04, Peter Lieven wrote:
>>> Hi,
>>>
>>> I remember we discussed a long time ago to limit the stack usage of all
>>> functions that are executed in a coroutine
>>> context to a very low value to be able to safely limit the coroutine
>>> stack size as well.
>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>> ide_atapi_cmd_reply_end.
>>
>> But perhaps it's not an issue, somebody needs to audit the code.
> I think John intended to get rid of the recursion sometime, but I doubt
> he has had the time so far.
Apart from this is is possible to define special cflags in the Makefile.objs just
for a subdirectory? I have patches ready to make the block layer files and
other coroutine users compile with -Wstack-size=2048. But I do not want
to specify each file separately.
Limiting the coroutine size to much could also lead to trouble in some third party
libraries that are called from a coroutine context, or not?
Thanks,
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 11:01 ` Peter Lieven
@ 2018-02-22 11:32 ` Kevin Wolf
2018-02-22 11:40 ` Daniel P. Berrangé
2018-02-22 11:42 ` Peter Lieven
0 siblings, 2 replies; 19+ messages in thread
From: Kevin Wolf @ 2018-02-22 11:32 UTC (permalink / raw)
To: Peter Lieven; +Cc: Paolo Bonzini, qemu block, qemu-devel, jsnow
Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> > Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> >> On 20/02/2018 18:04, Peter Lieven wrote:
> >>> Hi,
> >>>
> >>> I remember we discussed a long time ago to limit the stack usage of all
> >>> functions that are executed in a coroutine
> >>> context to a very low value to be able to safely limit the coroutine
> >>> stack size as well.
> >> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> >> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> >> ide_atapi_cmd_reply_end.
> >>
> >> But perhaps it's not an issue, somebody needs to audit the code.
> > I think John intended to get rid of the recursion sometime, but I doubt
> > he has had the time so far.
>
> Apart from this is is possible to define special cflags in the
> Makefile.objs just for a subdirectory? I have patches ready to make
> the block layer files and other coroutine users compile with
> -Wstack-size=2048. But I do not want to specify each file separately.
Our Makefiles have lines like this:
iscsi.o-cflags := $(LIBISCSI_CFLAGS)
I don't think there is a direct mechanism to apply cflags to a whole
directory or just to block-obj-y/block-obj-m, but just looping over them
could work. I'm not a Makefile expert at all, but after some toying with
a simple example, something like this might work:
$(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
> Limiting the coroutine size to much could also lead to trouble in some
> third party libraries that are called from a coroutine context, or
> not?
Yes, this is true.
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 11:32 ` Kevin Wolf
@ 2018-02-22 11:40 ` Daniel P. Berrangé
2018-02-22 11:51 ` Peter Lieven
2018-02-22 11:42 ` Peter Lieven
1 sibling, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2018-02-22 11:40 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Peter Lieven, Paolo Bonzini, jsnow, qemu-devel, qemu block
On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
> > Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> > > Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> > >> On 20/02/2018 18:04, Peter Lieven wrote:
> > >>> Hi,
> > >>>
> > >>> I remember we discussed a long time ago to limit the stack usage of all
> > >>> functions that are executed in a coroutine
> > >>> context to a very low value to be able to safely limit the coroutine
> > >>> stack size as well.
> > >> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> > >> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> > >> ide_atapi_cmd_reply_end.
> > >>
> > >> But perhaps it's not an issue, somebody needs to audit the code.
> > > I think John intended to get rid of the recursion sometime, but I doubt
> > > he has had the time so far.
> >
> > Apart from this is is possible to define special cflags in the
> > Makefile.objs just for a subdirectory? I have patches ready to make
> > the block layer files and other coroutine users compile with
> > -Wstack-size=2048. But I do not want to specify each file separately.
>
> Our Makefiles have lines like this:
>
> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>
> I don't think there is a direct mechanism to apply cflags to a whole
> directory or just to block-obj-y/block-obj-m, but just looping over them
> could work. I'm not a Makefile expert at all, but after some toying with
> a simple example, something like this might work:
>
> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
You'll need it for anything block layer depends on too - so that's much
of util/, crypto/ and io/ directories at least.
So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
globally for everything in QEMU, and then override -Wstack-size=$BIGGER
for the (hopefully) few sources that have a larger stack need ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 11:32 ` Kevin Wolf
2018-02-22 11:40 ` Daniel P. Berrangé
@ 2018-02-22 11:42 ` Peter Lieven
1 sibling, 0 replies; 19+ messages in thread
From: Peter Lieven @ 2018-02-22 11:42 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, qemu block, qemu-devel, jsnow
Am 22.02.2018 um 12:32 schrieb Kevin Wolf:
> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>>>> On 20/02/2018 18:04, Peter Lieven wrote:
>>>>> Hi,
>>>>>
>>>>> I remember we discussed a long time ago to limit the stack usage of all
>>>>> functions that are executed in a coroutine
>>>>> context to a very low value to be able to safely limit the coroutine
>>>>> stack size as well.
>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>>>> ide_atapi_cmd_reply_end.
>>>>
>>>> But perhaps it's not an issue, somebody needs to audit the code.
>>> I think John intended to get rid of the recursion sometime, but I doubt
>>> he has had the time so far.
>> Apart from this is is possible to define special cflags in the
>> Makefile.objs just for a subdirectory? I have patches ready to make
>> the block layer files and other coroutine users compile with
>> -Wstack-size=2048. But I do not want to specify each file separately.
> Our Makefiles have lines like this:
>
> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>
> I don't think there is a direct mechanism to apply cflags to a whole
> directory or just to block-obj-y/block-obj-m, but just looping over them
> could work. I'm not a Makefile expert at all, but after some toying with
> a simple example, something like this might work:
>
> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
Thanks for the hint.
If noone comes up with a better idea I will try this.
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 11:40 ` Daniel P. Berrangé
@ 2018-02-22 11:51 ` Peter Lieven
2018-02-22 12:00 ` Daniel P. Berrangé
0 siblings, 1 reply; 19+ messages in thread
From: Peter Lieven @ 2018-02-22 11:51 UTC (permalink / raw)
To: Daniel P. Berrangé, Kevin Wolf
Cc: Paolo Bonzini, jsnow, qemu-devel, qemu block
Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
> On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
>> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
>>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
>>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>>>>> On 20/02/2018 18:04, Peter Lieven wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I remember we discussed a long time ago to limit the stack usage of all
>>>>>> functions that are executed in a coroutine
>>>>>> context to a very low value to be able to safely limit the coroutine
>>>>>> stack size as well.
>>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>>>>> ide_atapi_cmd_reply_end.
>>>>>
>>>>> But perhaps it's not an issue, somebody needs to audit the code.
>>>> I think John intended to get rid of the recursion sometime, but I doubt
>>>> he has had the time so far.
>>> Apart from this is is possible to define special cflags in the
>>> Makefile.objs just for a subdirectory? I have patches ready to make
>>> the block layer files and other coroutine users compile with
>>> -Wstack-size=2048. But I do not want to specify each file separately.
>> Our Makefiles have lines like this:
>>
>> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>>
>> I don't think there is a direct mechanism to apply cflags to a whole
>> directory or just to block-obj-y/block-obj-m, but just looping over them
>> could work. I'm not a Makefile expert at all, but after some toying with
>> a simple example, something like this might work:
>>
>> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
> You'll need it for anything block layer depends on too - so that's much
> of util/, crypto/ and io/ directories at least.
>
> So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
> globally for everything in QEMU, and then override -Wstack-size=$BIGGER
> for the (hopefully) few sources that have a larger stack need ?
I tried that already. 2048 is a strong limit for many functions.
It breaks already as soon as some buffer has a size of PATH_MAX, but
thats handleable. But there are some structs around that are very large.
Generally, it would be a good idea to have a global limit, of course.
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 11:51 ` Peter Lieven
@ 2018-02-22 12:00 ` Daniel P. Berrangé
2018-02-22 12:02 ` Peter Lieven
0 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2018-02-22 12:00 UTC (permalink / raw)
To: Peter Lieven; +Cc: Kevin Wolf, Paolo Bonzini, jsnow, qemu-devel, qemu block
On Thu, Feb 22, 2018 at 12:51:58PM +0100, Peter Lieven wrote:
> Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
> > On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
> >> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
> >>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> >>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> >>>>> On 20/02/2018 18:04, Peter Lieven wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> I remember we discussed a long time ago to limit the stack usage of all
> >>>>>> functions that are executed in a coroutine
> >>>>>> context to a very low value to be able to safely limit the coroutine
> >>>>>> stack size as well.
> >>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> >>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> >>>>> ide_atapi_cmd_reply_end.
> >>>>>
> >>>>> But perhaps it's not an issue, somebody needs to audit the code.
> >>>> I think John intended to get rid of the recursion sometime, but I doubt
> >>>> he has had the time so far.
> >>> Apart from this is is possible to define special cflags in the
> >>> Makefile.objs just for a subdirectory? I have patches ready to make
> >>> the block layer files and other coroutine users compile with
> >>> -Wstack-size=2048. But I do not want to specify each file separately.
> >> Our Makefiles have lines like this:
> >>
> >> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
> >>
> >> I don't think there is a direct mechanism to apply cflags to a whole
> >> directory or just to block-obj-y/block-obj-m, but just looping over them
> >> could work. I'm not a Makefile expert at all, but after some toying with
> >> a simple example, something like this might work:
> >>
> >> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
> > You'll need it for anything block layer depends on too - so that's much
> > of util/, crypto/ and io/ directories at least.
> >
> > So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
> > globally for everything in QEMU, and then override -Wstack-size=$BIGGER
> > for the (hopefully) few sources that have a larger stack need ?
>
> I tried that already. 2048 is a strong limit for many functions.
> It breaks already as soon as some buffer has a size of PATH_MAX, but
> thats handleable. But there are some structs around that are very large.
There are surprisingly few "char [PATH_MAX]" variables left in QEMU - we
should have a final push to eliminate them regardless.
> Generally, it would be a good idea to have a global limit, of course.
We could at least put a limit on that matches the current worst case to
prevent it getting worse than it already is.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 12:00 ` Daniel P. Berrangé
@ 2018-02-22 12:02 ` Peter Lieven
2018-02-22 12:03 ` Daniel P. Berrangé
0 siblings, 1 reply; 19+ messages in thread
From: Peter Lieven @ 2018-02-22 12:02 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, Paolo Bonzini, jsnow, qemu-devel, qemu block
Am 22.02.2018 um 13:00 schrieb Daniel P. Berrangé:
> On Thu, Feb 22, 2018 at 12:51:58PM +0100, Peter Lieven wrote:
>> Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
>>> On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
>>>> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
>>>>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
>>>>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>>>>>>> On 20/02/2018 18:04, Peter Lieven wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I remember we discussed a long time ago to limit the stack usage of all
>>>>>>>> functions that are executed in a coroutine
>>>>>>>> context to a very low value to be able to safely limit the coroutine
>>>>>>>> stack size as well.
>>>>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>>>>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>>>>>>> ide_atapi_cmd_reply_end.
>>>>>>>
>>>>>>> But perhaps it's not an issue, somebody needs to audit the code.
>>>>>> I think John intended to get rid of the recursion sometime, but I doubt
>>>>>> he has had the time so far.
>>>>> Apart from this is is possible to define special cflags in the
>>>>> Makefile.objs just for a subdirectory? I have patches ready to make
>>>>> the block layer files and other coroutine users compile with
>>>>> -Wstack-size=2048. But I do not want to specify each file separately.
>>>> Our Makefiles have lines like this:
>>>>
>>>> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>>>>
>>>> I don't think there is a direct mechanism to apply cflags to a whole
>>>> directory or just to block-obj-y/block-obj-m, but just looping over them
>>>> could work. I'm not a Makefile expert at all, but after some toying with
>>>> a simple example, something like this might work:
>>>>
>>>> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
>>> You'll need it for anything block layer depends on too - so that's much
>>> of util/, crypto/ and io/ directories at least.
>>>
>>> So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
>>> globally for everything in QEMU, and then override -Wstack-size=$BIGGER
>>> for the (hopefully) few sources that have a larger stack need ?
>> I tried that already. 2048 is a strong limit for many functions.
>> It breaks already as soon as some buffer has a size of PATH_MAX, but
>> thats handleable. But there are some structs around that are very large.
> There are surprisingly few "char [PATH_MAX]" variables left in QEMU - we
> should have a final push to eliminate them regardless.
>
>> Generally, it would be a good idea to have a global limit, of course.
> We could at least put a limit on that matches the current worst case to
> prevent it getting worse than it already is.
That would be a good idea, yes.
How would you handle the override for a smaller -Wstack-usage ?
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 12:02 ` Peter Lieven
@ 2018-02-22 12:03 ` Daniel P. Berrangé
2018-02-22 12:06 ` Peter Lieven
0 siblings, 1 reply; 19+ messages in thread
From: Daniel P. Berrangé @ 2018-02-22 12:03 UTC (permalink / raw)
To: Peter Lieven; +Cc: Kevin Wolf, Paolo Bonzini, jsnow, qemu-devel, qemu block
On Thu, Feb 22, 2018 at 01:02:05PM +0100, Peter Lieven wrote:
> Am 22.02.2018 um 13:00 schrieb Daniel P. Berrangé:
> > On Thu, Feb 22, 2018 at 12:51:58PM +0100, Peter Lieven wrote:
> >> Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
> >>> On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
> >>>> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
> >>>>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> >>>>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> >>>>>>> On 20/02/2018 18:04, Peter Lieven wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> I remember we discussed a long time ago to limit the stack usage of all
> >>>>>>>> functions that are executed in a coroutine
> >>>>>>>> context to a very low value to be able to safely limit the coroutine
> >>>>>>>> stack size as well.
> >>>>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> >>>>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> >>>>>>> ide_atapi_cmd_reply_end.
> >>>>>>>
> >>>>>>> But perhaps it's not an issue, somebody needs to audit the code.
> >>>>>> I think John intended to get rid of the recursion sometime, but I doubt
> >>>>>> he has had the time so far.
> >>>>> Apart from this is is possible to define special cflags in the
> >>>>> Makefile.objs just for a subdirectory? I have patches ready to make
> >>>>> the block layer files and other coroutine users compile with
> >>>>> -Wstack-size=2048. But I do not want to specify each file separately.
> >>>> Our Makefiles have lines like this:
> >>>>
> >>>> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
> >>>>
> >>>> I don't think there is a direct mechanism to apply cflags to a whole
> >>>> directory or just to block-obj-y/block-obj-m, but just looping over them
> >>>> could work. I'm not a Makefile expert at all, but after some toying with
> >>>> a simple example, something like this might work:
> >>>>
> >>>> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
> >>> You'll need it for anything block layer depends on too - so that's much
> >>> of util/, crypto/ and io/ directories at least.
> >>>
> >>> So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
> >>> globally for everything in QEMU, and then override -Wstack-size=$BIGGER
> >>> for the (hopefully) few sources that have a larger stack need ?
> >> I tried that already. 2048 is a strong limit for many functions.
> >> It breaks already as soon as some buffer has a size of PATH_MAX, but
> >> thats handleable. But there are some structs around that are very large.
> > There are surprisingly few "char [PATH_MAX]" variables left in QEMU - we
> > should have a final push to eliminate them regardless.
> >
> >> Generally, it would be a good idea to have a global limit, of course.
> > We could at least put a limit on that matches the current worst case to
> > prevent it getting worse than it already is.
>
> That would be a good idea, yes.
>
> How would you handle the override for a smaller -Wstack-usage ?
If you have multiple -Wstack-size=$XXX flags to GCC, I expect the last
one wins. So just need to double check that the per-object file CFLAGS
occur after the global CFLAS in the compiler args
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 12:03 ` Daniel P. Berrangé
@ 2018-02-22 12:06 ` Peter Lieven
2018-02-22 12:16 ` Daniel P. Berrangé
2018-02-22 12:19 ` Kevin Wolf
0 siblings, 2 replies; 19+ messages in thread
From: Peter Lieven @ 2018-02-22 12:06 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, Paolo Bonzini, jsnow, qemu-devel, qemu block
Am 22.02.2018 um 13:03 schrieb Daniel P. Berrangé:
> On Thu, Feb 22, 2018 at 01:02:05PM +0100, Peter Lieven wrote:
>> Am 22.02.2018 um 13:00 schrieb Daniel P. Berrangé:
>>> On Thu, Feb 22, 2018 at 12:51:58PM +0100, Peter Lieven wrote:
>>>> Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
>>>>> On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
>>>>>> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
>>>>>>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
>>>>>>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>>>>>>>>> On 20/02/2018 18:04, Peter Lieven wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I remember we discussed a long time ago to limit the stack usage of all
>>>>>>>>>> functions that are executed in a coroutine
>>>>>>>>>> context to a very low value to be able to safely limit the coroutine
>>>>>>>>>> stack size as well.
>>>>>>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>>>>>>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>>>>>>>>> ide_atapi_cmd_reply_end.
>>>>>>>>>
>>>>>>>>> But perhaps it's not an issue, somebody needs to audit the code.
>>>>>>>> I think John intended to get rid of the recursion sometime, but I doubt
>>>>>>>> he has had the time so far.
>>>>>>> Apart from this is is possible to define special cflags in the
>>>>>>> Makefile.objs just for a subdirectory? I have patches ready to make
>>>>>>> the block layer files and other coroutine users compile with
>>>>>>> -Wstack-size=2048. But I do not want to specify each file separately.
>>>>>> Our Makefiles have lines like this:
>>>>>>
>>>>>> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>>>>>>
>>>>>> I don't think there is a direct mechanism to apply cflags to a whole
>>>>>> directory or just to block-obj-y/block-obj-m, but just looping over them
>>>>>> could work. I'm not a Makefile expert at all, but after some toying with
>>>>>> a simple example, something like this might work:
>>>>>>
>>>>>> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
>>>>> You'll need it for anything block layer depends on too - so that's much
>>>>> of util/, crypto/ and io/ directories at least.
>>>>>
>>>>> So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
>>>>> globally for everything in QEMU, and then override -Wstack-size=$BIGGER
>>>>> for the (hopefully) few sources that have a larger stack need ?
>>>> I tried that already. 2048 is a strong limit for many functions.
>>>> It breaks already as soon as some buffer has a size of PATH_MAX, but
>>>> thats handleable. But there are some structs around that are very large.
>>> There are surprisingly few "char [PATH_MAX]" variables left in QEMU - we
>>> should have a final push to eliminate them regardless.
>>>
>>>> Generally, it would be a good idea to have a global limit, of course.
>>> We could at least put a limit on that matches the current worst case to
>>> prevent it getting worse than it already is.
>> That would be a good idea, yes.
>>
>> How would you handle the override for a smaller -Wstack-usage ?
> If you have multiple -Wstack-size=$XXX flags to GCC, I expect the last
> one wins. So just need to double check that the per-object file CFLAGS
> occur after the global CFLAS in the compiler args
I will check that, thanks.
When I am at it, what would be the proper replacement for char[PATH_MAX] ?
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 12:06 ` Peter Lieven
@ 2018-02-22 12:16 ` Daniel P. Berrangé
2018-02-22 12:19 ` Kevin Wolf
1 sibling, 0 replies; 19+ messages in thread
From: Daniel P. Berrangé @ 2018-02-22 12:16 UTC (permalink / raw)
To: Peter Lieven; +Cc: Kevin Wolf, Paolo Bonzini, jsnow, qemu-devel, qemu block
On Thu, Feb 22, 2018 at 01:06:33PM +0100, Peter Lieven wrote:
> Am 22.02.2018 um 13:03 schrieb Daniel P. Berrangé:
> > On Thu, Feb 22, 2018 at 01:02:05PM +0100, Peter Lieven wrote:
> >> Am 22.02.2018 um 13:00 schrieb Daniel P. Berrangé:
> >>> On Thu, Feb 22, 2018 at 12:51:58PM +0100, Peter Lieven wrote:
> >>>> Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
> >>>>> On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
> >>>>>> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
> >>>>>>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> >>>>>>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> >>>>>>>>> On 20/02/2018 18:04, Peter Lieven wrote:
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> I remember we discussed a long time ago to limit the stack usage of all
> >>>>>>>>>> functions that are executed in a coroutine
> >>>>>>>>>> context to a very low value to be able to safely limit the coroutine
> >>>>>>>>>> stack size as well.
> >>>>>>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> >>>>>>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> >>>>>>>>> ide_atapi_cmd_reply_end.
> >>>>>>>>>
> >>>>>>>>> But perhaps it's not an issue, somebody needs to audit the code.
> >>>>>>>> I think John intended to get rid of the recursion sometime, but I doubt
> >>>>>>>> he has had the time so far.
> >>>>>>> Apart from this is is possible to define special cflags in the
> >>>>>>> Makefile.objs just for a subdirectory? I have patches ready to make
> >>>>>>> the block layer files and other coroutine users compile with
> >>>>>>> -Wstack-size=2048. But I do not want to specify each file separately.
> >>>>>> Our Makefiles have lines like this:
> >>>>>>
> >>>>>> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
> >>>>>>
> >>>>>> I don't think there is a direct mechanism to apply cflags to a whole
> >>>>>> directory or just to block-obj-y/block-obj-m, but just looping over them
> >>>>>> could work. I'm not a Makefile expert at all, but after some toying with
> >>>>>> a simple example, something like this might work:
> >>>>>>
> >>>>>> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
> >>>>> You'll need it for anything block layer depends on too - so that's much
> >>>>> of util/, crypto/ and io/ directories at least.
> >>>>>
> >>>>> So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
> >>>>> globally for everything in QEMU, and then override -Wstack-size=$BIGGER
> >>>>> for the (hopefully) few sources that have a larger stack need ?
> >>>> I tried that already. 2048 is a strong limit for many functions.
> >>>> It breaks already as soon as some buffer has a size of PATH_MAX, but
> >>>> thats handleable. But there are some structs around that are very large.
> >>> There are surprisingly few "char [PATH_MAX]" variables left in QEMU - we
> >>> should have a final push to eliminate them regardless.
> >>>
> >>>> Generally, it would be a good idea to have a global limit, of course.
> >>> We could at least put a limit on that matches the current worst case to
> >>> prevent it getting worse than it already is.
> >> That would be a good idea, yes.
> >>
> >> How would you handle the override for a smaller -Wstack-usage ?
> > If you have multiple -Wstack-size=$XXX flags to GCC, I expect the last
> > one wins. So just need to double check that the per-object file CFLAGS
> > occur after the global CFLAS in the compiler args
>
> I will check that, thanks.
>
> When I am at it, what would be the proper replacement for char[PATH_MAX] ?
Generally code should dynamically allocate paths. If they need to sprintf
a path, then g_strdup_printf() is the right approach.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 12:06 ` Peter Lieven
2018-02-22 12:16 ` Daniel P. Berrangé
@ 2018-02-22 12:19 ` Kevin Wolf
1 sibling, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2018-02-22 12:19 UTC (permalink / raw)
To: Peter Lieven
Cc: Daniel P. Berrangé, Paolo Bonzini, jsnow, qemu-devel, qemu block
Am 22.02.2018 um 13:06 hat Peter Lieven geschrieben:
> Am 22.02.2018 um 13:03 schrieb Daniel P. Berrangé:
> > On Thu, Feb 22, 2018 at 01:02:05PM +0100, Peter Lieven wrote:
> >> Am 22.02.2018 um 13:00 schrieb Daniel P. Berrangé:
> >>> On Thu, Feb 22, 2018 at 12:51:58PM +0100, Peter Lieven wrote:
> >>>> Am 22.02.2018 um 12:40 schrieb Daniel P. Berrangé:
> >>>>> On Thu, Feb 22, 2018 at 12:32:04PM +0100, Kevin Wolf wrote:
> >>>>>> Am 22.02.2018 um 12:01 hat Peter Lieven geschrieben:
> >>>>>>> Am 22.02.2018 um 11:57 schrieb Kevin Wolf:
> >>>>>>>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
> >>>>>>>>> On 20/02/2018 18:04, Peter Lieven wrote:
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> I remember we discussed a long time ago to limit the stack usage of all
> >>>>>>>>>> functions that are executed in a coroutine
> >>>>>>>>>> context to a very low value to be able to safely limit the coroutine
> >>>>>>>>>> stack size as well.
> >>>>>>>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
> >>>>>>>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
> >>>>>>>>> ide_atapi_cmd_reply_end.
> >>>>>>>>>
> >>>>>>>>> But perhaps it's not an issue, somebody needs to audit the code.
> >>>>>>>> I think John intended to get rid of the recursion sometime, but I doubt
> >>>>>>>> he has had the time so far.
> >>>>>>> Apart from this is is possible to define special cflags in the
> >>>>>>> Makefile.objs just for a subdirectory? I have patches ready to make
> >>>>>>> the block layer files and other coroutine users compile with
> >>>>>>> -Wstack-size=2048. But I do not want to specify each file separately.
> >>>>>> Our Makefiles have lines like this:
> >>>>>>
> >>>>>> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
> >>>>>>
> >>>>>> I don't think there is a direct mechanism to apply cflags to a whole
> >>>>>> directory or just to block-obj-y/block-obj-m, but just looping over them
> >>>>>> could work. I'm not a Makefile expert at all, but after some toying with
> >>>>>> a simple example, something like this might work:
> >>>>>>
> >>>>>> $(foreach x,$(block-obj-y),$(eval $x-cflags += -Wstack-size=2048))
> >>>>> You'll need it for anything block layer depends on too - so that's much
> >>>>> of util/, crypto/ and io/ directories at least.
> >>>>>
> >>>>> So perhaps it would be shorter if we do the opposite - set -Wstack-size=2048
> >>>>> globally for everything in QEMU, and then override -Wstack-size=$BIGGER
> >>>>> for the (hopefully) few sources that have a larger stack need ?
> >>>> I tried that already. 2048 is a strong limit for many functions.
> >>>> It breaks already as soon as some buffer has a size of PATH_MAX, but
> >>>> thats handleable. But there are some structs around that are very large.
> >>> There are surprisingly few "char [PATH_MAX]" variables left in QEMU - we
> >>> should have a final push to eliminate them regardless.
> >>>
> >>>> Generally, it would be a good idea to have a global limit, of course.
> >>> We could at least put a limit on that matches the current worst case to
> >>> prevent it getting worse than it already is.
> >> That would be a good idea, yes.
> >>
> >> How would you handle the override for a smaller -Wstack-usage ?
> > If you have multiple -Wstack-size=$XXX flags to GCC, I expect the last
> > one wins. So just need to double check that the per-object file CFLAGS
> > occur after the global CFLAS in the compiler args
>
> I will check that, thanks.
>
> When I am at it, what would be the proper replacement for char[PATH_MAX] ?
g_malloc() with the exact size?
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 10:57 ` Kevin Wolf
2018-02-22 11:01 ` Peter Lieven
@ 2018-02-22 17:06 ` John Snow
2018-02-23 11:17 ` Paolo Bonzini
1 sibling, 1 reply; 19+ messages in thread
From: John Snow @ 2018-02-22 17:06 UTC (permalink / raw)
To: Kevin Wolf, Paolo Bonzini; +Cc: Peter Lieven, qemu block, qemu-devel
On 02/22/2018 05:57 AM, Kevin Wolf wrote:
> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>> On 20/02/2018 18:04, Peter Lieven wrote:
>>> Hi,
>>>
>>> I remember we discussed a long time ago to limit the stack usage of all
>>> functions that are executed in a coroutine
>>> context to a very low value to be able to safely limit the coroutine
>>> stack size as well.
>>
>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>> ide_atapi_cmd_reply_end.
>>
>> But perhaps it's not an issue, somebody needs to audit the code.
>
> I think John intended to get rid of the recursion sometime, but I doubt
> he has had the time so far.
>
It hasn't been a priority for me.
Paolo tried to fix ATAPI by adding a BH callback, but that added the
possibility of a migration halfway through a data transfer IIRC.
If anyone wants to tackle it, I'll dig up Paolo's patches.
--js
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-22 17:06 ` John Snow
@ 2018-02-23 11:17 ` Paolo Bonzini
0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2018-02-23 11:17 UTC (permalink / raw)
To: John Snow, Kevin Wolf; +Cc: Peter Lieven, qemu block, qemu-devel
On 22/02/2018 18:06, John Snow wrote:
>
>
> On 02/22/2018 05:57 AM, Kevin Wolf wrote:
>> Am 20.02.2018 um 22:54 hat Paolo Bonzini geschrieben:
>>> On 20/02/2018 18:04, Peter Lieven wrote:
>>>> Hi,
>>>>
>>>> I remember we discussed a long time ago to limit the stack usage of all
>>>> functions that are executed in a coroutine
>>>> context to a very low value to be able to safely limit the coroutine
>>>> stack size as well.
>>>
>>> IIRC the only issue was that hw/ide/atapi.c has mutual recursion between
>>> ide_atapi_cmd_reply_end -> ide_transfer_start -> ahci_start_transfer ->
>>> ide_atapi_cmd_reply_end.
>>>
>>> But perhaps it's not an issue, somebody needs to audit the code.
>>
>> I think John intended to get rid of the recursion sometime, but I doubt
>> he has had the time so far.
>>
>
> It hasn't been a priority for me.
>
> Paolo tried to fix ATAPI by adding a BH callback, but that added the
> possibility of a migration halfway through a data transfer IIRC.
>
> If anyone wants to tackle it, I'll dig up Paolo's patches.
A better possibility is to make it into tail recursion first and then a
while loop. Maybe introducing some kind of ide_transfer_start_norecurse
that returns "true" if you have a start_transfer callback (so you need
to do another iteration immediately) and "false" if you don't. I'll
take a look...
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-02-20 17:04 [Qemu-devel] Limiting coroutine stack usage Peter Lieven
2018-02-20 21:54 ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
@ 2018-03-06 11:51 ` Stefan Hajnoczi
2018-03-07 20:36 ` Peter Lieven
1 sibling, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2018-03-06 11:51 UTC (permalink / raw)
To: Peter Lieven; +Cc: qemu block, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]
On Tue, Feb 20, 2018 at 06:04:02PM +0100, Peter Lieven wrote:
> I remember we discussed a long time ago to limit the stack usage of all functions that are executed in a coroutine
> context to a very low value to be able to safely limit the coroutine stack size as well.
>
> I checked through all functions in block/, migration/ and nbd/ and there are only very few larger or unbound stack
> allocations that can easily be fixed.
>
> Now my question: Is there an easy way to add a cflag like -Wstack-usage=2048 to all objects in a given directory only?
> I tried to add a llimit to the whole project, but fixing this will be a larger task.
2KB is fine for QEMU code but actual coroutine stack sizes will have to
be at least 8KB, I guess, in order for third-party libraries to work
(e.g. curl, rbd). PATH_MAX is 4KB on Linux.
Nested event loops in QEMU code can also result in deep call stacks.
This happens when aio_poll() invokes an fd handler or BH that also
invokes aio_poll().
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-03-06 11:51 ` Stefan Hajnoczi
@ 2018-03-07 20:36 ` Peter Lieven
2018-03-08 17:27 ` Stefan Hajnoczi
0 siblings, 1 reply; 19+ messages in thread
From: Peter Lieven @ 2018-03-07 20:36 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu block, qemu-devel
Am 06.03.2018 um 12:51 schrieb Stefan Hajnoczi:
> On Tue, Feb 20, 2018 at 06:04:02PM +0100, Peter Lieven wrote:
>> I remember we discussed a long time ago to limit the stack usage of all functions that are executed in a coroutine
>> context to a very low value to be able to safely limit the coroutine stack size as well.
>>
>> I checked through all functions in block/, migration/ and nbd/ and there are only very few larger or unbound stack
>> allocations that can easily be fixed.
>>
>> Now my question: Is there an easy way to add a cflag like -Wstack-usage=2048 to all objects in a given directory only?
>> I tried to add a llimit to the whole project, but fixing this will be a larger task.
> 2KB is fine for QEMU code but actual coroutine stack sizes will have to
> be at least 8KB, I guess, in order for third-party libraries to work
> (e.g. curl, rbd). PATH_MAX is 4KB on Linux.
>
> Nested event loops in QEMU code can also result in deep call stacks.
> This happens when aio_poll() invokes an fd handler or BH that also
> invokes aio_poll().
The plan was to limit the stack usage only as a compiler option. I would leave the coroutine stack size at 1MB
for now until we have a way to identify the worst case usage.
Peter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] Limiting coroutine stack usage
2018-03-07 20:36 ` Peter Lieven
@ 2018-03-08 17:27 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2018-03-08 17:27 UTC (permalink / raw)
To: Peter Lieven; +Cc: qemu block, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]
On Wed, Mar 07, 2018 at 09:36:38PM +0100, Peter Lieven wrote:
> Am 06.03.2018 um 12:51 schrieb Stefan Hajnoczi:
> > On Tue, Feb 20, 2018 at 06:04:02PM +0100, Peter Lieven wrote:
> >> I remember we discussed a long time ago to limit the stack usage of all functions that are executed in a coroutine
> >> context to a very low value to be able to safely limit the coroutine stack size as well.
> >>
> >> I checked through all functions in block/, migration/ and nbd/ and there are only very few larger or unbound stack
> >> allocations that can easily be fixed.
> >>
> >> Now my question: Is there an easy way to add a cflag like -Wstack-usage=2048 to all objects in a given directory only?
> >> I tried to add a llimit to the whole project, but fixing this will be a larger task.
> > 2KB is fine for QEMU code but actual coroutine stack sizes will have to
> > be at least 8KB, I guess, in order for third-party libraries to work
> > (e.g. curl, rbd). PATH_MAX is 4KB on Linux.
> >
> > Nested event loops in QEMU code can also result in deep call stacks.
> > This happens when aio_poll() invokes an fd handler or BH that also
> > invokes aio_poll().
>
> The plan was to limit the stack usage only as a compiler option. I would leave the coroutine stack size at 1MB
> for now until we have a way to identify the worst case usage.
I'm not sure we'll be able to confidently set a small stack size, but a
compile-time check doesn't hurt.
Maybe someday we'll switch to stackless coroutines. Clang now has
coroutines support and maybe gcc will get it too (for C++20). If the
compiler transforms the code then coroutine state can be kept in a
smaller data area that is not a call stack. A single per-thread call
stack is used to run any coroutine and only during coroutine execution.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-03-08 17:27 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-20 17:04 [Qemu-devel] Limiting coroutine stack usage Peter Lieven
2018-02-20 21:54 ` [Qemu-devel] [Qemu-block] " Paolo Bonzini
2018-02-22 10:57 ` Kevin Wolf
2018-02-22 11:01 ` Peter Lieven
2018-02-22 11:32 ` Kevin Wolf
2018-02-22 11:40 ` Daniel P. Berrangé
2018-02-22 11:51 ` Peter Lieven
2018-02-22 12:00 ` Daniel P. Berrangé
2018-02-22 12:02 ` Peter Lieven
2018-02-22 12:03 ` Daniel P. Berrangé
2018-02-22 12:06 ` Peter Lieven
2018-02-22 12:16 ` Daniel P. Berrangé
2018-02-22 12:19 ` Kevin Wolf
2018-02-22 11:42 ` Peter Lieven
2018-02-22 17:06 ` John Snow
2018-02-23 11:17 ` Paolo Bonzini
2018-03-06 11:51 ` Stefan Hajnoczi
2018-03-07 20:36 ` Peter Lieven
2018-03-08 17:27 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.