From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752132AbdK3Kqj (ORCPT ); Thu, 30 Nov 2017 05:46:39 -0500 Received: from mail-eopbgr00125.outbound.protection.outlook.com ([40.107.0.125]:38418 "EHLO EUR02-AM5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750948AbdK3Kqh (ORCPT ); Thu, 30 Nov 2017 05:46:37 -0500 Subject: [PATCH v2-REBASED] mm: Make count list_lru_one::nr_items lockless From: Kirill Tkhai To: shakeelb@google.com, akpm@linux-foundation.org, vdavydov.dev@gmail.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, aryabinin@virtuozzo.com, ktkhai@virtuozzo.com Date: Thu, 30 Nov 2017 13:46:30 +0300 Message-ID: <151203869520.3915.2587549826865799173.stgit@localhost.localdomain> User-Agent: StGit/0.18 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.6] X-ClientProxiedBy: VI1PR08CA0239.eurprd08.prod.outlook.com (2603:10a6:802:15::48) To AM5PR0801MB1332.eurprd08.prod.outlook.com (2603:10a6:203:1f::10) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c34bffc5-6f3c-42df-36ff-08d537df9fc7 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(4534020)(4602075)(7168020)(4627115)(201703031133081)(201702281549075)(5600026)(4604075)(2017052603284);SRVR:AM5PR0801MB1332; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1332;3:OhCQdCzIkawR2HxHhNz5Wa9xGfJKL8W/zsFFzfl6LaHcSKoDEXOyXYb3GJQPwbwTU8etihf29eLKhEVQKIBlCiXenFSLW2lUYVnCo5Ym3W7ZSDYB0LxyFLmCTtxzlkHblaetC7q5l8XvqNJB2kiRi1JqIwwQYopcVk/kApWYn9/GaRBM3YFC75PR1IE1Xos3uVr0nMbmaLliywo51N72AL5Hwa6uDOV+mH/0TfTdPRLob1pbZUJ7dWBgIFZeRZls;25:Jc1qK6JSslP/kHMY/rkFMkc9C5W9WHjF6KlMWyAWFmajG9PnTYBIYmY/n6HDmnJqdnhEAVlQPaSdXZcPxnGcc5jC/6OmhSPAIkaGnQv0WtbVquH6o4DJzEaoaHiMJlqR8ZSShfs+Ajuqhi7UUi/ysOQZeWIgfWu+/qGBsKfjvcuAE8L4xYbGmOXeG6dVMH0hSp/jONIo7qEjNYAcUTgXkCwS+U1Idk+lqEdSiqSyW6ALDjFQtSA/R2tdz5F4USB/GHjWIxiEcmUN37AbLAGV7CY9b9TKWkZ27GvcUfH1lybJbpvpkwyhztAA8dVBB1U6L2K5ZKlkRWBMixhIFRpqZ5W2s+fJSWdO1Vz+4nDHgO4=;31:U7qI5nXis670RGi9MODPurCkHF0ObcyvtjGCq6XLRH8r9E7AkrQz3RJqGi5wK+2WfjmIFbc23fQd6OXPzdsU1v6YjTHHKbL4Yon0yyJkxGPZJlPHlJPaIToMDumIq7gzmyXGYuawa2zYBIMXGcB5pegAvPcGunMyxL7KijqTynIYPjponGN0MbLBVLzMCv+PDP+zAEenpZ99m7bCCq37N/mr36jRwTmC0vGC1mFzu5I= X-MS-TrafficTypeDiagnostic: AM5PR0801MB1332: X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1332;20:Qse+n1Wq4kcyyftyIGtWb0NRdB7zKvL6UumusfvtB6zGLN5ZbhqnKqnrRv7B4N4CTJwhP5FqPnuA7Q5IcBJHd3JA+AbKd0A2TCIs06gJciZ6LsOCyVekc+B2z8O3arRdJwqtjnPzcWLjjQbiZOa5wfuo953ty5qMiY+TjSXU3pOgqesiYGAZA0zIoAY7WoZU1g0leEChVXbUL/ydEFIiBLcoBw2mujX+NzvvK9VhxJdfG80zYLEYRWA0HcmX5rhdU2ErYMzpIin5wwOb1jpU7UU9zt4BKRaaDaLqn5Yd0+oBMrw1n4AfaFu21h5dlnaJmwTUJnAVb6nPskuwyDq/78h4ck+Gl9dyFO55xtsb5PaVqf1fM/3fwS0ZBf/K/P8nPS+7YqZDNq3HKFC0NcS2ignOFwW3VlIjkCvsYYHtSQY=;4:D7oqMWzMLXs+dM5xUElsSWj5oU0cYCTriIdy8/Cbl4H7SF7JRy7QpR2lBRnzXgMpuueGIR6HkNlKknzuMTjNqh+VJcCKdwsYnvqZiGXVP5k8q6X+TMuCNyUILh0+RhJ+8SC7two5XaQgCcwuMABHL2rJ13SFi2W8gORmXWPiFwyEYw8vEYb5NK/Zqb7n1ueLAUEHsWOACFOXE2JBDLHQLW5jhsw06ep+HdjzZqZ14wHuVmAWLcSKUZxWFmkTgGWvp7z5Dkei+tYzfKU0aBkLYfPQTgTX0HcQu2aNh5LAufQ2MUw73H+Zn3YZjtyVmR247EhMikuqj4N6k2ljqUULNkV1+nCPLuaMEjxnZIQ+4TcP+PK0a/2LhE36vrZOkbaP X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(190756311086443)(211936372134217)(153496737603132); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(2401047)(5005006)(8121501046)(3002001)(10201501046)(93006095)(93001095)(3231022)(920507027)(6041248)(20161123564025)(20161123562025)(20161123558100)(20161123555025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(201708071742011);SRVR:AM5PR0801MB1332;BCL:0;PCL:0;RULEID:(100000803101)(100110400095);SRVR:AM5PR0801MB1332; X-Forefront-PRVS: 05079D8470 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6009001)(6069001)(366004)(346002)(376002)(189002)(199003)(86362001)(25786009)(575784001)(9686003)(8676002)(5660300001)(16526018)(105586002)(68736007)(8936002)(7736002)(58126008)(106356001)(2486003)(305945005)(7696005)(52116002)(23676004)(230700001)(54356010)(50986010)(316002)(83506002)(189998001)(34040400001)(97736004)(6116002)(3846002)(2906002)(81166006)(478600001)(101416001)(103116003)(81156014)(55236003)(53936002)(61506002)(66066001)(55016002)(47776003)(39060400002)(6506006)(33646002)(50466002);DIR:OUT;SFP:1102;SCL:1;SRVR:AM5PR0801MB1332;H:localhost.localdomain;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=ktkhai@virtuozzo.com; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtBTTVQUjA4MDFNQjEzMzI7MjM6bVVybzYxMkpTTkgwazlucVlvMEtIMkxV?= =?utf-8?B?d05QNUlvL0FxODVlLzc4VXNGajRuYURZMTRZVURsdEN3U3lTQmFnd2pLdU9M?= =?utf-8?B?MjNwZXoxTCtQbXpZaFFIQlBlN1NJRTdqcmFJL1luQ21nbXVvQzFiNkozTGtr?= =?utf-8?B?V2lsYWlreVR1TGwyUG50WmZwYzdwUE1vVWpOSFBQQWd6ZlRPR2NvOHg5dDRt?= =?utf-8?B?NjB1NENkTTM5dGxJNlMzNC9sUjFNbzVRdVFRb0d2Z2VDSTgxanoyV3ViTDZ3?= =?utf-8?B?ekRrWDNMajlhMzBseTBtU3o3c05HSlVaM0srZVJFS1hudGxWZjhVRFErZ2xt?= =?utf-8?B?Nk1MODdVQVJ4R2hRNDZ3SE5LeCtSQ3AyQmlGS2VyZWpLN2FhcHVpSHR3QkVR?= =?utf-8?B?UWorV2VSOWZYNHR0Z2loNVc4MSt6ZUdNSTllL0lsSDJxc1BUL1JXK2dFMHZq?= =?utf-8?B?WWxWbTRHRHhzdnV5ZG91YnBEWURiSVlacjJ1amJkOTczMkREYUUwaW9mVXdW?= =?utf-8?B?dmtsSXdiR1FOaUJHNXRiNnVvNFg1V1VHVWN2d2hLdmp6eklTNHp4Y1RzN09z?= =?utf-8?B?U3E0bWhGbGIxZUNFajJvWE5LNnBRZVNzM0Yyc0dnVC9YMk9JTFQ2SnhzakZ1?= =?utf-8?B?RS8wV21JajRha2VoQnNhN1BBOGZ4dVdWRFplL0ZvaklFSENMNkRMbStFTmpH?= =?utf-8?B?UEEyVHFPY0xZaDBjYkxaNmVua0gzSTZUY00zKzQ3YW1wSzkrNXFoa0c3SUpt?= =?utf-8?B?cm5sRitsZHVTaE1ybzJ6elM5ZlJXbndINkhIUmlKdVNMQUcrU0lBZVVVMXhh?= =?utf-8?B?NW54Nm5jemx1VW1qcGRBMzFSbjlSYStTN2lwWmcrN3AvZkcwajQvcnFqa3JG?= =?utf-8?B?Y0g0VGlxUnFsWWVyM1lPamtXdGdEaGVvU3JDYkRSRnBPbzljY2UxdXZjYW1h?= =?utf-8?B?QWcrdFdLcW9PdE14TUpvZVlVYTFMUzRVM1Raa1lQUEFlTHJWRk1UVFkrMjJI?= =?utf-8?B?c3FXQ3BqbndNazIzOUo0M2tjY0xpY3gwS3JxR05qTCtxZmg0SC9OcjFLSUFU?= =?utf-8?B?WHFUdlcyZDNxTnZRNVR1M2xHWXg3VEZva1p4cmRUejU2OFRHTFFVTTNBNjB5?= =?utf-8?B?YUg1TjZVQ0puZ2k0bmtjdHo4L1JRcTdjWDRnanpnQ0RIT1BvV0tDQTNBWW5I?= =?utf-8?B?S0xPSmlLQ25aOEZrY1liZnF6YjNkZXFnaHN5a05leDhDWEFZYjhHUlp4YzBz?= =?utf-8?B?bWFuS2czek00a0ZtOVNNWWsxalV6cjFHSzRPOW1Sa0pObzUzYktLV3F6cGlR?= =?utf-8?B?Q2VJS3JBTG5rQlkzR3VqbDJUejNqVWtXRkVBeDJybTNKVVhuZ0lYcU1aaUw1?= =?utf-8?B?THVwK3lGWmFRc3ZJa2ZxNGNBRDZSS2haSzZ6VEQ1ZWUwMVZaRFgweEEyTkhn?= =?utf-8?B?cGhwUkpnQjF2elhXZ2VFRndVVThHTkFtbmlUQlMwUCtSTXZzd1ZaaXM2WmVR?= =?utf-8?B?bXAzbkU0UGJIRWhXWmdPM1RwaXdSekY4TjN3SFNRWXcxT0luYitFT2ZOOHhs?= =?utf-8?B?YVJ0amVhNTQyOVhCaEQ3MXdsQUhHanJlcVZUNFNwc2hmUWxXZ00wWkpiK0JJ?= =?utf-8?Q?4=3D?= X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1332;6:v/9gzmzWTNZ1Q73msaS6fX6kKhYVyDYN8vpPcDKHX5Qc6uFwTzPzaNwVN/uyFzIaQZoUTyhZNe2aaURzzlOuaE6G5GS3WpF8xyH0pILCkYbnWr/VhDY0t3pdOTddsu2dudKFPpPobxh8ThVkTDJ6QLB/TM+oEoxnxy/IVg7KLBjzjiGRKeB46+9Ap2ziUg8C5duCDYZnovhCNDpiWckhOFxdZ8SCmgBluFurtUk1RrlbzhiVg89M8vhKrv8e3dGg050ca8QbDDd/uB5EIcZBCrRwkBCoZXqs/ZrC9AqPrpBrpV7m7ebidjkxnWc6nEMh5pnRVyEavVIV8CbgdHCdj5fTap3tFur8uLWlpWVvvEs=;5:ESA5UBHPVJfmulZDFxpaSkLbxOXFSb+pmCbXsZokcMSnxYDQMf6AL041EZVkw8TFIrMUWejJgLokSyWC+Ipua1Ds9S1DLqgCNNoHNSuAiG9bIjFwgPucefQ9Tx1hbkLABOk9KJ8dUpMc/i/ozSn/74WfpbRXoj59oSXZaegc3CI=;24:wrd4N79lBZfwqulbdpvOGJdggfhkywpOwggYgdaBgE4vjrsE8JEO5+umEcNI1xfuLAarAo6FTZvzqUcYxwW+ObgN0gh04G583xHNHe9XJ5c=;7:qTpVxr7wMxlJbxqZ7m0Y1duQLYY2xcYZtqUavqgPbMO+BNN3Fibgzp/fBQEhxuWAX2mQAnm3I1OXEds3d7hnw97QxX3ZkmjMI2kaJZ3miazPO6xcw71dgki//0Nw7AGQ8a3DgTg69XQxaUs7WQd9ujn8O3egLh6mpcMOGkkLB5N/BED7Fo0fPrKTiDcStKtt/Uy96XYRdvPjkGVtDfzwlTsPiBSJRRFM1v9809WzUuqkbRUI8c9dp5K7p7OV3bdO SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1332;20:cWtcJHpMFuUOW8IA/M8yulHF7tflQkWfUNtgs0jDgY/EKOPDPQuVIplOTwX80UOR1FdleYfcUnE3VEdalp542q5GNHdau8otz2mvRSPGQ90G0Hbry41hpLmOGUcfUWBx5BH5G1ZjMoAd9QJfoRAn60xvm5gAoegD+Lzj8LYS+Ak= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2017 10:46:33.3629 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: c34bffc5-6f3c-42df-36ff-08d537df9fc7 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1332 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org During the reclaiming slab of a memcg, shrink_slab iterates over all registered shrinkers in the system, and tries to count and consume objects related to the cgroup. In case of memory pressure, this behaves bad: I observe high system time and time spent in list_lru_count_one() for many processes on RHEL7 kernel. This patch makes list_lru_node::memcg_lrus rcu protected, that allows to skip taking spinlock in list_lru_count_one(). Shakeel Butt with the patch observes signify perf graph change. He says: ======================================================================== Setup: running a fork-bomb in a memcg of 200MiB on a 8GiB and 4 vcpu VM and recording the trace with 'perf record -g -a'. The trace without the patch: + 34.19% fb.sh [kernel.kallsyms] [k] queued_spin_lock_slowpath + 30.77% fb.sh [kernel.kallsyms] [k] _raw_spin_lock + 3.53% fb.sh [kernel.kallsyms] [k] list_lru_count_one + 2.26% fb.sh [kernel.kallsyms] [k] super_cache_count + 1.68% fb.sh [kernel.kallsyms] [k] shrink_slab + 0.59% fb.sh [kernel.kallsyms] [k] down_read_trylock + 0.48% fb.sh [kernel.kallsyms] [k] _raw_spin_unlock_irqrestore + 0.38% fb.sh [kernel.kallsyms] [k] shrink_node_memcg + 0.32% fb.sh [kernel.kallsyms] [k] queue_work_on + 0.26% fb.sh [kernel.kallsyms] [k] count_shadow_nodes With the patch: + 0.16% swapper [kernel.kallsyms] [k] default_idle + 0.13% oom_reaper [kernel.kallsyms] [k] mutex_spin_on_owner + 0.05% perf [kernel.kallsyms] [k] copy_user_generic_string + 0.05% init.real [kernel.kallsyms] [k] wait_consider_task + 0.05% kworker/0:0 [kernel.kallsyms] [k] finish_task_switch + 0.04% kworker/2:1 [kernel.kallsyms] [k] finish_task_switch + 0.04% kworker/3:1 [kernel.kallsyms] [k] finish_task_switch + 0.04% kworker/1:0 [kernel.kallsyms] [k] finish_task_switch + 0.03% binary [kernel.kallsyms] [k] copy_page ======================================================================== Thanks Shakeel for the testing. Tested-by: Shakeel Butt Signed-off-by: Kirill Tkhai Acked-by: Vladimir Davydov v2: Rebase on kvmalloc() and kvfree(). Name of kvfree_rcu() was choosen to help not to skip this place, when someone will implement such the global interface. --- include/linux/list_lru.h | 3 +- mm/list_lru.c | 67 +++++++++++++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h index bb8129a3474d..96def9d15b1b 100644 --- a/include/linux/list_lru.h +++ b/include/linux/list_lru.h @@ -32,6 +32,7 @@ struct list_lru_one { }; struct list_lru_memcg { + struct rcu_head rcu; /* array of per cgroup lists, indexed by memcg_cache_id */ struct list_lru_one *lru[0]; }; @@ -43,7 +44,7 @@ struct list_lru_node { struct list_lru_one lru; #if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB) /* for cgroup aware lrus points to per cgroup lists, otherwise NULL */ - struct list_lru_memcg *memcg_lrus; + struct list_lru_memcg __rcu *memcg_lrus; #endif long nr_items; } ____cacheline_aligned_in_smp; diff --git a/mm/list_lru.c b/mm/list_lru.c index f141f0c80ff3..dc42a964b896 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -52,14 +52,15 @@ static inline bool list_lru_memcg_aware(struct list_lru *lru) static inline struct list_lru_one * list_lru_from_memcg_idx(struct list_lru_node *nlru, int idx) { + struct list_lru_memcg *memcg_lrus; /* - * The lock protects the array of per cgroup lists from relocation - * (see memcg_update_list_lru_node). + * Either lock or RCU protects the array of per cgroup lists + * from relocation (see memcg_update_list_lru_node). */ - lockdep_assert_held(&nlru->lock); - if (nlru->memcg_lrus && idx >= 0) - return nlru->memcg_lrus->lru[idx]; - + memcg_lrus = rcu_dereference_check(nlru->memcg_lrus, + lockdep_is_held(&nlru->lock)); + if (memcg_lrus && idx >= 0) + return memcg_lrus->lru[idx]; return &nlru->lru; } @@ -168,10 +169,10 @@ static unsigned long __list_lru_count_one(struct list_lru *lru, struct list_lru_one *l; unsigned long count; - spin_lock(&nlru->lock); + rcu_read_lock(); l = list_lru_from_memcg_idx(nlru, memcg_idx); count = l->nr_items; - spin_unlock(&nlru->lock); + rcu_read_unlock(); return count; } @@ -323,24 +324,41 @@ static int __memcg_init_list_lru_node(struct list_lru_memcg *memcg_lrus, static int memcg_init_list_lru_node(struct list_lru_node *nlru) { + struct list_lru_memcg *memcg_lrus; int size = memcg_nr_cache_ids; - nlru->memcg_lrus = kvmalloc(size * sizeof(void *), GFP_KERNEL); - if (!nlru->memcg_lrus) + memcg_lrus = kvmalloc(sizeof(*memcg_lrus) + + size * sizeof(void *), GFP_KERNEL); + if (!memcg_lrus) return -ENOMEM; - if (__memcg_init_list_lru_node(nlru->memcg_lrus, 0, size)) { - kvfree(nlru->memcg_lrus); + if (__memcg_init_list_lru_node(memcg_lrus, 0, size)) { + kvfree(memcg_lrus); return -ENOMEM; } + RCU_INIT_POINTER(nlru->memcg_lrus, memcg_lrus); return 0; } static void memcg_destroy_list_lru_node(struct list_lru_node *nlru) { - __memcg_destroy_list_lru_node(nlru->memcg_lrus, 0, memcg_nr_cache_ids); - kvfree(nlru->memcg_lrus); + struct list_lru_memcg *memcg_lrus; + /* + * This is called when shrinker has already been unregistered, + * and nobody can use it. So, there is no need to use kvfree_rcu(). + */ + memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, true); + __memcg_destroy_list_lru_node(memcg_lrus, 0, memcg_nr_cache_ids); + kvfree(memcg_lrus); +} + +static void kvfree_rcu(struct rcu_head *head) +{ + struct list_lru_memcg *mlru; + + mlru = container_of(head, struct list_lru_memcg, rcu); + kvfree(mlru); } static int memcg_update_list_lru_node(struct list_lru_node *nlru, @@ -350,8 +368,9 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru, BUG_ON(old_size > new_size); - old = nlru->memcg_lrus; - new = kvmalloc(new_size * sizeof(void *), GFP_KERNEL); + old = rcu_dereference_protected(nlru->memcg_lrus, + lockdep_is_held(&list_lrus_mutex)); + new = kvmalloc(sizeof(*new) + new_size * sizeof(void *), GFP_KERNEL); if (!new) return -ENOMEM; @@ -360,29 +379,33 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru, return -ENOMEM; } - memcpy(new, old, old_size * sizeof(void *)); + memcpy(&new->lru, &old->lru, old_size * sizeof(void *)); /* - * The lock guarantees that we won't race with a reader - * (see list_lru_from_memcg_idx). + * The locking below allows readers that hold nlru->lock avoid taking + * rcu_read_lock (see list_lru_from_memcg_idx). * * Since list_lru_{add,del} may be called under an IRQ-safe lock, * we have to use IRQ-safe primitives here to avoid deadlock. */ spin_lock_irq(&nlru->lock); - nlru->memcg_lrus = new; + rcu_assign_pointer(nlru->memcg_lrus, new); spin_unlock_irq(&nlru->lock); - kvfree(old); + call_rcu(&old->rcu, kvfree_rcu); return 0; } static void memcg_cancel_update_list_lru_node(struct list_lru_node *nlru, int old_size, int new_size) { + struct list_lru_memcg *memcg_lrus; + + memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, + lockdep_is_held(&list_lrus_mutex)); /* do not bother shrinking the array back to the old size, because we * cannot handle allocation failures here */ - __memcg_destroy_list_lru_node(nlru->memcg_lrus, old_size, new_size); + __memcg_destroy_list_lru_node(memcg_lrus, old_size, new_size); } static int memcg_init_list_lru(struct list_lru *lru, bool memcg_aware) From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f200.google.com (mail-pf0-f200.google.com [209.85.192.200]) by kanga.kvack.org (Postfix) with ESMTP id 67AFC6B0038 for ; Thu, 30 Nov 2017 05:46:39 -0500 (EST) Received: by mail-pf0-f200.google.com with SMTP id a6so4672612pff.17 for ; Thu, 30 Nov 2017 02:46:39 -0800 (PST) Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00130.outbound.protection.outlook.com. [40.107.0.130]) by mx.google.com with ESMTPS id i62si3050367pfj.336.2017.11.30.02.46.37 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 30 Nov 2017 02:46:37 -0800 (PST) Subject: [PATCH v2-REBASED] mm: Make count list_lru_one::nr_items lockless From: Kirill Tkhai Date: Thu, 30 Nov 2017 13:46:30 +0300 Message-ID: <151203869520.3915.2587549826865799173.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: shakeelb@google.com, akpm@linux-foundation.org, vdavydov.dev@gmail.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, aryabinin@virtuozzo.com, ktkhai@virtuozzo.com During the reclaiming slab of a memcg, shrink_slab iterates over all registered shrinkers in the system, and tries to count and consume objects related to the cgroup. In case of memory pressure, this behaves bad: I observe high system time and time spent in list_lru_count_one() for many processes on RHEL7 kernel. This patch makes list_lru_node::memcg_lrus rcu protected, that allows to skip taking spinlock in list_lru_count_one(). Shakeel Butt with the patch observes signify perf graph change. He says: ======================================================================== Setup: running a fork-bomb in a memcg of 200MiB on a 8GiB and 4 vcpu VM and recording the trace with 'perf record -g -a'. The trace without the patch: + 34.19% fb.sh [kernel.kallsyms] [k] queued_spin_lock_slowpath + 30.77% fb.sh [kernel.kallsyms] [k] _raw_spin_lock + 3.53% fb.sh [kernel.kallsyms] [k] list_lru_count_one + 2.26% fb.sh [kernel.kallsyms] [k] super_cache_count + 1.68% fb.sh [kernel.kallsyms] [k] shrink_slab + 0.59% fb.sh [kernel.kallsyms] [k] down_read_trylock + 0.48% fb.sh [kernel.kallsyms] [k] _raw_spin_unlock_irqrestore + 0.38% fb.sh [kernel.kallsyms] [k] shrink_node_memcg + 0.32% fb.sh [kernel.kallsyms] [k] queue_work_on + 0.26% fb.sh [kernel.kallsyms] [k] count_shadow_nodes With the patch: + 0.16% swapper [kernel.kallsyms] [k] default_idle + 0.13% oom_reaper [kernel.kallsyms] [k] mutex_spin_on_owner + 0.05% perf [kernel.kallsyms] [k] copy_user_generic_string + 0.05% init.real [kernel.kallsyms] [k] wait_consider_task + 0.05% kworker/0:0 [kernel.kallsyms] [k] finish_task_switch + 0.04% kworker/2:1 [kernel.kallsyms] [k] finish_task_switch + 0.04% kworker/3:1 [kernel.kallsyms] [k] finish_task_switch + 0.04% kworker/1:0 [kernel.kallsyms] [k] finish_task_switch + 0.03% binary [kernel.kallsyms] [k] copy_page ======================================================================== Thanks Shakeel for the testing. Tested-by: Shakeel Butt Signed-off-by: Kirill Tkhai Acked-by: Vladimir Davydov v2: Rebase on kvmalloc() and kvfree(). Name of kvfree_rcu() was choosen to help not to skip this place, when someone will implement such the global interface. --- include/linux/list_lru.h | 3 +- mm/list_lru.c | 67 +++++++++++++++++++++++++++++++--------------- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h index bb8129a3474d..96def9d15b1b 100644 --- a/include/linux/list_lru.h +++ b/include/linux/list_lru.h @@ -32,6 +32,7 @@ struct list_lru_one { }; struct list_lru_memcg { + struct rcu_head rcu; /* array of per cgroup lists, indexed by memcg_cache_id */ struct list_lru_one *lru[0]; }; @@ -43,7 +44,7 @@ struct list_lru_node { struct list_lru_one lru; #if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB) /* for cgroup aware lrus points to per cgroup lists, otherwise NULL */ - struct list_lru_memcg *memcg_lrus; + struct list_lru_memcg __rcu *memcg_lrus; #endif long nr_items; } ____cacheline_aligned_in_smp; diff --git a/mm/list_lru.c b/mm/list_lru.c index f141f0c80ff3..dc42a964b896 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -52,14 +52,15 @@ static inline bool list_lru_memcg_aware(struct list_lru *lru) static inline struct list_lru_one * list_lru_from_memcg_idx(struct list_lru_node *nlru, int idx) { + struct list_lru_memcg *memcg_lrus; /* - * The lock protects the array of per cgroup lists from relocation - * (see memcg_update_list_lru_node). + * Either lock or RCU protects the array of per cgroup lists + * from relocation (see memcg_update_list_lru_node). */ - lockdep_assert_held(&nlru->lock); - if (nlru->memcg_lrus && idx >= 0) - return nlru->memcg_lrus->lru[idx]; - + memcg_lrus = rcu_dereference_check(nlru->memcg_lrus, + lockdep_is_held(&nlru->lock)); + if (memcg_lrus && idx >= 0) + return memcg_lrus->lru[idx]; return &nlru->lru; } @@ -168,10 +169,10 @@ static unsigned long __list_lru_count_one(struct list_lru *lru, struct list_lru_one *l; unsigned long count; - spin_lock(&nlru->lock); + rcu_read_lock(); l = list_lru_from_memcg_idx(nlru, memcg_idx); count = l->nr_items; - spin_unlock(&nlru->lock); + rcu_read_unlock(); return count; } @@ -323,24 +324,41 @@ static int __memcg_init_list_lru_node(struct list_lru_memcg *memcg_lrus, static int memcg_init_list_lru_node(struct list_lru_node *nlru) { + struct list_lru_memcg *memcg_lrus; int size = memcg_nr_cache_ids; - nlru->memcg_lrus = kvmalloc(size * sizeof(void *), GFP_KERNEL); - if (!nlru->memcg_lrus) + memcg_lrus = kvmalloc(sizeof(*memcg_lrus) + + size * sizeof(void *), GFP_KERNEL); + if (!memcg_lrus) return -ENOMEM; - if (__memcg_init_list_lru_node(nlru->memcg_lrus, 0, size)) { - kvfree(nlru->memcg_lrus); + if (__memcg_init_list_lru_node(memcg_lrus, 0, size)) { + kvfree(memcg_lrus); return -ENOMEM; } + RCU_INIT_POINTER(nlru->memcg_lrus, memcg_lrus); return 0; } static void memcg_destroy_list_lru_node(struct list_lru_node *nlru) { - __memcg_destroy_list_lru_node(nlru->memcg_lrus, 0, memcg_nr_cache_ids); - kvfree(nlru->memcg_lrus); + struct list_lru_memcg *memcg_lrus; + /* + * This is called when shrinker has already been unregistered, + * and nobody can use it. So, there is no need to use kvfree_rcu(). + */ + memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, true); + __memcg_destroy_list_lru_node(memcg_lrus, 0, memcg_nr_cache_ids); + kvfree(memcg_lrus); +} + +static void kvfree_rcu(struct rcu_head *head) +{ + struct list_lru_memcg *mlru; + + mlru = container_of(head, struct list_lru_memcg, rcu); + kvfree(mlru); } static int memcg_update_list_lru_node(struct list_lru_node *nlru, @@ -350,8 +368,9 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru, BUG_ON(old_size > new_size); - old = nlru->memcg_lrus; - new = kvmalloc(new_size * sizeof(void *), GFP_KERNEL); + old = rcu_dereference_protected(nlru->memcg_lrus, + lockdep_is_held(&list_lrus_mutex)); + new = kvmalloc(sizeof(*new) + new_size * sizeof(void *), GFP_KERNEL); if (!new) return -ENOMEM; @@ -360,29 +379,33 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru, return -ENOMEM; } - memcpy(new, old, old_size * sizeof(void *)); + memcpy(&new->lru, &old->lru, old_size * sizeof(void *)); /* - * The lock guarantees that we won't race with a reader - * (see list_lru_from_memcg_idx). + * The locking below allows readers that hold nlru->lock avoid taking + * rcu_read_lock (see list_lru_from_memcg_idx). * * Since list_lru_{add,del} may be called under an IRQ-safe lock, * we have to use IRQ-safe primitives here to avoid deadlock. */ spin_lock_irq(&nlru->lock); - nlru->memcg_lrus = new; + rcu_assign_pointer(nlru->memcg_lrus, new); spin_unlock_irq(&nlru->lock); - kvfree(old); + call_rcu(&old->rcu, kvfree_rcu); return 0; } static void memcg_cancel_update_list_lru_node(struct list_lru_node *nlru, int old_size, int new_size) { + struct list_lru_memcg *memcg_lrus; + + memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, + lockdep_is_held(&list_lrus_mutex)); /* do not bother shrinking the array back to the old size, because we * cannot handle allocation failures here */ - __memcg_destroy_list_lru_node(nlru->memcg_lrus, old_size, new_size); + __memcg_destroy_list_lru_node(memcg_lrus, old_size, new_size); } static int memcg_init_list_lru(struct list_lru *lru, bool memcg_aware) -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org