Received: by 2002:a25:4158:0:0:0:0:0 with SMTP id o85csp999649yba; Thu, 9 May 2019 09:08:02 -0700 (PDT) X-Google-Smtp-Source: APXvYqzWvEAsV13gNkJwByDQwYS61/o/vsx/Wt0ofMvgwAdk+K2RmoiJXsqRVrepqz37yuKO/nPT X-Received: by 2002:a63:117:: with SMTP id 23mr6627515pgb.34.1557418081954; Thu, 09 May 2019 09:08:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1557418081; cv=none; d=google.com; s=arc-20160816; b=byyamWQ//zm9/DXuPux9XwjyJ7EXUWnhGkoNYamYH3fOy7RZBhVy587cVxF//w7CF8 8RYpzbBooUx0YhCPfswuFfwS4Zf2Hjf/R6XXP/aZrf5uUlfozdtU5P/YXWV6PdTYExYX 2citsaxLg699I7i6SRrUMMqkmQQBFS1El8iRe/7KCOV14VlKP9MIAwm2NrKnLVkRSvCz OxiXqVj2i9lmnWJV7/DC8sgplIuVwMsO384XCe8cjHxPkajrqrRYvK4GDzxa2EgVtBVv r0EorAellxa6yDT0Fa+wuMAckwY2rQBbj7nb0jgVLSN4jIOAG7c37e7tGX7SY1BuOJQS V5Dg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=2bZRaBuaJVzOagPgAxGnwl37KeIxRpAUuki7qHiwS7w=; b=JpiZUwo235cWPScsE6PgCT+2G9IGDKSumPNvjjKlzRLuoXosGhtP7dsgUZSqd6o1bj sV2w9qqQUIbXNqw5UrJo0youkHu1V43e65sB87N+F27O5tlx7CBiBlJo8Vm41yoQgiQO p0lEuBObJuCKXrhBVkSKh268h2AmpAp1KKPavUlZyx7ZRtQ/TYeM1BhLbzyiLSIZ/04R 8jVMFq+hbaqZcKqrEruTaBFwKDXWQPhSooVvMmF9esj6uJrkiyDCu0STndW4uCtyUZLt iCQFXGDhwNZpPzPDXQXeXKqLPxGl4Tn+VGxxSUEfxAZQTfTGtH+auc58pjZJnn9XORNJ /Mtw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=lQJMF1J3; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j23si3456989pgg.541.2019.05.09.09.07.44; Thu, 09 May 2019 09:08:01 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=lQJMF1J3; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726711AbfEIQGN (ORCPT + 99 others); Thu, 9 May 2019 12:06:13 -0400 Received: from mail-yw1-f65.google.com ([209.85.161.65]:41559 "EHLO mail-yw1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726560AbfEIQGM (ORCPT ); Thu, 9 May 2019 12:06:12 -0400 Received: by mail-yw1-f65.google.com with SMTP id o65so2282602ywd.8 for ; Thu, 09 May 2019 09:06:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2bZRaBuaJVzOagPgAxGnwl37KeIxRpAUuki7qHiwS7w=; b=lQJMF1J3Hq9E2jyZLMr011Qppcx+H+511s6SILzdH2F5jrBYvd7nkNxUY1FX0ESHU7 EgKbptxzbUfCTPygnZGyReLUb6p9aZ5h7qv0WnVnKliRI4f8Lurl9Ge0Vs/nEGhf/jko JbCaUPv2yMdy3be/pvaZJp7d9vfwtZ5/67SB5faK9ry2CYsF92ymF5Dq6lJLWefmso/h 1D1bPpKvuwR5vDBgRn24Uo/XG2Ix49bJ+Kw+A9NggKf4vPZougpgmSRsEstw0q4KVhn6 WCWdjAJfCFckonpFRaVdI99ABUANiwAVNrk3EwUJS/3kNJTH/mlTAc2Ln+sA8c/Ma63n 5Uig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2bZRaBuaJVzOagPgAxGnwl37KeIxRpAUuki7qHiwS7w=; b=hXtxd1W3/W7UOVD0ng3MaoGWlO9QyvRFE2c6Y1MWK1IZEBFHpX5k0x6aqK9+fvKSP3 3eJ7Z9JJDxTx/ttrmzYxtlMq3j+SmE/PnH948m1TCA4JNrAkPqs8sKq5vhRo0l3zKOvU 2DcVA+A1N1P52xkaM62vDSw3EF0+TuwRZGYXpuhNRs2NSlntR4O5qSmeBRcwGozBg7zj ujOjWlkGojtgpOdFvUe+weR5HeLiQsGAKMyPOGV8LxRyVER+eqNGtpNDf+4588HU3DSK ZnB69w3hmwHrrgOytjPO5lJCuBysYAnl3YYE6BYcW2yMe0In7yXN8JQH7VwaDs9McKhi 7Iqg== X-Gm-Message-State: APjAAAXhtVRFm/1ej0ZRNmh5Pjb+0Miwx9asWgCWat7qXn9toDt+6rEn oOOhbMLVr0yNnKvxWdANsdoB7bMhrEz4XAsZrCIYqw== X-Received: by 2002:a25:6708:: with SMTP id b8mr2579929ybc.377.1557417970993; Thu, 09 May 2019 09:06:10 -0700 (PDT) MIME-Version: 1.0 References: <359d98e6-044a-7686-8522-bdd2489e9456@suse.cz> <20190429105939.11962-1-jslaby@suse.cz> <20190509122526.ck25wscwanooxa3t@esperanza> In-Reply-To: <20190509122526.ck25wscwanooxa3t@esperanza> From: Shakeel Butt Date: Thu, 9 May 2019 09:05:59 -0700 Message-ID: Subject: Re: [PATCH] memcg: make it work on sparse non-0-node systems To: Vladimir Davydov Cc: Jiri Slaby , Linux MM , LKML , Johannes Weiner , Michal Hocko , Cgroups , Raghavendra K T Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 9, 2019 at 5:25 AM Vladimir Davydov wrote: > > On Mon, Apr 29, 2019 at 12:59:39PM +0200, Jiri Slaby wrote: > > We have a single node system with node 0 disabled: > > Scanning NUMA topology in Northbridge 24 > > Number of physical nodes 2 > > Skipping disabled node 0 > > Node 1 MemBase 0000000000000000 Limit 00000000fbff0000 > > NODE_DATA(1) allocated [mem 0xfbfda000-0xfbfeffff] > > > > This causes crashes in memcg when system boots: > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 > > #PF error: [normal kernel read fault] > > ... > > RIP: 0010:list_lru_add+0x94/0x170 > > ... > > Call Trace: > > d_lru_add+0x44/0x50 > > dput.part.34+0xfc/0x110 > > __fput+0x108/0x230 > > task_work_run+0x9f/0xc0 > > exit_to_usermode_loop+0xf5/0x100 > > > > It is reproducible as far as 4.12. I did not try older kernels. You have > > to have a new enough systemd, e.g. 241 (the reason is unknown -- was not > > investigated). Cannot be reproduced with systemd 234. > > > > The system crashes because the size of lru array is never updated in > > memcg_update_all_list_lrus and the reads are past the zero-sized array, > > causing dereferences of random memory. > > > > The root cause are list_lru_memcg_aware checks in the list_lru code. > > The test in list_lru_memcg_aware is broken: it assumes node 0 is always > > present, but it is not true on some systems as can be seen above. > > > > So fix this by checking the first online node instead of node 0. > > > > Signed-off-by: Jiri Slaby > > Cc: Johannes Weiner > > Cc: Michal Hocko > > Cc: Vladimir Davydov > > Cc: > > Cc: > > Cc: Raghavendra K T > > --- > > mm/list_lru.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/mm/list_lru.c b/mm/list_lru.c > > index 0730bf8ff39f..7689910f1a91 100644 > > --- a/mm/list_lru.c > > +++ b/mm/list_lru.c > > @@ -37,11 +37,7 @@ static int lru_shrinker_id(struct list_lru *lru) > > > > static inline bool list_lru_memcg_aware(struct list_lru *lru) > > { > > - /* > > - * This needs node 0 to be always present, even > > - * in the systems supporting sparse numa ids. > > - */ > > - return !!lru->node[0].memcg_lrus; > > + return !!lru->node[first_online_node].memcg_lrus; > > } > > > > static inline struct list_lru_one * > > Yep, I didn't expect node 0 could ever be unavailable, my bad. > The patch looks fine to me: > > Acked-by: Vladimir Davydov > > However, I tend to agree with Michal that (ab)using node[0].memcg_lrus > to check if a list_lru is memcg aware looks confusing. I guess we could > simply add a bool flag to list_lru instead. Something like this, may be: > I think the bool flag approach is much better. No assumption on the node initialization. If we go with bool approach then add Reviewed-by: Shakeel Butt > diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h > index aa5efd9351eb..d5ceb2839a2d 100644 > --- a/include/linux/list_lru.h > +++ b/include/linux/list_lru.h > @@ -54,6 +54,7 @@ struct list_lru { > #ifdef CONFIG_MEMCG_KMEM > struct list_head list; > int shrinker_id; > + bool memcg_aware; > #endif > }; > > diff --git a/mm/list_lru.c b/mm/list_lru.c > index 0730bf8ff39f..8e605e40a4c6 100644 > --- a/mm/list_lru.c > +++ b/mm/list_lru.c > @@ -37,11 +37,7 @@ static int lru_shrinker_id(struct list_lru *lru) > > static inline bool list_lru_memcg_aware(struct list_lru *lru) > { > - /* > - * This needs node 0 to be always present, even > - * in the systems supporting sparse numa ids. > - */ > - return !!lru->node[0].memcg_lrus; > + return lru->memcg_aware; > } > > static inline struct list_lru_one * > @@ -451,6 +447,7 @@ static int memcg_init_list_lru(struct list_lru *lru, bool memcg_aware) > { > int i; > > + lru->memcg_aware = memcg_aware; > if (!memcg_aware) > return 0; >