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=-2.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 3ACC2C433E0 for ; Mon, 28 Dec 2020 20:03:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 92CDD20867 for ; Mon, 28 Dec 2020 20:03:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 92CDD20867 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 873208D001D; Mon, 28 Dec 2020 15:03:42 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8248D8D0018; Mon, 28 Dec 2020 15:03:42 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6C4ED8D001D; Mon, 28 Dec 2020 15:03:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0218.hostedemail.com [216.40.44.218]) by kanga.kvack.org (Postfix) with ESMTP id 54BB78D0018 for ; Mon, 28 Dec 2020 15:03:42 -0500 (EST) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 0AADE180AD801 for ; Mon, 28 Dec 2020 20:03:42 +0000 (UTC) X-FDA: 77643766284.27.land84_230878727496 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin27.hostedemail.com (Postfix) with ESMTP id D7CEC3D8EE for ; Mon, 28 Dec 2020 20:03:41 +0000 (UTC) X-HE-Tag: land84_230878727496 X-Filterd-Recvd-Size: 6116 Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf35.hostedemail.com (Postfix) with ESMTP for ; Mon, 28 Dec 2020 20:03:41 +0000 (UTC) Received: by mail-ed1-f44.google.com with SMTP id b2so10778884edm.3 for ; Mon, 28 Dec 2020 12:03:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=l6wLs3O3NbOgM5Ikne900yyPCAu6Ee+BSm2CLHZBcHs=; b=j0cC9TO1gDQxvqxWiUNWhnTF78mOXGz1VUQ1QK2ltNTP+r3WsgDcHAbjiDuEaUT/aW BU0DPotHLrWwj+fUP8C8oGxaI24drzA97kJTPgcsRlFnoK7vBNsgEt+r4l8qaOFXcNtI yP7hGvEnJb/CdBzcs/1QXTuNfQRZv4T0RMPWVRbhvpp8XdrrSpP4l0Z/WP6nLH7cx9yD XnsWySG9f289D+LXaTGmaWZK+9SJvNfEL/nDWQVOss/+3cAKUysEgpLAdOJXVo5d7I0s c8I/p6AtgpOPK+ELH4w8pbzu47Xx2FnJo9Gn37gWXeoFPSPV0YaA72or2DSb3EX6FWCy zk/A== 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=l6wLs3O3NbOgM5Ikne900yyPCAu6Ee+BSm2CLHZBcHs=; b=je0CQtu/XwXFhCEpQVGM5NV837w/YvbdXGCkYtuyDWbJQFoE6ywIYrb2nl9L+OhG2e VLpzFU4tCJCQ1WSNOHdTd4A1jmU183xXxv1Pteq9PEBkn1ojUu6GjDIx+Qobv6dpENSV Dpy/SOjHd5OD3lNVHei0Wp5xEln34S/x/DA4LY9f1Gn9uU6jm3lalDpOOVADRseT9Edk WcSGdb9WUt+lyMO7T2DilIlpjxmbinyv4xBXsUfw2O6irA/QTY+EJm60TMQjn76557Xw g4/P5Ta+a6J7Bv4Bex4JI+hKnttYYQFG7glMc86JN1YLRz/zRV1SmbJc+JyLFMp0CdKf a0pQ== X-Gm-Message-State: AOAM530bB0Gl+cDHMDnKbpeSsZ0jQy1zRATVvR3hoM1t/cNEDHgxWWRF Htzl8/IeHz/+yACDhv4RB4C9kK0mM44RTkmxBk4= X-Google-Smtp-Source: ABdhPJwlQf5wLF9FUY8a25sToNVh5qWgpnEg5KLjLvsGQ1lxQga3FI+dAsDiqTRhVt44lfrT95MqTjsmPwE91/1tbV8= X-Received: by 2002:a05:6402:1c8a:: with SMTP id cy10mr43621201edb.151.1609185820234; Mon, 28 Dec 2020 12:03:40 -0800 (PST) MIME-Version: 1.0 References: <20201214223722.232537-1-shy828301@gmail.com> <20201214223722.232537-4-shy828301@gmail.com> <20201215171439.GC385334@cmpxchg.org> In-Reply-To: From: Yang Shi Date: Mon, 28 Dec 2020 12:03:28 -0800 Message-ID: Subject: Re: [v2 PATCH 3/9] mm: vmscan: guarantee shrinker_slab_memcg() sees valid shrinker_maps for online memcg To: Johannes Weiner Cc: Roman Gushchin , Kirill Tkhai , Shakeel Butt , Dave Chinner , Michal Hocko , Andrew Morton , Linux MM , Linux FS-devel Mailing List , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" 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: I think Johannes's point makes sense to me. If the shrinker_maps is not initialized yet it means the memcg is too young to have a number of reclaimable slab caches. It sounds fine to just skip it. And, with consolidating shrinker_maps and shrinker_deferred into one struct, we could just check the pointer of the struct. So, it seems this patch is not necessary anymore. This patch will be dropped in v3. On Tue, Dec 15, 2020 at 12:31 PM Yang Shi wrote: > > On Tue, Dec 15, 2020 at 9:16 AM Johannes Weiner wrote: > > > > On Mon, Dec 14, 2020 at 02:37:16PM -0800, Yang Shi wrote: > > > The shrink_slab_memcg() races with mem_cgroup_css_online(). A visibility of CSS_ONLINE flag > > > in shrink_slab_memcg()->mem_cgroup_online() does not guarantee that we will see > > > memcg->nodeinfo[nid]->shrinker_maps != NULL. This may occur because of processor reordering > > > on !x86. > > > > > > This seems like the below case: > > > > > > CPU A CPU B > > > store shrinker_map load CSS_ONLINE > > > store CSS_ONLINE load shrinker_map > > > > But we have a separate check on shrinker_maps, so it doesn't matter > > that it isn't guaranteed, no? > > IIUC, yes. Checking shrinker_maps is the alternative way to detect the > reordering to prevent from seeing NULL shrinker_maps per Kirill. > > We could check shrinker_deferred too, then just walk away if it is NULL. > > > > > The only downside I can see is when CSS_ONLINE isn't visible yet and > > we bail even though we'd be ready to shrink. Although it's probably > > unlikely that there would be any objects allocated already... > > Yes, it seems so. > > > > > Can somebody remind me why we check mem_cgroup_online() at all? > > IIUC it should be mainly used to skip offlined memcgs since there is > nothing on offlined memcgs' LRU because all objects have been > reparented. But shrinker_map won't be freed until .css_free is called. > So the shrinkers might be called in vain. > > > > > If shrinker_map is set, we can shrink: .css_alloc is guaranteed to be > > complete, and by using RCU for the shrinker_map pointer, the map is > > also guaranteed to be initialized. There is nothing else happening > > during onlining that you may depend on. > > > > If shrinker_map isn't set, we cannot iterate the bitmap. It does not > > really matter whether CSS_ONLINE is reordered and visible already. > > As I mentioned above it should be used to skip offlined memcgs, but it > also opens the race condition due to memory reordering. As Kirill > explained in the earlier email, we could either check the pointer or > use memory barriers. > > If the memory barriers seems overkilling, I could definitely switch > back to NULL pointer check approach. > > > > > Agreed with Dave: if we need that synchronization around onlining, it > > needs to happen inside the cgroup core. But I wouldn't add that until > > somebody actually required it.