* Re: [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages()
2010-11-09 1:15 [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages() Greg Thelen
@ 2010-11-09 1:28 ` Minchan Kim
2010-11-09 1:47 ` Johannes Weiner
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Minchan Kim @ 2010-11-09 1:28 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, Balbir Singh, KAMEZAWA Hiroyuki,
Daisuke Nishimura, Johannes Weiner, Wu Fengguang, linux-mm,
linux-kernel
On Tue, Nov 9, 2010 at 10:15 AM, Greg Thelen <gthelen@google.com> wrote:
> Use page counts rather than byte counts to avoid overflowing
> unsigned long local variables.
>
> Signed-off-by: Greg Thelen <gthelen@google.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages()
2010-11-09 1:15 [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages() Greg Thelen
2010-11-09 1:28 ` Minchan Kim
@ 2010-11-09 1:47 ` Johannes Weiner
2010-11-09 3:44 ` Daisuke Nishimura
2010-11-16 3:45 ` KAMEZAWA Hiroyuki
3 siblings, 0 replies; 6+ messages in thread
From: Johannes Weiner @ 2010-11-09 1:47 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, Balbir Singh, KAMEZAWA Hiroyuki,
Daisuke Nishimura, Wu Fengguang, linux-mm, linux-kernel
On Mon, Nov 08, 2010 at 05:15:20PM -0800, Greg Thelen wrote:
> Use page counts rather than byte counts to avoid overflowing
> unsigned long local variables.
>
> Signed-off-by: Greg Thelen <gthelen@google.com>
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages()
2010-11-09 1:15 [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages() Greg Thelen
2010-11-09 1:28 ` Minchan Kim
2010-11-09 1:47 ` Johannes Weiner
@ 2010-11-09 3:44 ` Daisuke Nishimura
2010-11-09 3:52 ` Greg Thelen
2010-11-16 3:45 ` KAMEZAWA Hiroyuki
3 siblings, 1 reply; 6+ messages in thread
From: Daisuke Nishimura @ 2010-11-09 3:44 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, Balbir Singh, KAMEZAWA Hiroyuki, Johannes Weiner,
Wu Fengguang, linux-mm, linux-kernel, Daisuke Nishimura
On Mon, 8 Nov 2010 17:15:20 -0800
Greg Thelen <gthelen@google.com> wrote:
> Use page counts rather than byte counts to avoid overflowing
> unsigned long local variables.
>
> Signed-off-by: Greg Thelen <gthelen@google.com>
> ---
> mm/memcontrol.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6c7115d..b287afd 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1345,17 +1345,17 @@ memcg_hierarchical_free_pages(struct mem_cgroup *mem)
> {
> unsigned long free, min_free;
>
hmm, the default value of RES_LIMIT is LLONG_MAX, so I think we must declare
"free" as unsinged long long to avoid overflow.
Thanks,
Daisuke Nishimura.
> - min_free = global_page_state(NR_FREE_PAGES) << PAGE_SHIFT;
> + min_free = global_page_state(NR_FREE_PAGES);
>
> while (mem) {
> - free = res_counter_read_u64(&mem->res, RES_LIMIT) -
> - res_counter_read_u64(&mem->res, RES_USAGE);
> + free = (res_counter_read_u64(&mem->res, RES_LIMIT) -
> + res_counter_read_u64(&mem->res, RES_USAGE)) >>
> + PAGE_SHIFT;
> min_free = min(min_free, free);
> mem = parent_mem_cgroup(mem);
> }
>
> - /* Translate free memory in pages */
> - return min_free >> PAGE_SHIFT;
> + return min_free;
> }
>
> /*
> --
> 1.7.3.1
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages()
2010-11-09 3:44 ` Daisuke Nishimura
@ 2010-11-09 3:52 ` Greg Thelen
0 siblings, 0 replies; 6+ messages in thread
From: Greg Thelen @ 2010-11-09 3:52 UTC (permalink / raw)
To: Daisuke Nishimura
Cc: Andrew Morton, Balbir Singh, KAMEZAWA Hiroyuki, Johannes Weiner,
Wu Fengguang, linux-mm, linux-kernel
On Mon, Nov 8, 2010 at 7:44 PM, Daisuke Nishimura
<nishimura@mxp.nes.nec.co.jp> wrote:
> On Mon, 8 Nov 2010 17:15:20 -0800
> Greg Thelen <gthelen@google.com> wrote:
>
>> Use page counts rather than byte counts to avoid overflowing
>> unsigned long local variables.
>>
>> Signed-off-by: Greg Thelen <gthelen@google.com>
>> ---
>> mm/memcontrol.c | 10 +++++-----
>> 1 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 6c7115d..b287afd 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -1345,17 +1345,17 @@ memcg_hierarchical_free_pages(struct mem_cgroup *mem)
>> {
>> unsigned long free, min_free;
>>
> hmm, the default value of RES_LIMIT is LLONG_MAX, so I think we must declare
> "free" as unsinged long long to avoid overflow.
Agreed. I am testing a fix for that issue now. I do not want
complicate this patch with the RES_LIMIT issue you mention. The fix
will be in a separate patch.
> Thanks,
> Daisuke Nishimura.
>
>> - min_free = global_page_state(NR_FREE_PAGES) << PAGE_SHIFT;
>> + min_free = global_page_state(NR_FREE_PAGES);
>>
>> while (mem) {
>> - free = res_counter_read_u64(&mem->res, RES_LIMIT) -
>> - res_counter_read_u64(&mem->res, RES_USAGE);
>> + free = (res_counter_read_u64(&mem->res, RES_LIMIT) -
>> + res_counter_read_u64(&mem->res, RES_USAGE)) >>
>> + PAGE_SHIFT;
>> min_free = min(min_free, free);
>> mem = parent_mem_cgroup(mem);
>> }
>>
>> - /* Translate free memory in pages */
>> - return min_free >> PAGE_SHIFT;
>> + return min_free;
>> }
>>
>> /*
>> --
>> 1.7.3.1
>>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages()
2010-11-09 1:15 [PATCH] memcg: avoid overflow in memcg_hierarchical_free_pages() Greg Thelen
` (2 preceding siblings ...)
2010-11-09 3:44 ` Daisuke Nishimura
@ 2010-11-16 3:45 ` KAMEZAWA Hiroyuki
3 siblings, 0 replies; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-11-16 3:45 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, Balbir Singh, Daisuke Nishimura, Johannes Weiner,
Wu Fengguang, linux-mm, linux-kernel
On Mon, 8 Nov 2010 17:15:20 -0800
Greg Thelen <gthelen@google.com> wrote:
> Use page counts rather than byte counts to avoid overflowing
> unsigned long local variables.
>
> Signed-off-by: Greg Thelen <gthelen@google.com>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread