Messages in this thread | | | Date | Tue, 18 Oct 2011 02:17:02 +0600 | Subject | Re: [PATCH] drivers, vmw_balloon.c: Increment alloc and sleep_alloc only when page allocation succeeds. | From | Rakib Mullick <> |
| |
On Tue, Oct 18, 2011 at 1:45 AM, Dmitry Torokhov <dtor@vmware.com> wrote: > On Monday, October 17, 2011 12:35:34 PM David Rientjes wrote: >> 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. > > Right, b->stats.alloc and b->stats.sleep_alloc show number of allocation > attempts and alloc_fail and sleep_alloc_fail show how many of these > attempts failed. This behavior matches behavior of the driver we have > been shipping out of the tree for many years and I would prefer to keep > it as is. > Then why b->stats.alloc and b->stats.sleep_alloc are not renamed as b->stats.alloc_attempted and b->stats.sleep_alloc_attempted respectively. Current naming is confusing, anyone looking at the alloc and sleep_alloc stat would easily think that, if 10 alloc and 5 alloc_fail then 10 has been allocated and 5 times has been allocation fails. Isn't it?
Thanks, Rakib -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |