From patchwork Thu May 17 13:51:21 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Davydov X-Patchwork-Id: 10406831 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 357F26037D for ; Thu, 17 May 2018 13:51:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1E9B927CEE for ; Thu, 17 May 2018 13:51:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C66002821F; Thu, 17 May 2018 13:51:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D75DF28600 for ; Thu, 17 May 2018 13:51:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D02816B04F0; Thu, 17 May 2018 09:51:27 -0400 (EDT) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id CB3FB6B04F1; Thu, 17 May 2018 09:51:27 -0400 (EDT) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BCBC56B04F2; Thu, 17 May 2018 09:51:27 -0400 (EDT) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from mail-lf0-f72.google.com (mail-lf0-f72.google.com [209.85.215.72]) by kanga.kvack.org (Postfix) with ESMTP id 4CA616B04F0 for ; Thu, 17 May 2018 09:51:27 -0400 (EDT) Received: by mail-lf0-f72.google.com with SMTP id b5-v6so2002339lff.3 for ; Thu, 17 May 2018 06:51:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:dkim-signature:date:from:to:cc:subject :message-id:references:mime-version:content-disposition:in-reply-to; bh=nMVgZF6WDAF3M4s6hOq2abtYZfhwqYffzQyyvsq3Nfo=; b=nT4mh7uJ/k46kI0j6IHuWqI0xfVpILMGsvG/oAAyXQUWctLBHyLPFYuGJth4WRk8Tw Iwx8hqteqzl+925EPjqkxbSoHlx7ewNLvGDQtgdMdd1kot943DSaSAxstxgjb5kQZv4V QDiF6qsDDXf7VOoHMNW7I10syOAg3GATJH5vxB7r8zyP0EDS1pi460lnaMT5qRFeQmx7 xKBE9PJxj9dFual1lF87x2JWAzW8hputvDgGrNtoDtZX0SAlVen6E3WpgsesKgaJyxkr nZWp/oFI4JUh6NOolVYr3BIKyPuLg6+DsVK0bAnymusmt6z4LST6Bx916eqz2TsJCei3 igNQ== X-Gm-Message-State: ALKqPweSOcyfiR4Pbygfb0GmdGapU59lcVensFGh0IxF/WlwVDa9hxeX yIbb4yqjATjSLC2kBq5evH1U36jfApfiWHggMajvVciqFCLKk66cdykl88QkS9c1aiqY6DnEGK4 8aO9ctIR9pTH5QOz6+qfv8f874E1cFwC5QrVFpJH0q2PP6kV4mG7eDtYXIUQTzg7u6NXkWuig0B dgKGFdstbP7+Ch+ruGXcjePZr8fYiHtKOBj3gM+wIGDTH54o0VLddal2bhs3ytS+qBk9XakbKKP X2vc2Z75fIwDvFLbGJizdnhsB2MTfk5IbDxn+XDyfuzSPSNegHjCcDGfKApy9GY4CjYO1X/hwYT X/tFY71nr0fEolLmfmpNsCTj9/U5KqJ9dpneUvjJFwFrbHrnU8wQzZsQw+988v82ALmtRoIXXVM S X-Received: by 2002:a2e:9b01:: with SMTP id u1-v6mr3358645lji.26.1526565086585; Thu, 17 May 2018 06:51:26 -0700 (PDT) X-Received: by 2002:a2e:9b01:: with SMTP id u1-v6mr3358599lji.26.1526565085485; Thu, 17 May 2018 06:51:25 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526565085; cv=none; d=google.com; s=arc-20160816; b=N6Z5HbRIDr7Mfc9rIe2NalxgR1AFJNjAur1NiOaQtnUpBayvC7ix4rkJAUT6RcNTut vL3+t8aEMpuVdsPnQTxZVbCaHxcztGbky3Ds8hHzcHqe1CZ97Tc7guXEzWsdq3ZitCly zMnBq+DjQR8FK2RGryHtZ5n9I+gBPOVREI75wm8AsR/E2EOYVt9l8cuqvc6sotQYB2WY sGDzOAVUAhIV3vNQx1Dy53BCcDv0VMYBErHtUZZ00gVlcTo/kmy4FqQW6NPAUskiG7bj HdYlKooXlOFWjuGAU0qPgC6oEgsZr+Lafni3rEpu6W12RjkoJHD8SjH0ovYHCKcxxGUC vCFQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:dkim-signature:arc-authentication-results; bh=nMVgZF6WDAF3M4s6hOq2abtYZfhwqYffzQyyvsq3Nfo=; b=nMdRgQeo8c9mcbV+zyIWhFnX9jYPlUoco+HmKTjLdb6tWHLE83A9zidaXbDmgLk9Xl uy4QsVn91Q+7ZN6S5o8JkgxZan0rfCo2bOBV/xut8IGAnbPo+IithjI4YBnffukOF89u g4LY2hF730b+0JiNDQs8YJUjkFZaMlLmOhwcYVwkKnNnFjeWzbdNtnhs77H5rMxsg3ff JoD+MP9nY05cFZvzzF8+/WI0Sj7ZCNvr+terjwt21okUBvVFrfYhfOv4YHw8La+VLC70 Xj3hbTYnTL2fEKdWXm4T3BRhP3D/kRNE+aSHL+39TZkZggQmQjpp73Cytre1Y3ZgEhU5 11Kg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=MITynNe8; spf=pass (google.com: domain of vdavydov.dev@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=vdavydov.dev@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id y6-v6sor1359805lfe.112.2018.05.17.06.51.25 for (Google Transport Security); Thu, 17 May 2018 06:51:25 -0700 (PDT) Received-SPF: pass (google.com: domain of vdavydov.dev@gmail.com designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=MITynNe8; spf=pass (google.com: domain of vdavydov.dev@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=vdavydov.dev@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=nMVgZF6WDAF3M4s6hOq2abtYZfhwqYffzQyyvsq3Nfo=; b=MITynNe8lXmaLVvsvdxWD/Db7BjxoNdzQr5f8NJcEZNEie2NtvgqWVikF/OdV/lMzQ 7UpsvyxbnNuwlGNeudAQ5I1baIBln0QrRQZXYgrn3vuUwdvJzIaSqLLeuUCv0sYka604 DfHsEiUi5nOBa+qA21bvs3IWgAoFQgTj5kMvl0nll/ob6uDzSsGzUgawfQK5h9xspR2m f9KJSR2mqgvFDWikkJI/0xpvE+IlebOW7W5I4wA5t0CpOB1RUBa3pVe1tdKqxzSEgwpz UfNxnbABNjjdNKnG4lUzeE17GqPd4GrefaWZFagqlB8uKegzmArw5HFN4D8WI0o3pFu9 Olpw== X-Google-Smtp-Source: AB8JxZpFVvtimP8KNst6pG74StDPCWvF8CT0tp+eEYVop2eZwCRUCPuYdLfINWd8YY3tbkRM918Nnw== X-Received: by 2002:a19:9e12:: with SMTP id h18-v6mr17508171lfe.101.1526565084953; Thu, 17 May 2018 06:51:24 -0700 (PDT) Received: from esperanza ([185.6.245.156]) by smtp.gmail.com with ESMTPSA id c1-v6sm860620ljk.80.2018.05.17.06.51.23 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 17 May 2018 06:51:24 -0700 (PDT) Date: Thu, 17 May 2018 16:51:21 +0300 From: Vladimir Davydov To: Kirill Tkhai Cc: akpm@linux-foundation.org, shakeelb@google.com, viro@zeniv.linux.org.uk, hannes@cmpxchg.org, mhocko@kernel.org, tglx@linutronix.de, pombredanne@nexb.com, stummala@codeaurora.org, gregkh@linuxfoundation.org, sfr@canb.auug.org.au, guro@fb.com, mka@chromium.org, penguin-kernel@I-love.SAKURA.ne.jp, chris@chris-wilson.co.uk, longman@redhat.com, minchan@kernel.org, ying.huang@intel.com, mgorman@techsingularity.net, jbacik@fb.com, linux@roeck-us.net, linux-kernel@vger.kernel.org, linux-mm@kvack.org, willy@infradead.org, lirongqing@baidu.com, aryabinin@virtuozzo.com Subject: Re: [PATCH v5 11/13] mm: Iterate only over charged shrinkers during memcg shrink_slab() Message-ID: <20180517135121.wtaiuj6pqxzodrlr@esperanza> References: <152594582808.22949.8353313986092337675.stgit@localhost.localdomain> <152594603565.22949.12428911301395699065.stgit@localhost.localdomain> <20180515054445.nhe4zigtelkois4p@esperanza> <5c0dbd12-8100-61a2-34fd-8878c57195a3@virtuozzo.com> <20180517041634.lgkym6gdctya3oq6@esperanza> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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: X-Virus-Scanned: ClamAV using ClamSMTP On Thu, May 17, 2018 at 02:49:26PM +0300, Kirill Tkhai wrote: > On 17.05.2018 07:16, Vladimir Davydov wrote: > > On Tue, May 15, 2018 at 05:49:59PM +0300, Kirill Tkhai wrote: > >>>> @@ -589,13 +647,7 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, > >>>> .memcg = memcg, > >>>> }; > >>>> > >>>> - /* > >>>> - * If kernel memory accounting is disabled, we ignore > >>>> - * SHRINKER_MEMCG_AWARE flag and call all shrinkers > >>>> - * passing NULL for memcg. > >>>> - */ > >>>> - if (memcg_kmem_enabled() && > >>>> - !!memcg != !!(shrinker->flags & SHRINKER_MEMCG_AWARE)) > >>>> + if (!!memcg != !!(shrinker->flags & SHRINKER_MEMCG_AWARE)) > >>>> continue; > >>> > >>> I want this check gone. It's easy to achieve, actually - just remove the > >>> following lines from shrink_node() > >>> > >>> if (global_reclaim(sc)) > >>> shrink_slab(sc->gfp_mask, pgdat->node_id, NULL, > >>> sc->priority); > >> > >> This check is not related to the patchset. > > > > Yes, it is. This patch modifies shrink_slab which is used only by > > shrink_node. Simplifying shrink_node along the way looks right to me. > > shrink_slab() is used not only in this place. drop_slab_node() doesn't really count as it is an extract from shrink_node() > I does not seem a trivial change for me. > > >> Let's don't mix everything in the single series of patches, because > >> after your last remarks it will grow at least up to 15 patches. > > > > Most of which are trivial so I don't see any problem here. > > > >> This patchset can't be responsible for everything. > > > > I don't understand why you balk at simplifying the code a bit while you > > are patching related functions anyway. > > Because this function is used in several places, and we have some particulars > on root_mem_cgroup initialization, and this function called from these places > with different states of root_mem_cgroup. It does not seem trivial fix for me. Let me do it for you then: Seems simple enough to fold it into this patch, doesn't it? diff --git a/mm/vmscan.c b/mm/vmscan.c index 9b697323a88c..e778569538de 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -486,10 +486,8 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl, * @nid is passed along to shrinkers with SHRINKER_NUMA_AWARE set, * unaware shrinkers will receive a node id of 0 instead. * - * @memcg specifies the memory cgroup to target. If it is not NULL, - * only shrinkers with SHRINKER_MEMCG_AWARE set will be called to scan - * objects from the memory cgroup specified. Otherwise, only unaware - * shrinkers are called. + * @memcg specifies the memory cgroup to target. Unaware shrinkers + * are called only if it is the root cgroup. * * @priority is sc->priority, we take the number of objects and >> by priority * in order to get the scan target. @@ -554,6 +552,7 @@ void drop_slab_node(int nid) struct mem_cgroup *memcg = NULL; freed = 0; + memcg = mem_cgroup_iter(NULL, NULL, NULL); do { freed += shrink_slab(GFP_KERNEL, nid, memcg, 0); } while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL); @@ -2557,9 +2556,8 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc) shrink_node_memcg(pgdat, memcg, sc, &lru_pages); node_lru_pages += lru_pages; - if (memcg) - shrink_slab(sc->gfp_mask, pgdat->node_id, - memcg, sc->priority); + shrink_slab(sc->gfp_mask, pgdat->node_id, + memcg, sc->priority); /* Record the group's reclaim efficiency */ vmpressure(sc->gfp_mask, memcg, false, @@ -2583,10 +2581,6 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc) } } while ((memcg = mem_cgroup_iter(root, memcg, &reclaim))); - if (global_reclaim(sc)) - shrink_slab(sc->gfp_mask, pgdat->node_id, NULL, - sc->priority); - if (reclaim_state) { sc->nr_reclaimed += reclaim_state->reclaimed_slab; reclaim_state->reclaimed_slab = 0;