From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69379C07E95 for ; Fri, 16 Jul 2021 12:43:26 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DEC82613D3 for ; Fri, 16 Jul 2021 12:43:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DEC82613D3 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id E2BCF8D00F5; Fri, 16 Jul 2021 08:43:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DDCA78D00EC; Fri, 16 Jul 2021 08:43:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7CEB8D00F5; Fri, 16 Jul 2021 08:43:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0120.hostedemail.com [216.40.44.120]) by kanga.kvack.org (Postfix) with ESMTP id A66C28D00EC for ; Fri, 16 Jul 2021 08:43:25 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 6A10A27770 for ; Fri, 16 Jul 2021 12:43:24 +0000 (UTC) X-FDA: 78368416728.18.316A17E Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) by imf13.hostedemail.com (Postfix) with ESMTP id 28582100C843 for ; Fri, 16 Jul 2021 12:43:24 +0000 (UTC) Received: by mail-lf1-f53.google.com with SMTP id y42so15802356lfa.3 for ; Fri, 16 Jul 2021 05:43:23 -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=UPc9GTX5VyyHJrW3MaVc/E6nclOhvbKeO0mnyiHC1zY=; b=jTDn3YWmpC/FctJCzOr8WRnUBwHnxzMHo6DqKPNmiGLy7vDPPPLB4CoOJgvlWnGd69 1Q8rZ/brO31Gt+Rzo3WlU/IubHn5fpxkY56zOl4KqpgQ4lH+DBu1idCiLO8mq8jqAntK rcEUS6HsyPDTmFBi8BsI6igmDhywar8Atu0HGjEyaxOImobB3BSWHN8v9YzMnRJbzXtE 9ugX++nQzb8nu0PCvHKczzxjDtnaAbCgbAiFQwpBXd13mnzP05IUojyinF4UlVxU9MqB IXH3jvOfnbSLwyORz8Afsk4XbTf5MY8KKDQwaEgLxms/twcMUI2wCCtvPK2yX+THlcJx zGvQ== 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=UPc9GTX5VyyHJrW3MaVc/E6nclOhvbKeO0mnyiHC1zY=; b=rY2UP4Eeip8UzF0eHhvx0cgGFWHXAxBNGv9BQgxFSGyd0afNJNHAYLJcHT01aq/YcF jMsYFc0XE60S1Tg4rWl/gY9kSWatMgBd/VBGIcGlUhnoC3yaQsxxDTo4e1WNhmxW9tOz gSK5BSpPncb3IPj6ikSrigLOC7YLglttFDTwUVhyWaU8C92+gswPgSwY9dIl922J6T9Z xAdC142QFuukTSuE/ZoWIuzXohkLM0d5Bp0Ozu8xcGpCW7F21eUT9EPm1NwSH8Jm8jKn 1pCI7So3Jx3tXbs3YL5OkCtKmZDu1sDmsNdhUj86zRuvP8c19id+mRStNRBW284vELt2 ySsg== X-Gm-Message-State: AOAM530PWq6s+vS8IgC7yRlIzCpnXG03Re+wVpSSmPqNxIVvNswveg64 jhbFww6hsc9XVnsFuQnElY4ymdAhR08so6MWfSu+tw== X-Google-Smtp-Source: ABdhPJypb+ne/bF2pi7D5wbSASimiWqxfKvpKwgf6g1mYmJZ8KQSKlgp/ZS+hS5L0JSY8vTIjhr/BfGAcCTJhlOp7lE= X-Received: by 2002:ac2:4d86:: with SMTP id g6mr7332429lfe.549.1626439402177; Fri, 16 Jul 2021 05:43:22 -0700 (PDT) MIME-Version: 1.0 References: <20210716084805.273744-1-yangyingliang@huawei.com> In-Reply-To: <20210716084805.273744-1-yangyingliang@huawei.com> From: Shakeel Butt Date: Fri, 16 Jul 2021 05:43:10 -0700 Message-ID: Subject: Re: [PATCH -next] memcg: fix sleep in invalid context in cgroup_rstat_flush() To: Yang Yingliang Cc: LKML , Cgroups , Linux MM , Johannes Weiner , Tejun Heo , Andrew Morton , Stephen Rothwell Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=jTDn3YWm; spf=pass (imf13.hostedemail.com: domain of shakeelb@google.com designates 209.85.167.53 as permitted sender) smtp.mailfrom=shakeelb@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam02 X-Stat-Signature: 9jz4iebjryse399mw67bkousneoazbkz X-Rspamd-Queue-Id: 28582100C843 X-HE-Tag: 1626439404-73786 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Jul 16, 2021 at 1:45 AM Yang Yingliang wrote: > > When start the kernel, I got this report: > > [ 13.787531][ T248] BUG: sleeping function called from invalid context at kernel/cgroup/rstat.c:200 > [ 13.788799][ T248] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 248, name: kworker/u8:3 > [ 13.789971][ T248] 3 locks held by kworker/u8:3/248: > [ 13.790638][ T248] #0: ffff888100100138 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x942/0x17d0 > [ 13.792170][ T248] #1: ffffc900018bfe00 (stats_flush_work){+.+.}-{0:0}, at: process_one_work+0x977/0x17d0 > [ 13.793477][ T248] #2: ffffffff8baccf78 (stats_flush_lock){+.+.}-{2:2}, at: mem_cgroup_flush_stats+0xd/0x50 > [ 13.794815][ T248] Preemption disabled at: > [ 13.794821][ T248] [<0000000000000000>] 0x0 > [ 13.795951][ T248] CPU: 2 PID: 248 Comm: kworker/u8:3 Tainted: G W 5.14.0-rc1-next-20210716+ #342 > [ 13.797287][ T248] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > [ 13.798564][ T248] Workqueue: events_unbound flush_memcg_stats_work > [ 13.799400][ T248] Call Trace: > [ 13.799817][ T248] dump_stack_lvl+0xcd/0x134 > [ 13.800405][ T248] ___might_sleep.cold.155+0x1f2/0x238 > [ 13.801107][ T248] cgroup_rstat_flush+0x1c/0x50 > [ 13.801724][ T248] mem_cgroup_flush_stats+0x39/0x50 > [ 13.802131][ T248] process_one_work+0xa6c/0x17d0 > [ 13.802131][ T248] ? pwq_dec_nr_in_flight+0x360/0x360 > [ 13.802131][ T248] ? do_raw_spin_lock+0x121/0x2d0 > [ 13.802131][ T248] worker_thread+0x8c/0xda0 > [ 13.802131][ T248] ? process_one_work+0x17d0/0x17d0 > [ 13.802131][ T248] kthread+0x3d5/0x4c0 > [ 13.802131][ T248] ? set_kthread_struct+0x130/0x130 > [ 13.802131][ T248] ret_from_fork+0x1f/0x30 > > To fix this, move stats_flush_lock into cgroup_rstat_flush() after might_sleep(), > unlock stats_flush_lock before sleep in cgroup_rstat_flush_locked() and lock > stats_flush_lock after wake up. > > Fixes: 42265e014ac7 ("memcg: infrastructure to flush memcg stats") > Signed-off-by: Yang Yingliang Thanks for the report. > --- > kernel/cgroup/rstat.c | 7 +++++++ > mm/memcontrol.c | 5 ----- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c > index 7f0e58917432..d7ce96ba0105 100644 > --- a/kernel/cgroup/rstat.c > +++ b/kernel/cgroup/rstat.c > @@ -5,6 +5,7 @@ > > static DEFINE_SPINLOCK(cgroup_rstat_lock); > static DEFINE_PER_CPU(raw_spinlock_t, cgroup_rstat_cpu_lock); > +static DEFINE_SPINLOCK(stats_flush_lock); > > static void cgroup_base_stat_flush(struct cgroup *cgrp, int cpu); > > @@ -175,8 +176,10 @@ static void cgroup_rstat_flush_locked(struct cgroup *cgrp, bool may_sleep) > if (may_sleep && (need_resched() || > spin_needbreak(&cgroup_rstat_lock))) { > spin_unlock_irq(&cgroup_rstat_lock); > + spin_unlock(&stats_flush_lock); > if (!cond_resched()) > cpu_relax(); > + spin_lock(&stats_flush_lock); > spin_lock_irq(&cgroup_rstat_lock); > } > } > @@ -199,9 +202,13 @@ void cgroup_rstat_flush(struct cgroup *cgrp) > { > might_sleep(); > > + if (!spin_trylock(&stats_flush_lock)) > + return; > + No, there is no point of stats_flush_lock if we add this to rstat and take it for everyone. > spin_lock_irq(&cgroup_rstat_lock); > cgroup_rstat_flush_locked(cgrp, true); > spin_unlock_irq(&cgroup_rstat_lock); > + spin_unlock(&stats_flush_lock); > } > > /** > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 456f5310ea59..c6b0f5b41893 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -109,7 +109,6 @@ static DECLARE_DEFERRABLE_WORK(stats_flush_dwork, flush_memcg_stats_dwork); > static void flush_memcg_stats_work(struct work_struct *w); > static DECLARE_WORK(stats_flush_work, flush_memcg_stats_work); > static DEFINE_PER_CPU(unsigned int, stats_flush_threshold); > -static DEFINE_SPINLOCK(stats_flush_lock); > > #define THRESHOLDS_EVENTS_TARGET 128 > #define SOFTLIMIT_EVENTS_TARGET 1024 > @@ -5355,11 +5354,7 @@ static void mem_cgroup_css_reset(struct cgroup_subsys_state *css) > > void mem_cgroup_flush_stats(void) > { > - if (!spin_trylock(&stats_flush_lock)) > - return; > - > cgroup_rstat_flush(root_mem_cgroup->css.cgroup); We should change this to cgroup_rstat_flush_irqsafe(). One addition we can do is to change all rstat flush callsites in memcontrol.c to go through mem_cgroup_flush_stats() > - spin_unlock(&stats_flush_lock); > } > > static void flush_memcg_stats_dwork(struct work_struct *w) > -- > 2.25.1 >