From patchwork Mon Apr 23 08:28:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 8021 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 5344823E20 for ; Mon, 23 Apr 2012 08:29:54 +0000 (UTC) Received: from mail-iy0-f180.google.com (mail-iy0-f180.google.com [209.85.210.180]) by fiordland.canonical.com (Postfix) with ESMTP id F2E75A1880E for ; Mon, 23 Apr 2012 08:29:53 +0000 (UTC) Received: by iage36 with SMTP id e36so22387446iag.11 for ; Mon, 23 Apr 2012 01:29:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:date:from :to:cc:subject:message-id:references:mime-version:content-type :content-disposition:in-reply-to:user-agent:x-gm-message-state; bh=PbQE2DxrAyJWvN98Y42XK6b7e51Y8XjaLBupVQWztHM=; b=Rqt+BQsk2fFP2j/bW+cNNm9aH9gCCM4g2nbJb5YyjYlMiPh4rj9O9F10kZkUVC5L5X ZSiJy8pHKZxqpB75GHwWyZcJVGkKxr1nBO7MtBbgoMnQQVVOsVeIxgOlNQmnq1W9gnsr m50H3K7JXU/MZBi2ZiLGy4b/6xk1ejF5SA+Sx3ltSXFqchSBKfIkZfM7pWnC2M02nEXM i3J0CMcotJqEAk1M5YaVog0juim2mXLxQs9X1RjRBmiuIg3n1deUVaIkMEiPj+VqJ3Fu exZYVEVOFCDzORoYmQj5pXq1ldpOK26JkBbus/Gh8yW7fnDEDN1p8jKk0EUag9B3+L6g gPOQ== Received: by 10.43.49.201 with SMTP id vb9mr995270icb.35.1335169793450; Mon, 23 Apr 2012 01:29:53 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.137.198 with SMTP id x6csp95319ibt; Mon, 23 Apr 2012 01:29:53 -0700 (PDT) Received: by 10.182.12.6 with SMTP id u6mr22038949obb.12.1335169792905; Mon, 23 Apr 2012 01:29:52 -0700 (PDT) Received: from mail-ob0-f178.google.com (mail-ob0-f178.google.com [209.85.214.178]) by mx.google.com with ESMTPS id rz10si7966742obc.104.2012.04.23.01.29.52 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 23 Apr 2012 01:29:52 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.214.178 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) client-ip=209.85.214.178; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.214.178 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) smtp.mail=anton.vorontsov@linaro.org Received: by obbwc18 with SMTP id wc18so9155780obb.37 for ; Mon, 23 Apr 2012 01:29:52 -0700 (PDT) Received: by 10.182.164.105 with SMTP id yp9mr14464081obb.75.1335169792674; Mon, 23 Apr 2012 01:29:52 -0700 (PDT) Received: from localhost ([69.199.155.45]) by mx.google.com with ESMTPS id k7sm12349123oei.0.2012.04.23.01.29.50 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 23 Apr 2012 01:29:51 -0700 (PDT) Date: Mon, 23 Apr 2012 01:28:35 -0700 From: Anton Vorontsov To: KAMEZAWA Hiroyuki Cc: cgroups@vger.kernel.org, Johannes Weiner , Michal Hocko , Balbir Singh , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , KOSAKI Motohiro , John Stultz , linaro-kernel@lists.linaro.org, patches@linaro.org Subject: [PATCH RFC] memcg: MEMCG_NR_FILE_MAPPED should update _STAT_CACHE as well Message-ID: <20120423082835.GA32359@lizard> References: <20120302162753.GA11748@oksana.dev.rtsoft.ru> <20120305091934.588c160b.kamezawa.hiroyu@jp.fujitsu.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20120305091934.588c160b.kamezawa.hiroyu@jp.fujitsu.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQlsvH1txiwgX13D0/SLC10E76JFVTJWHkkyaFnqwVkZcOyomvGvOK2nky1Hzd+SH5qpfUaA ...otherwise the we're getting the wrong numbers in usage_in_bytes. On Mon, Mar 05, 2012 at 09:19:34AM +0900, KAMEZAWA Hiroyuki wrote: [...] > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 228d646..c8abdc5 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -3812,6 +3812,9 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *memcg, bool swap) > > > > val = mem_cgroup_recursive_stat(memcg, MEM_CGROUP_STAT_CACHE); > > val += mem_cgroup_recursive_stat(memcg, MEM_CGROUP_STAT_RSS); > > + val += mem_cgroup_recursive_stat(memcg, MEM_CGROUP_STAT_FILE_MAPPED); > > > > 1. Is there any particular reason we don't currently account file mapped > > memory in usage_in_bytes? > > > > To me, MEM_CGROUP_STAT_FILE_MAPPED hunk seems logical even if we > > don't use it for lowmemory notifications. > > > > Plus, it seems that FILE_MAPPED _is_ accounted for the non-root > > cgroups, so I guess it's clearly a bug for the root memcg? > > CACHE includes all file caches. Why do you think FILE_MAPPED is not included in CACHE ? There were tons of changes in the memcg lately, but I believe the issue is still there. For example, looking into this code flow: -> page_add_file_rmap() (mm/rmap.c) -> mem_cgroup_inc_page_stat(page, MEMCG_NR_FILE_MAPPED) (include/linux/memcontrol.h) -> void mem_cgroup_update_page_stat(page, MEMCG_NR_FILE_MAPPED, 1) (mm/memcontrol.c) And then: void mem_cgroup_update_page_stat(struct page *page, enum mem_cgroup_page_stat_item idx, int val) { ... switch (idx) { case MEMCG_NR_FILE_MAPPED: idx = MEM_CGROUP_STAT_FILE_MAPPED; break; default: BUG(); } this_cpu_add(memcg->stat->count[idx], val); ... } So, clearly, this function only bothers updating _FILE_MAPPED only, leaving _CACHE alone. If you're saying that _CACHE meant to include _FILE_MAPPED, then I guess the patch down below would be a proper fix then... Otherwise we need to be consistent on stats reporting, and either fall-back to my original fix (in mem_cgroup_usage()), or think about doing it some other way... Signed-off-by: Anton Vorontsov --- The patch is against current -next. Thanks, mm/memcontrol.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 884e936..760ecf5 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1958,6 +1958,8 @@ void mem_cgroup_update_page_stat(struct page *page, switch (idx) { case MEMCG_NR_FILE_MAPPED: + idx = MEM_CGROUP_STAT_CACHE; + this_cpu_add(memcg->stat->count[idx], val); idx = MEM_CGROUP_STAT_FILE_MAPPED; break; default: