linux-kernel - Re: [PATCH 6/6] memcg: make mem_cgroup_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 12 Nov 2010 12:41:15 -0800
From:	Greg Thelen <gthelen@...gle.com>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Daisuke Nishimura <nishimura@....nes.nec.co.jp>,
	Wu Fengguang <fengguang.wu@...el.com>,
	Minchan Kim <minchan.kim@...il.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/6] memcg: make mem_cgroup_page_stat() return value unsigned

DeJohannes Weiner <hannes@...xchg.org> writes:

> On Tue, Nov 09, 2010 at 01:24:31AM -0800, Greg Thelen wrote:
>> mem_cgroup_page_stat() used to return a negative page count
>> value to indicate value.
>
> Whoops :)
>
>> mem_cgroup_page_stat() has changed so it never returns
>> error so convert the return value to the traditional page
>> count type (unsigned long).
>
> This changelog feels a bit beside the point.
>
> What's really interesting is that we now don't consider negative sums
> to be invalid anymore, but just assume zero!  There is a real
> semantical change here.

Prior to this patch series mem_cgroup_page_stat() returned a negative
value (specifically -EINVAL) to indicate that the current task was in
the root_cgroup and thus the per-cgroup usage and limit counter were
invalid.  Callers treated all negative values as an indication of
root-cgroup message.

Unfortunately there was another way that mem_cgroup_page_stat() could
return a negative value even when current was not in the root cgroup.
Negative sums were a possibility due to summing of unsynchronized
per-cpu counters.  These occasional negative sums would fool callers
into thinking that the current task was in the root cgroup.

Would adding this description to the commit message address your
concerns?

> That the return type can then be changed to unsigned long is a nice
> follow-up cleanup that happens to be folded into this patch.

Good point.  I can separate the change into two sub-patches:
1. use zero for a min-value (as described above)
2. change return value to unsigned
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/