Messages in this thread | | | Date | Mon, 17 Oct 2011 12:35:34 -0700 (PDT) | From | David Rientjes <> | Subject | Re: [PATCH] drivers, vmw_balloon.c: Increment alloc and sleep_alloc only when page allocation succeeds. |
| |
On Tue, 18 Oct 2011, Rakib Mullick wrote:
> While doing allocation statistics in vmballoon_reserve_page function, > alloc and sleep_alloc has been incremented even if allocation fails. > But, > b->stats.alloc and b->stats.sleep_alloc supposed to increment only when > they succeed. This patch makes sure that, alloc and sleep_alloc gets > incremented when page allocation succeeds. >
Dmitry could say for sure, but this seems to actually change the semantics. If the allocations fail, it increments alloc_fail and sleep_alloc_fail accordingly so you could easily see 10 alloc and 5 alloc_fail. With your patch, it would be 5 alloc and 5 alloc_fail.
I don't know which one is best, but I would opt to stay with the semantics that alloc and sleep_alloc have already had rather than changing them.
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com> > --- > > diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c > index 053d36c..d2c25c9 100644 > --- a/drivers/misc/vmw_balloon.c > +++ b/drivers/misc/vmw_balloon.c > @@ -414,11 +414,6 @@ static int vmballoon_reserve_page(struct vmballoon *b, bool can_sleep) > bool locked = false; > > do { > - if (!can_sleep) > - STATS_INC(b->stats.alloc); > - else > - STATS_INC(b->stats.sleep_alloc); > - > flags = can_sleep ? VMW_PAGE_ALLOC_CANSLEEP : VMW_PAGE_ALLOC_NOSLEEP; > page = alloc_page(flags); > if (!page) { > @@ -427,6 +422,11 @@ static int vmballoon_reserve_page(struct vmballoon *b, bool can_sleep) > else > STATS_INC(b->stats.sleep_alloc_fail); > return -ENOMEM; > + } else { > + if (!can_sleep) > + STATS_INC(b->stats.alloc); > + else > + STATS_INC(b->stats.sleep_alloc); > } > > /* inform monitor */
| |