linux-kernel - Re: [PATCH 0/2] memcgroup: work better with tmpfs

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: <Pine.LNX.4.64.0712201553010.6414@blonde.wat.veritas.com>
Date:	Thu, 20 Dec 2007 16:00:22 +0000 (GMT)
From:	Hugh Dickins <hugh@...itas.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
cc:	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/2] memcgroup: work better with tmpfs

On Wed, 19 Dec 2007, KAMEZAWA Hiroyuki wrote:
> On Tue, 18 Dec 2007 22:19:22 +0000 (GMT)
> Hugh Dickins <hugh@...itas.com> wrote:
> > 1. Why is spin_lock_irqsave rather than spin_lock needed on mz->lru_lock?
> > If it is needed, doesn't mem_cgroup_isolate_pages need to use it too?
> > 
> When I wrote a patch to treat lru_lock (it was not per-zone yet.), I got a
> comment to use irqsafe version. So I wonder there is some plan which needs
> irq safe locking.

It might have come about because struct zone's lru_lock really does need
the irq disabled (because of things that happen at I/O completion time).
But I don't think struct mem_cgroup_per_zone's lru_lock needs it.

> About mem_cgroup_isolate_pages(), zone->lock is acquired with irq-disable 
> before it is called. Then, it's not necesary there.

Ah, yes, I completely missed that, thank you.

> 
> But as you say, it looks we can do it without irq-disable, now.
> 
> 
> > 2. There's mem_cgroup_charge and mem_cgroup_cache_charge (wouldn't the
> > former be better called mem_cgroup_charge_mapped? why does the latter
> > test MEM_CGROUP_TYPE_ALL instead of MEM_CGROUP_TYPE_CACHED? I still don't
> > understand your enums there).  But there's only mem_cgroup_uncharge.
> > So when, for example, an add_to_page_cache fails, the uncharge may not
> > balance the charge?
> >
> Ah...it seems bug. We should add type handling in uncharge. 
> Then, changing control_type after start using mem_cgroup seems dangerous.
> (Default is ALL now.)
> 
> Maybe following will be fix.
> 
> - allow changing contorl_type only when there is no task.
> - run force_empty when control_type is changed. and drop all charges.
> 
> This will change current behavior but I think it's reasonable.
> How do you think ?

Perhaps... my grasp on this is much too weak to be sure:
I'm pretty sceptical about changing control_type altogether.
Let me reply to Balbir, who still sees no issue there.

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