On Tue, 22 Mar 2011 08:31:50 +0100 Michal Hocko <mhocko@xxxxxxx> wrote: > On Tue 22-03-11 10:47:23, Daisuke Nishimura wrote: > > On Tue, 22 Mar 2011 09:10:14 +0900 > > KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote: > > > > > On Mon, 21 Mar 2011 11:24:20 +0100 > > > Michal Hocko <mhocko@xxxxxxx> wrote: > > > > > > > [Sorry for reposting but I forgot to fully refresh the patch before > > > > posting...] > > > > > > > > On Mon 21-03-11 10:34:19, Michal Hocko wrote: > > > > > On Fri 18-03-11 16:25:32, Michal Hocko wrote: > > > > > [...] > > > > > > According to our documention this is a reasonable test case: > > > > > > Documentation/cgroups/memory.txt: > > > > > > memory.usage_in_bytes # show current memory(RSS+Cache) usage. > > > > > > > > > > > > This however doesn't work after your commit: > > > > > > cdec2e4265d (memcg: coalesce charging via percpu storage) > > > > > > > > > > > > because since then we are charging in bulks so we can end up with > > > > > > rss+cache <= usage_in_bytes. > > > > > [...] > > > > > > I think we have several options here > > > > > > 1) document that the value is actually >= rss+cache and it shows > > > > > > the guaranteed charges for the group > > > > > > 2) use rss+cache rather then res->count > > > > > > 3) remove the file > > > > > > 4) call drain_all_stock_sync before asking for the value in > > > > > > mem_cgroup_read > > > > > > 5) collect the current amount of stock charges and subtract it > > > > > > from the current res->count value > > > > > > > > > > > > 1) and 2) would suggest that the file is actually not very much useful. > > > > > > 3) is basically the interface change as well > > > > > > 4) sounds little bit invasive as we basically lose the advantage of the > > > > > > pool whenever somebody reads the file. Btw. for who is this file > > > > > > intended? > > > > > > 5) sounds like a compromise > > > > > > > > > > I guess that 4) is really too invasive - for no good reason so here we > > > > > go with the 5) solution. > > > > > > I think the test in LTP is bad...(it should be fuzzy.) because we cannot > > > avoid races... > > I agree. > > I think that as well. In fact, I quite do not understand what it is > testing actually (that we account charges correctly? If yes then what if > both values are wrong?). The other point is, though, we have exported this > interface and documented its semantic. This is the reason I have asked > for the initial purpose of the file in the first place. Is this > something for debugging only? Can we make use of the value somehow > (other than a shortcut for rss+cache)? > > If there is realy no strong reason for the file existence I would rather > vote for its removing than having a unusable semantic. > Considering more, without these files, we cannot know the actual usage of a res_counter, although we set a limit to a res_counter. So, I want to keep these files. If no-one have any objections, I'll prepare a patch to update the documentation. Thanks, Daisuke Nishimura. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>