From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932902AbcHDDeL (ORCPT ); Wed, 3 Aug 2016 23:34:11 -0400 Received: from mail-db5eur01on0091.outbound.protection.outlook.com ([104.47.2.91]:5392 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932237AbcHDDeH (ORCPT ); Wed, 3 Aug 2016 23:34:07 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=VDavydov@virtuozzo.com; Date: Wed, 3 Aug 2016 14:46:40 +0300 From: Vladimir Davydov To: Michal Hocko CC: Andrew Morton , , Johannes Weiner , , Subject: Re: [PATCH v2 1/3] mm: memcontrol: fix swap counter leak on swapout from offline cgroup Message-ID: <20160803114639.GI13263@esperanza> References: <20160802160025.GB28900@dhcp22.suse.cz> <20160803095049.GG13263@esperanza> <20160803110941.GA19196@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160803110941.GA19196@dhcp22.suse.cz> X-Originating-IP: [195.214.232.10] X-ClientProxiedBy: VI1PR07CA0050.eurprd07.prod.outlook.com (10.164.94.146) To AM5PR0801MB1857.eurprd08.prod.outlook.com (10.168.157.14) X-MS-Office365-Filtering-Correlation-Id: 442dc14d-c9eb-4133-320b-08d3bb93d7cb X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1857;2:kBLD2wxZwp3f/ynS34UkkSy8XjpKhfoN+CeBWVLPIjkq8Q5VDwMf7/z59mxttpRQGvM93/oyTUYG8f6NeVhj+uK7IE77GmGJGZJxugN11reclpoPxjO3FZ0s3l301cx5oICRbSrUwR8ZycGfpTFeTjaaPrWxxs0uKkZ16+mvgLh5MzFqDdfsdIITSwN1bJoK;3:+Y+R+VS88+5UW2tyNVqeqMF+EYrJ/n1ZiYBPoK5hSWHUzA2arLvEplDAeHJth0fPYL+XOvp2Q9dePPQI0sNHP9+Njfrj1iyxoeZ1/+Mjte6frnQQYMGifNUJJtWSOZVJ X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB1857; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1857;25:ufL4A+s3gXqp1z7+ncczgHv2O58EWE+DznBo4+5KQ6QTZ+L705OJ7Cw+KJ7bU3S8O2kspazllWvahwyhX6WofjZaDwxFsqxqhsDbI38jUJI4CcwewOwL5S2KiZ/dEBsQYGAX1FDhfrS6PeIAVPDz4GQkcIHcovyMBGBnRziuMC0+Q48sUS4AoYVF6bmIL3+1ns9MemdQ+fBxKPosQVh5yndYzeLH4GV01Kj5vr2zGsHSSuhA+ASWugBIxghJh8Pxw+qUUW7btGcnlV9WaZqIPVxJDcrt6iu0ZoDLec565mB7wPzV7TznXLHqJ/Asb8MoRGUW6o2k2dqSDUOfzcWeHQ8CjnVVwvwAQot+wSH11pnfP+t60B3f8uItk/w2/Bi2t3orjhsTtd9DuZLFX3HOGGOMiE2qY1U9pDe0pwZyzw5pYbTvp+O0HQo7W3KcnjiOOY8t4SP9jt3vfnZCVw0mLTvhgOSu0qFY1bO6azlWu85punlDtwwjNdl0xE/H3dTqEWyaCDe6CHF9BGLT9GxR3ZZQ1D9Ul19fb/ZBKyF7N0iV0NPRKQQltCNbF21K4iwJIggMgWXUBkNXwPQbzBUjL43xNH7Y5xYOyms8wZMI5PHxxkZehmYRhEaik6hXb1YLY2yd8KiJbmC7bk0gq082Ojlq3BO4TwHMkGFakdYiJPShqQJc7dbGPMejmEt11N1OrTaZY00c/nAaj91GoPWudA==;31:taK711Qv+VNO/UvjL2Ma4Ke+V6SfnC38RrptrXFvioZtHrtun4C33m656G3RntGsAb2mEhhClhdyZSRGUBJinb/eYXZuj++TQKVn5tL9uPyzOvSaAmJmzOVDBtpluCbXJIqLtU3Uh5rfemoVULfM4Bj75bY2wS7h8mWMvf9tb6ibLj+MvvbojeRi/DDDGaY0YRPoQoX6EgtCsdKyc8UcI4gAIYUXS873cFwGZVxRxdA= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6041072)(6043046);SRVR:AM5PR0801MB1857;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB1857; X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1857;4:rlTUbh9Tl+ze6Sv6zG0gViAiyrARz57H5UDJ26tGPPT2yJ1a8/aF15nVrjFLGQwlOnie3CJSe4PuA8PvC8k4VDEtWJ9iS5IHVjmRI+Fb14YqRtqd64i4wGQAdWuYuSitCYRsRFTfT/lDQL8qFKLPpZy7dGZiXlOaeIlffwUnBIbrygYxbEvK3PHhkE6DBxv4Gwa8pnxlYQ75/ykKN4jK4kt0Uhj3yvG2Ff4rVsH9oGT8fNrJqC3JeRedISeA095SvG5DhYtKa7zIUyIzF3cTnS5IYfj+ZdkpLgLSrxbLt8HRWRGySmkglnWgWJmapk1lrsVdDBt1ILZGris0EPEgWKxoEGA0sLadME3lQOVC18sj9SXJ5dxvJ2KAOVFaSEe5UHm6DF44QKO1RgWZ/IPRvDkr07yVgDuxF7CLbYSezvI= X-Forefront-PRVS: 00235A1EEF X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(7916002)(199003)(189002)(377424004)(24454002)(9686002)(7846002)(7736002)(106356001)(97736004)(68736007)(305945005)(110136002)(77096005)(80792005)(575784001)(66066001)(50466002)(33716001)(76176999)(54356999)(2950100001)(47776003)(86362001)(50986999)(46406003)(101416001)(81166006)(105586002)(8676002)(81156014)(97756001)(42186005)(33656002)(2906002)(4326007)(189998001)(93886004)(19580395003)(586003)(92566002)(1076002)(23726003)(6116002)(3846002);DIR:OUT;SFP:1102;SCL:1;SRVR:AM5PR0801MB1857;H:esperanza;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;AM5PR0801MB1857;23:NRlziNAiinUuw9D8/eCsHbNRiBLYYglFh2dkP3Q?= =?us-ascii?Q?A4xOUUDw45za/wn2ytwPHJJ44JP9xk+WaD2iAxQXNSFBJm7d5BNSc2S7Glkf?= =?us-ascii?Q?JuCLxvNs7rW6ashhcVAWYherEMIFvQrTBIgpg7pbiNBSd7IMohCyo5bundIP?= =?us-ascii?Q?rxjUzVz4fMO1UPaBqjRDwnUe1sdtOB6MgHeUIzfhHUqsXNE6WgUbV+zpzbBP?= =?us-ascii?Q?cotdTsMlIvaTCV70nDdSiyYYRYdnebp5SRLiptj2ldCvPVBTXzoBdCtiq6Yq?= =?us-ascii?Q?Vdqu7Ox99CwO1+EyO+Q4HHiqXZDaBhFiiSbiymDNnz7JaeuoBLioadvWbxb+?= =?us-ascii?Q?tf2PxY4zFUOwm895rS/QgFL5Oadn+jCSiR80Of/Q1tIKujG6w2QpRuHgr7i6?= =?us-ascii?Q?gCiq/XsdFaKPkz3min1AM6Az84VzGVtM1CbKuf4L+ysb1EOyhO5koN0FRUNb?= =?us-ascii?Q?jco0jmQRIAOyf3R29+aICh+3FW0B+g5u3UaWS6+WYDMa8tJvM5tIH5QcpCz4?= =?us-ascii?Q?793ymPiRj/zh5zrfsYFlU6uHysySBYvNYTFGRVHSRqblyk3KMrXCFoIx9Ebk?= =?us-ascii?Q?yGDNocCxhVLq1rwSvPnvZ00XjnaYLTnmE/1AhSoiHBgBxkfxC6FeS7ZnCz4u?= =?us-ascii?Q?OKiISpgcdDysYlT7b/jbT4ThPasp2Wr50QrW7+RdYnxXtTMWCFR9gJ3bq/10?= =?us-ascii?Q?k2fYdbFdQ4B2F9jvMqq5hvrZE6c2NPz/2cyD9cBrUCKwN/P4TJeATUcwBIGo?= =?us-ascii?Q?3cl7OtVfo274Fn8+OL2XPkpl2RQOYKc/XS3yWAu+Al0mfPYV+zl28FPanL5L?= =?us-ascii?Q?xFZomT4O5UbrFOZW81+hxA/OZDGiTw9E3K++zkOR7N1tBe9DBzFRknZKFJEz?= =?us-ascii?Q?Day/cnXDgy3PEfj52+nW9dW9uSDtsmpw0hHTDN9H/CdR0kb6rS64Xbi+a9I6?= =?us-ascii?Q?o02dYrRKlpDSbjcDYPnc2sgHTsTk89BxLmjhVkBWsHYqLNMYuY0y6DenAxuY?= =?us-ascii?Q?K+KC+/6bl2bvfquDI0C7IhU75LwoOHMhEjnUGL8K7qsX9FlKZ1Kdjai6PVs6?= =?us-ascii?Q?NW4f3RD8QrRkZbEZGaFHhhK5jVNp6uYB7JiEE1NszvRYZld16kIOaOjcPgus?= =?us-ascii?Q?5EE5Hb9S8wwCt/xxIe1gDDJOdcNEbCl5zLRLXgLkHHa/F9sPRgEHSKg=3D?= =?us-ascii?Q?=3D?= X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1857;6:VR0XIkFDRA/gi4AiN4W98IWH+38A4Bx9FiaDYOerNx4oJMDWVUogakuOzSkbnGRU9gHvrsZcoJK+9Ka5S1/b6FSzKFvZklgNdewLnTC91nIC6i1AaHBjaJAnXcRgX9g+lIaV71TzmjTu2eAybpxG0hcVWdeq/GOgq8ZlNWH8r5qaPpVsQ+OJPdZQlnUdfZrLBrgNsrnW5pC5tGrihv8MvDZnljmbts+ruu15LT5DjmtZOBndMb+4sETQjhyTXNxDlFRCGOHXx/7Lca3FbRzPL6B/M5W7ofMdcLcFNb6VOGYp2qiOEXN4cNrWuRw2WeYN;5:m7NrGL2XIVsIMGS3bCvCDJDZUEc3yfVPdQ+zE2LJzdqyNdfob6UPE1Wzeg76JYzGT94eu6nS/2qlJT8VaBdRe5XGiXUIM0/VWUcbOkhbRII7vYMBS42nmjUDP2aj36BgpAGTFtZt/4p8xozq6RUMaQ==;24:3aMj0sFazWTExZxjPvAh9ImwnbRD4GA6kJ86aMLyI8ZUsnheLMBgarLxhoxlGFtKTd5qJHikZA5vawRPTiujMndXt72hHpXyrg3MeYa51Uk=;7:7i2Z1rQekm117xbmbmNQimsKr0S9yPf9bgLlnz3tG/HFLHmzLNSUWARSNuBGfP5wDiVRDFrukVzDYRkl/B7Yyb+K0xImhCykakbasyDarGvjJnjtFlxhuHug/+898o59ksoaUVstLgbwn7SPssg+Rw+1ybin4UImBeiSlPI4Fx7wB35n6qyrxU/1ICGPutgw9yWOvS7F6NKHSiVSd/p40fk/vCoL/Oq/sHCDs8ZgOOyGCA1LfYPkrJSsyJT1zEkW SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;AM5PR0801MB1857;20:i3mVbsIbJ/kXrvKfpRqdNT1UZc6AY9+1ZViKz2D+r3qJrl+p2vOQFa2RrYG23HhCstNR/V+xPggYO8PtO1e6rOVjHlEmjwNu/wdX70DoxxpN3Fphnc9kmJ0hytRqENwD3HCMNEuAo8c9Lel0RrailUzTOG8gBISvFz1kO7cz9Ck= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Aug 2016 11:46:43.7010 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1857 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 03, 2016 at 01:09:42PM +0200, Michal Hocko wrote: > On Wed 03-08-16 12:50:49, Vladimir Davydov wrote: > > On Tue, Aug 02, 2016 at 06:00:26PM +0200, Michal Hocko wrote: > > > On Tue 02-08-16 18:00:48, Vladimir Davydov wrote: > > ... > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > > index 3be791afd372..4ae12effe347 100644 > > > > --- a/mm/memcontrol.c > > > > +++ b/mm/memcontrol.c > > > > @@ -4036,6 +4036,24 @@ static void mem_cgroup_id_get(struct mem_cgroup *memcg) > > > > atomic_inc(&memcg->id.ref); > > > > } > > > > > > > > +static struct mem_cgroup *mem_cgroup_id_get_active(struct mem_cgroup *memcg) > > > > +{ > > > > + while (!atomic_inc_not_zero(&memcg->id.ref)) { > > > > + /* > > > > + * The root cgroup cannot be destroyed, so it's refcount must > > > > + * always be >= 1. > > > > + */ > > > > + if (memcg == root_mem_cgroup) { > > > > + VM_BUG_ON(1); > > > > + break; > > > > + } > > > > > > why not simply VM_BUG_ON(memcg == root_mem_cgroup)? > > > > Because with DEBUG_VM disabled we could wind up looping forever here if > > the refcount of the root_mem_cgroup got screwed up. On production > > kernels, it's better to break the loop and carry on closing eyes on > > diverging counters rather than getting a lockup. > > Wouldn't this just paper over a real bug? Anyway I will not insist but > making the code more complex just to pretend we can handle a situation > gracefully doesn't sound right to me. But we can handle this IMO. AFAICS diverging id refcount will typically result in leaking swap charges, which aren't even a real resource. At worst, we can leak an offline mem_cgroup, which is also not critical enough to crash the production system. I see your concern of papering over a bug though. What about adding a warning there? diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 1c0aa59fd333..8c8e68becee9 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4044,7 +4044,7 @@ static struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg) * The root cgroup cannot be destroyed, so it's refcount must * always be >= 1. */ - if (memcg == root_mem_cgroup) { + if (WARN_ON_ONCE(memcg == root_mem_cgroup)) { VM_BUG_ON(1); break; }