From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756254Ab2BUXD3 (ORCPT ); Tue, 21 Feb 2012 18:03:29 -0500 Received: from mail-pz0-f46.google.com ([209.85.210.46]:41930 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754676Ab2BUXD2 (ORCPT ); Tue, 21 Feb 2012 18:03:28 -0500 Authentication-Results: mr.google.com; spf=pass (google.com: domain of hughd@google.com designates 10.68.196.234 as permitted sender) smtp.mail=hughd@google.com; dkim=pass header.i=hughd@google.com Date: Tue, 21 Feb 2012 15:03:03 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: KAMEZAWA Hiroyuki cc: Andrew Morton , Konstantin Khlebnikov , Johannes Weiner , Ying Han , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/10] mm/memcg: take care over pc->mem_cgroup In-Reply-To: <20120221181321.637556cd.kamezawa.hiroyu@jp.fujitsu.com> Message-ID: References: <20120221181321.637556cd.kamezawa.hiroyu@jp.fujitsu.com> User-Agent: Alpine 2.00 (LSU 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 21 Feb 2012, KAMEZAWA Hiroyuki wrote: > On Mon, 20 Feb 2012 15:34:28 -0800 (PST) > Hugh Dickins wrote: > return NULL; > > > > + lruvec = page_lock_lruvec(page); > > lock_page_cgroup(pc); > > > > Do we need to take lrulock+irq disable per page in this very very hot path ? I'm sure we don't want to: I hope you were pleased to find it goes away (from most cases) a couple of patches later. I had lruvec lock nested inside page_cgroup lock in the rollup I sent in December, whereas you went for page_cgroup lock nested inside lruvec lock in your lrucare patch. I couldn't find an imperative reason why they should be one way round or the other, so I tried hard to stick with your ordering, and it did work (in this 6/10). But then I couldn't work out how to get rid of the overheads added in doing it this way round, so swapped them back. > > Hmm.... How about adding NR_ISOLATED counter into lruvec ? > > Then, we can delay freeing lruvec until all conunters goes down to zero. > as... > > bool we_can_free_lruvec = true; > > lock_lruvec(lruvec->lock); > for_each_lru_lruvec(lru) > if (!list_empty(&lruvec->lru[lru])) > we_can_free_lruvec = false; > if (lruvec->nr_isolated) > we_can_free_lruvec = false; > unlock_lruvec(lruvec) > if (we_can_free_lruvec) > kfree(lruvec); > > If compaction, lumpy reclaim free a page taken from LRU, > it knows what it does and can decrement lruvec->nr_isolated properly > (it seems zone's NR_ISOLATED is decremented at putback.) At the moment I'm thinking that what we end up with by 9/10 is better than adding such a refcount. But I'm not entirely happy with mem_cgroup_reset_uncharged_to_root (it adds a further page_cgroup lookup just after I got rid of some others), and need yet to think about the race which Konstantin posits, so all options remain open. Hugh