lkml.org 
[lkml]   [2009]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [patch 14/48] KVM: Reduce kvm stack usage in kvm_arch_vm_ioctl()
From
Date
On Sun, 2009-09-06 at 07:47 +0200, Pavel Machek wrote:
> On Fri 2009-09-04 13:07:26, Greg KH wrote:
> > On my machine with gcc 3.4, kvm uses ~2k of stack in a few
> > select functions. This is mostly because gcc fails to
> > notice that the different case: statements could have their
> > stack usage combined. It overflows very nicely if interrupts
> > happen during one of these large uses.
> >
> > This patch uses two methods for reducing stack usage.
> > 1. dynamically allocate large objects instead of putting
> > on the stack.
> > 2. Use a union{} member for all of the case variables. This
> > tricks gcc into combining them all into a single stack
> > allocation. (There's also a comment on this)
>
> Are the 'reduce stack usage' patches suitable for stable? The rules
> said that fix must be for 'serious problem', not 'theoretical
> issue'...

I guess some context got dropped at some point. I was getting really
consistent oopses and goofy memory corruption when running KVM:

http://lkml.org/lkml/2008/3/25/340

Not theoretical at all. I think it cost me a few new gray hairs.

> > Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
> > Signed-off-by: Avi Kivity <avi@qumranet.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> > ---
> > arch/x86/kvm/x86.c | 72 +++++++++++++++++++++++++++++++++--------------------
> > 1 file changed, 45 insertions(+), 27 deletions(-)
> >
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1630,6 +1630,15 @@ long kvm_arch_vm_ioctl(struct file *filp
> > struct kvm *kvm = filp->private_data;
> > void __user *argp = (void __user *)arg;
> > int r = -EINVAL;
> > + /*
> > + * This union makes it completely explicit to gcc-3.x
> > + * that these two variables' stack usage should be
> > + * combined, not added together.
> > + */
> > + union {
> > + struct kvm_pit_state ps;
> > + struct kvm_memory_alias alias;
> > + } u;
> >
> > switch (ioctl) {
> > case KVM_SET_TSS_ADDR:
>
> ...plus this is really ugly hack. Just declare the variable inside the
> case block that needs it?

Do we need to give that a better comment? It's explained a bit better
here:

http://lkml.org/lkml/2008/7/17/12
http://lkml.org/lkml/2008/7/17/16

Would this comment help?

/*
* gcc-3.x will sum the stack usage of two stack variables
* if they are declared in two different case blocks. This
* union makes it explicit that their stack space can be
* shared which greatly reduces stack usage.
*/
-- Dave



\
 
 \ /
  Last update: 2009-09-06 15:45    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site