linux-kernel - Re: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

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]

Message-ID: <20795958.1215245782985.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Sat, 5 Jul 2008 17:16:22 +0900 (JST)
From:	kamezawa.hiroyu@...fujitsu.com
To:	balbir@...ux.vnet.ibm.com
Cc:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, hugh@...itas.com,
	nishimura@....nes.nec.co.jp, yamamoto@...inux.co.jp
Subject: Re: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1

----- Original Message -----
>Date: Sat, 05 Jul 2008 12:19:11 +0530
>From: Balbir Singh <balbir@...ux.vnet.ibm.com>
>To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
>CC: Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org,
>   linux-mm@...ck.org, "hugh@...itas.com" <hugh@...itas.com>,
>   "nishimura@....nes.nec.co.jp" <nishimura@....nes.nec.co.jp>,
>   "yamamoto@...inux.co.jp" <yamamoto@...inux.co.jp>
>Subject: Re: [PATCH] memcg: handle shmem's swap cache (Was 2.6.26-rc8-mm1
>
>
>KAMEZAWA Hiroyuki wrote:
>> On Sat, 05 Jul 2008 11:11:10 +0530
>> Balbir Singh <balbir@...ux.vnet.ibm.com> wrote:
>> 
>>> KAMEZAWA Hiroyuki wrote:
>>>> My swapcache accounting under memcg patch failed to catch tmpfs(shmem)'s 
one.
>>>> Can I test this under -mm tree ?
>>>> (If -mm is busy, I'm not in hurry.)
>>>> This patch works well in my box.
>>>> =
>>>> SwapCache handling fix.
>>>>
>>>> shmem's swapcache behavior is a little different from anonymous's one and
>>>> memcg failed to handle it. This patch tries to fix it.
>>>>
>>>> After this:
>>>>
>>>> Any page marked as SwapCache is not uncharged. (delelte_from_swap_cache()
>>>> delete the SwapCache flag.)
>>>>
>>>> To check a shmem-page-cache is alive or not we use
>>>>  page->mapping && !PageAnon(page) instead of
>>>>  pc->flags & PAGE_CGROUP_FLAG_CACHE.
>>>>
>>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
>>> Though I am not opposed to this, I do sit up and think if keeping the refe
rence
>>> count around could avoid this complexity and from my point, the maintenanc
e
>>> overhead of this logic/code (I fear there might be more special cases :( )
>> 
>> yes, to me. but we have to fix..
>> 
>> But I don't like old code's refcnt handling which does
>>    - increment
>>      - does this increment was really neccesary ?
>>        No? ok, decrement it again.
>> 
>> This was much more complex to me than current code.
>> 
At first, what I have to say is
"this is a fix against handle-swapcache patch not against remove-refcnt"

This complex comes from handle-swapcache. (But it's necessary.)

>
>That can be redone -- the moment a page is used by a path, refcnt (increment)
>it. Undo the same when the page is no longer in use.
>
>I expect
>
>rmap path to increment/decrement it on mapping
>radix-tree (cache's) to do the same
>
>
>Using a kref we should be able to get this logic right - no?
>
no
What the old code does was

  - a page is added to rmap (mapcount 0->1) +1
  - a page is removed from rmap (mapcount ->0) -1
  - a page is added to radix-tree (+1)
  - a page is removed from radix-tree (-1)

All information is recorded in struct page because it exists for.
Then, why duplicates information ? It's usually bad habit.


>> And old ones will needs the check at treating swap-cache. (it couldn't but 
if we want)
>> 
>>> The trade-off is complexity versus the overhead of reference counting.
>>>
>> refcnt was also very complex ;)
>
>I think that is easier to simply, instead of adding the complex checks we hav
e
>right now. refcnt is easier to prove as working correct than the checks.

About swap-cache, refcnt is just obstacle because you can't handle
add-to-swapcache by refcnt.

If you want to add refcnt (or some code) for "debug", I have no objection.

Thanks,
-Kame


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