| Message ID | 1313039267-25951-2-git-send-email-walimisdev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2011-08-11 at 13:07 +0800, Liming Wang wrote: > If num_pages is negative, balloon will make kernel crash with > "out of memory". So we check this value to avoid it to be negative. > > Signed-off-by: Liming Wang <walimisdev@gmail.com> > --- > tools/kvm/virtio/balloon.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c > index 854d04b..0223ee4 100644 > --- a/tools/kvm/virtio/balloon.c > +++ b/tools/kvm/virtio/balloon.c > @@ -222,8 +222,13 @@ static void handle_sigmem(int sig) > { > if (sig == SIGKVMADDMEM) > bdev.config.num_pages += 256; > - else > + else { > bdev.config.num_pages -= 256; > + if ((s32)bdev.config.num_pages < 0){ imo it's worth doing this check before the decrement instead of casting to signed here. you also need to wrap the 'if ()' with parenthesis if you add them to the 'else' case. > + bdev.config.num_pages = 0; > + return; > + } > + } > > /* Notify that the configuration space has changed */ > bdev.isr = VIRTIO_PCI_ISR_CONFIG;
On Thu, Aug 11, 2011 at 08:37:01AM +0300, Sasha Levin wrote: >On Thu, 2011-08-11 at 13:07 +0800, Liming Wang wrote: >> If num_pages is negative, balloon will make kernel crash with >> "out of memory". So we check this value to avoid it to be negative. >> >> Signed-off-by: Liming Wang <walimisdev@gmail.com> >> --- >> tools/kvm/virtio/balloon.c | 7 ++++++- >> 1 files changed, 6 insertions(+), 1 deletions(-) >> >> diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c >> index 854d04b..0223ee4 100644 >> --- a/tools/kvm/virtio/balloon.c >> +++ b/tools/kvm/virtio/balloon.c >> @@ -222,8 +222,13 @@ static void handle_sigmem(int sig) >> { >> if (sig == SIGKVMADDMEM) >> bdev.config.num_pages += 256; >> - else >> + else { >> bdev.config.num_pages -= 256; >> + if ((s32)bdev.config.num_pages < 0){ > >imo it's worth doing this check before the decrement instead of casting >to signed here. You mean that check whether num_pages less than 256 or equal to 0? > >you also need to wrap the 'if ()' with parenthesis if you add them to >the 'else' case. Sorry, I'm not clear what that does mean? walimis > >> + bdev.config.num_pages = 0; >> + return; >> + } >> + } >> >> /* Notify that the configuration space has changed */ >> bdev.isr = VIRTIO_PCI_ISR_CONFIG; > >-- > >Sasha. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2011-08-11 at 13:40 +0800, walimis wrote: > On Thu, Aug 11, 2011 at 08:37:01AM +0300, Sasha Levin wrote: > >On Thu, 2011-08-11 at 13:07 +0800, Liming Wang wrote: > >> If num_pages is negative, balloon will make kernel crash with > >> "out of memory". So we check this value to avoid it to be negative. > >> > >> Signed-off-by: Liming Wang <walimisdev@gmail.com> > >> --- > >> tools/kvm/virtio/balloon.c | 7 ++++++- > >> 1 files changed, 6 insertions(+), 1 deletions(-) > >> > >> diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c > >> index 854d04b..0223ee4 100644 > >> --- a/tools/kvm/virtio/balloon.c > >> +++ b/tools/kvm/virtio/balloon.c > >> @@ -222,8 +222,13 @@ static void handle_sigmem(int sig) > >> { > >> if (sig == SIGKVMADDMEM) > >> bdev.config.num_pages += 256; > >> - else > >> + else { > >> bdev.config.num_pages -= 256; > >> + if ((s32)bdev.config.num_pages < 0){ > > > >imo it's worth doing this check before the decrement instead of casting > >to signed here. > You mean that check whether num_pages less than 256 or equal to 0? > If it's less than 256. > > > >you also need to wrap the 'if ()' with parenthesis if you add them to > >the 'else' case. > Sorry, I'm not clear what that does mean? > It's a coding style thing. If you have braces in one branch you should have them in all branches. For example: if (something) do_this(); else { do_this(); do_that(); } should be: if (something) { do_this(); } else { do_this(); do_that(); } We follow the kernel coding style, you can find the full documentation under the kernel tree in Documentation/CodingStyle . > walimis > > > >> + bdev.config.num_pages = 0; > >> + return; > >> + } > >> + } > >> > >> /* Notify that the configuration space has changed */ > >> bdev.isr = VIRTIO_PCI_ISR_CONFIG; > > > >-- > > > >Sasha. > >
diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c index 854d04b..0223ee4 100644 --- a/tools/kvm/virtio/balloon.c +++ b/tools/kvm/virtio/balloon.c @@ -222,8 +222,13 @@ static void handle_sigmem(int sig) { if (sig == SIGKVMADDMEM) bdev.config.num_pages += 256; - else + else { bdev.config.num_pages -= 256; + if ((s32)bdev.config.num_pages < 0){ + bdev.config.num_pages = 0; + return; + } + } /* Notify that the configuration space has changed */ bdev.isr = VIRTIO_PCI_ISR_CONFIG;
If num_pages is negative, balloon will make kernel crash with "out of memory". So we check this value to avoid it to be negative. Signed-off-by: Liming Wang <walimisdev@gmail.com> --- tools/kvm/virtio/balloon.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)