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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 989A4C54E67 for ; Wed, 20 Mar 2024 12:09:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2BED96B0088; Wed, 20 Mar 2024 08:09:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 249876B0089; Wed, 20 Mar 2024 08:09:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0E7446B008A; Wed, 20 Mar 2024 08:09:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id ECE406B0088 for ; Wed, 20 Mar 2024 08:09:42 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id C275D1C1152 for ; Wed, 20 Mar 2024 12:09:42 +0000 (UTC) X-FDA: 81917298204.09.E9D7958 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf10.hostedemail.com (Postfix) with ESMTP id 986C7C002C for ; Wed, 20 Mar 2024 12:09:40 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b="mj/2E0/5"; dkim=pass header.d=suse.com header.s=susede1 header.b="mj/2E0/5"; spf=pass (imf10.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710936581; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=EHOMMEAuTVumsvUa0s4ONSCEm/gaHIgEjhSYOqP1F4Y=; b=mShUcSFELzUJ/FbH4r9P8+6S8HvBABPbR9L7GVHt62ala/7oL1NidNgRMaRwpKiwMry53W K6eq/RFjfEALIbPjEEAwHYqtg/JlFqQpLN0rW9NN0cNUlzKk3nnJbBkS9a3TKoj2NVyKD1 4ax6aCtRLY66b+3G61FAas22GOsGWjU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710936581; a=rsa-sha256; cv=none; b=s73eq0VLqCfwUxZ+SLTuTz8IgHKk3SZDLlx60XOuChGxupI3GZHkiywRrRJ6x7dzQKLy+l bRmdva2thfLGI2ktWbYJLLXYC2ZhAZd0tC3cK+Ut6rzz7XdMCbZBscPUHIuu3ywVp8yTcy Q003uJGXbRudZyDfpLGlOQgW6DYSylM= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b="mj/2E0/5"; dkim=pass header.d=suse.com header.s=susede1 header.b="mj/2E0/5"; spf=pass (imf10.hostedemail.com: domain of mhocko@suse.com designates 195.135.223.130 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 1DB51343A8; Wed, 20 Mar 2024 12:09:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1710936577; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EHOMMEAuTVumsvUa0s4ONSCEm/gaHIgEjhSYOqP1F4Y=; b=mj/2E0/5TZON9G1Mp/Nhmw8SXKJ5mIo0sqnTy2GCUF7obOGsl9qV+Apk1EVo9RUTQlb5E5 CsD/HRa9wquFGI9RkjNgQQUDviFFQ0fO1GfTZoWkCFGbmkMtLWGrMBRcdyPM6iXOA1k0u9 7AScJw+OPTp6T/hN+mbYYGFBb853moQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1710936577; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EHOMMEAuTVumsvUa0s4ONSCEm/gaHIgEjhSYOqP1F4Y=; b=mj/2E0/5TZON9G1Mp/Nhmw8SXKJ5mIo0sqnTy2GCUF7obOGsl9qV+Apk1EVo9RUTQlb5E5 CsD/HRa9wquFGI9RkjNgQQUDviFFQ0fO1GfTZoWkCFGbmkMtLWGrMBRcdyPM6iXOA1k0u9 7AScJw+OPTp6T/hN+mbYYGFBb853moQ= Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id EE820136D6; Wed, 20 Mar 2024 12:09:36 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id OMDZNwDS+mXcDgAAD6G6ig (envelope-from ); Wed, 20 Mar 2024 12:09:36 +0000 Date: Wed, 20 Mar 2024 13:09:32 +0100 From: Michal Hocko To: Pavel Tikhomirov Cc: Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Andrew Morton , Vladimir Davydov , cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel@openvz.org Subject: Re: [PATCH] mm/memcontrol: stop resize loop if limit was changed again Message-ID: References: <20240320100556.463266-1-ptikhomirov@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 986C7C002C X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: as3cbeetcnre5xjs94hrz7kxajoj6j98 X-HE-Tag: 1710936580-124128 X-HE-Meta: U2FsdGVkX1/c8mQrQj/voeJeP0NupDj6KU0T2V1li8htdin8uk4gpaPjSS8IbofftGEJln0CFG6vLUdMkDNqPUjIZYRvmf3ioRG2eOenG289nepP1bWptr02uyk2HUFTiggfuKzSl6uRrSHdrwqIsxm6X3yYcR4i4Y3rzF8GPeDQNG4xijJkGZ8N4F5v1wb/01cLy0DFg6n7Rou1NaLCwmnxeeRz+PZV4x4QwJfK4hHxTr8tieHU+qruG4GZ33zVjMVD8ZGKLpoMufL4FTNyscZHzn2oqqLbkxZeWX84zB1Kmpn9mEh+qvyXvWOGfZ4GNP37qnAp5UrpvBedi5mQORVJilds1ocXw3mNRMain5SyONsFXJFevTTBTSA4uQVtF+78vr658X/FH1TLwBd5dxeFU8ZUDtjm+SkjuoksnpuUtGP3BStxRjyXpDpBwS5ZCTjLvfmhSSNmGDT1zx0JWwrrTMWYAEKrWYYXVFeQCxn6t/XWHVM1l4FhjG8Tc+lIIV3OvM2iU4Vo+RfjJzzbvumnRZYK7IR7XBp9ju8VSBKWrWVgZft1WrOkDUktBnJGHqhWLyRgnoHKHCUOrAlcRHhH9DNFCkZuG1zWnBFo6Mi3tlV1IGlvmDHhQ8fuF8VabvY/YvwyUqJ2pUn7Mea8BE2StSmF91KMj0plf9bCdJLXLL1wIzYcDRQCEN51ZU0kxmX1rcGw+2uaFNlKHJkNdB/Z8BZRLga++D9vdJ43DBC2B6PFYrUhCKqnSdJef0qWT/5QxJ/yFq8RwA0Ijm+79HjAzqXopx63Q/6lBmAQtXa2jbzVA+E3X7Ija7QF5Tqv+EYPDooM+d5SVeQKykzVLCwdN4vgywIdPjKOa7hAOd5AqupDG1sb1sMsoBzT2/+ZGztG30ukev+WKSmnrXbkOANbm4bLgtXO5Z0gXyT2GiUijp5HWgpgTS0Dpx+q9FRJr/UK1T/PyYmrt8lEDCn 5PA== 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: List-Subscribe: List-Unsubscribe: On Wed 20-03-24 18:55:05, Pavel Tikhomirov wrote: > > > On 20/03/2024 18:28, Michal Hocko wrote: > > On Wed 20-03-24 18:03:30, Pavel Tikhomirov wrote: > > > In memory_max_write() we first set memcg->memory.max and only then > > > try to enforce it in loop. What if while we are in loop someone else > > > have changed memcg->memory.max but we are still trying to enforce > > > the old value? I believe this can lead to nasty consequence like getting > > > an oom on perfectly fine cgroup within it's limits or excess reclaim. > > > > I would argue that uncoordinated hard limit configuration can cause > > problems on their own. > > Sorry, didn't know that. Well, just consider potential over-reclaim as a result of several competing actors to set the same limit. Or completely indeterministic final output of the limit setting depending on timing. This simply cannot work reliably. > > Beside how is this any different from changing > > the high limit while we are inside the reclaim loop? > > I believe reclaim loop rereads limits on each iteration, e.g. in > reclaim_high(), so it should always be enforcing the right limit. Reclaim loop might happen to take quite some time... > > > We also have exactly the same thing in memory_high_write(). > > > > > > So let's stop enforcing old limits if we already have a new ones. > > > > I do see any reasons why this would be harmful I just do not see why > > this is a real thing or why the new behavior is any better for racing > > updaters as those are not deterministic anyway. If you have any actual > > usecase then more details would really help to justify this change. > > > > The existing behavior makes some sense as it enforces the given limit > > deterministically. > > I don't have any actual problem, usecase or reproduce at hand, I only see a > potential problem: > > Let's imagine that: > > a) We set cgroup max limit to some small value, memory_max_write updates > memcg->memory.max and starts spinning in loop as it wants to reclaim some > memory which does not fit in new limit. > > b) We don't need small limit anymore and we raise the limit to a big value, > but memory_max_write() from (a) is still spinning. And if we are lucky > enough and processes of cgroup are constantly consuming memory, to > compensate effect from memory_max_write() from (a), so that it will continue > spinning there forever. This is a killable operation, so if you decide to change mind about limit setting and the current update is still in progress then just terminate it rather then override by a different process. > Yes it is not that bad, because memory_max/high_write() also constantly > checks for pending signals in loop so they won't actually get irreversibly > stuck. But I just thought it was worth fixing. If we want to fix this parallel limits setting then we should also think about a reasonable and predictable behavior and that would likely require some sort of locking IMO. -- Michal Hocko SUSE Labs