linux-kernel - Re: [PATCH] Add file based RSS accounting for memory resource controller (v2)

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, 17 Apr 2009 07:10:42 +0530
From:	Balbir Singh <balbir@...ux.vnet.ibm.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Add file based RSS accounting for memory resource
	controller (v2)

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> [2009-04-17 09:14:59]:

> On Thu, 16 Apr 2009 17:33:16 +0530
> Balbir Singh <balbir@...ux.vnet.ibm.com> wrote:
> 
> > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> [2009-04-16 17:15:35]:
> > 
> > > 
> > > > Sorry, some troubles found. Ignore above Ack. 3points now.
> > > > 
> > > > 1. get_cpu should be after (*)
> > > > ==mem_cgroup_update_mapped_file_stat()
> > > > +	int cpu = get_cpu();
> > > > +
> > > > +	if (!page_is_file_cache(page))
> > > > +		return;
> > > > +
> > > > +	if (unlikely(!mm))
> > > > +		mm = &init_mm;
> > > > +
> > > > +	mem = try_get_mem_cgroup_from_mm(mm);
> > > > +	if (!mem)
> > > > +		return;
> > > > + ----------------------------------------(*)
> > > > +	stat = &mem->stat;
> > > > +	cpustat = &stat->cpustat[cpu];
> > > > +
> > > > +	__mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, val);
> > > > +	put_cpu();
> > > > +}
> > > > ==
> > 
> > Yes or I should have a goto
> > 
> > > > 
> > > > 2. In above, "mem" shouldn't be got from "mm"....please get "mem" from page_cgroup.
> > > > (Because it's file cache, pc->mem_cgroup is not NULL always.)
> > 
> > Hmmm.. not sure I understand this part. Are you suggesting that mm can
> > be NULL?
> No.
> 
> > I added the check for !mm as a safety check. Since this
> > routine is only called from rmap context, mm is not NULL, hence mem
> > should not be NULL. Did you find a race between mm->owner assignment
> > and lookup via mm->owner?
> > 
> No.
> 
> page_cgroup->mem_cgroup != try_get_mem_cgroup_from_mm(mm);  in many many cases.
> 
> For example, libc and /bin/*** is tend to be loaded into default cgroup at boot but
> used by many cgroups. But mapcount of page caches for /bin/*** is 0 if not running.
> 
> Then, File_Mapped can be greater than Cached easily if you use mm->owner.
> 
> I can't estimate RSS in *my* cgroup if File_Mapped includes pages which is under 
> other cgroups. It's meaningless.
> Especially, when Cached==0 but File_Mapped > 0, I think "oh, the kernel leaks somehing..hmm..."
> 
> By useing page_cgroup->mem_cgroup, we can avoid above mess.

Yes, I see your point. I wanted mapped_file to show up in the cgroup
that mapped the page. But this works for me as well, but that means
we'll nest the page cgroup lock under the PTE lock.

-- 
	Balbir
--
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/