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=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 B68BBC433DB for ; Fri, 15 Jan 2021 16:23:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 22B44238EE for ; Fri, 15 Jan 2021 16:23:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 22B44238EE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 45C3B8D0194; Fri, 15 Jan 2021 11:23:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 40D5C8D0187; Fri, 15 Jan 2021 11:23:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 325488D0194; Fri, 15 Jan 2021 11:23:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0074.hostedemail.com [216.40.44.74]) by kanga.kvack.org (Postfix) with ESMTP id 1D7658D0187 for ; Fri, 15 Jan 2021 11:23:21 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id D40B61803F495 for ; Fri, 15 Jan 2021 16:23:20 +0000 (UTC) X-FDA: 77708529360.20.paper12_56099af27530 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin20.hostedemail.com (Postfix) with ESMTP id B58AA180C7234 for ; Fri, 15 Jan 2021 16:23:20 +0000 (UTC) X-HE-Tag: paper12_56099af27530 X-Filterd-Recvd-Size: 7334 Received: from mail-qk1-f173.google.com (mail-qk1-f173.google.com [209.85.222.173]) by imf33.hostedemail.com (Postfix) with ESMTP for ; Fri, 15 Jan 2021 16:23:19 +0000 (UTC) Received: by mail-qk1-f173.google.com with SMTP id z11so12159753qkj.7 for ; Fri, 15 Jan 2021 08:23:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=d8Z+PWPkeXqU4sVnEZWxtzrZF8DqYxnDeZ23COUUVZo=; b=jmNfe3u7/Jx6sddnUHr0M4wMCCLmSUAgwMTvm1sMlKyzlzfIXrkClAgtlPfPBD8WtH 6pf5xxYYrew/dipIl81zwoGMZcDWbZw0hgxiowtOTRfToSarJCNfK0ElFwdtZD/JAihm JwuWtyH/AlO43NtcdSSFAU5Y7z622ZHR4KWaJD/T9rDlP+qxQ8B80ZgTC3t7upJB5rO3 gVJRDkv4B7g2IA8PeJbfu++m81oGgu6R7ULd24CpqYiwAkB/MFl9azyzi3cyoNH0dl86 Jhf9HO1STrmiowlPFWFLjC15z3Xy6Mg8MKi66qj6uC5E4yD7KaLwYLsfyBBhl/M1EL/h tOUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=d8Z+PWPkeXqU4sVnEZWxtzrZF8DqYxnDeZ23COUUVZo=; b=HW9b9xYWxc8TUwYzwvJ/emI1Vjclub+d3RP5jAwxIIvRX8Ul49xTF1E2FWepY26D7M IKN/rXvcRR0CDAIq8LjiWODrDOpKyAR+qmJXa7l773x4DmKHKxPPBaIz7vC/DUhohEjA nU9SRC/soqnesomOIqeGtd+r9tvfAiuKgs5uDixwmb96Cq+00OiVzFI6pPsE99ZSD5Pb 2vJmhQFXN2vQSTvxRsJn2yJK+2Zr14i6uVAZHZwv9Rgta9QtKIzBfE26hsy4Ves+3PK2 atbphCxDt66zd1rfomnoJBaYmSg3wphyyW41bvihk1yK+av6RfNDgfw57tj7a+mJZ6yk LKgw== X-Gm-Message-State: AOAM532m2OoVTuGr0nMPT6dFfmecsR0Gh217aKQtsarYI7XCzBxmWuUV 8BZMLMbGNR1c+VHPv427ZiJJPw== X-Google-Smtp-Source: ABdhPJxXUtVdGXDE1KDYt2fuo9xFsv8jd/AAg4JV8ZLoeAReCdNnvp+uKN7wG3+X2yEaCz8ncdCdAw== X-Received: by 2002:a05:620a:f92:: with SMTP id b18mr13123017qkn.146.1610727799140; Fri, 15 Jan 2021 08:23:19 -0800 (PST) Received: from localhost ([2620:10d:c091:480::1:dbed]) by smtp.gmail.com with ESMTPSA id h26sm4950319qtq.18.2021.01.15.08.23.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Jan 2021 08:23:17 -0800 (PST) Date: Fri, 15 Jan 2021 11:20:50 -0500 From: Johannes Weiner To: Michal Hocko Cc: Andrew Morton , Tejun Heo , Roman Gushchin , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH] mm: memcontrol: prevent starvation when writing memory.high Message-ID: References: <20210112163011.127833-1-hannes@cmpxchg.org> <20210113144654.GD22493@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210113144654.GD22493@dhcp22.suse.cz> 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: On Wed, Jan 13, 2021 at 03:46:54PM +0100, Michal Hocko wrote: > On Tue 12-01-21 11:30:11, Johannes Weiner wrote: > > When a value is written to a cgroup's memory.high control file, the > > write() context first tries to reclaim the cgroup to size before > > putting the limit in place for the workload. Concurrent charges from > > the workload can keep such a write() looping in reclaim indefinitely. > > > > In the past, a write to memory.high would first put the limit in place > > for the workload, then do targeted reclaim until the new limit has > > been met - similar to how we do it for memory.max. This wasn't prone > > to the described starvation issue. However, this sequence could cause > > excessive latencies in the workload, when allocating threads could be > > put into long penalty sleeps on the sudden memory.high overage created > > by the write(), before that had a chance to work it off. > > > > Now that memory_high_write() performs reclaim before enforcing the new > > limit, reflect that the cgroup may well fail to converge due to > > concurrent workload activity. Bail out of the loop after a few tries. > > I can see that you have provided some more details in follow up replies > but I do not see any explicit argument why an excessive time for writer > is an actual problem. Could you be more specific? Our writer isn't necessarily time sensitive, but there is a difference between a) the write taking a few seconds to reclaim down the requested delta and b) the writer essentially turning into kswapd for the workload and busy-spinning inside the kernel indefinitely. We've seen the writer stuck in this function for minutes, long after the requested delta has been reclaimed, consuming alarming amounts of CPU cycles - CPU time that should really be accounted to the workload, not the system software performing the write. Obviously, we could work around it using timeouts and signals. In fact, we may have to until the new kernel is deployed everywhere. But this is the definition of an interface change breaking userspace, so I'm a bit surprised by your laid-back response. > > Fixes: 536d3bf261a2 ("mm: memcontrol: avoid workload stalls when lowering memory.high") > > Cc: # 5.8+ > > Why is this worth backporting to stable? The behavior is different but I > do not think any of them is harmful. The referenced patch changed user-visible behavior in a way that is causing real production problems for us. From stable-kernel-rules: - It must fix a real bug that bothers people (not a, "This could be a problem..." type thing). > > Reported-by: Tejun Heo > > Signed-off-by: Johannes Weiner > > I am not against the patch. The existing interface doesn't provide any > meaningful feedback to the userspace anyway. User would have to re check > to see the result of the operation. So how hard we try is really an > implementation detail. Yeah, I wish it was a bit more consistent from an interface POV. Btw, if you have noticed, Roman's patch to enforce memcg->high *after* trying to reclaim went into the tree at the same exact time as Chris's series "mm, memcg: reclaim harder before high throttling" (commit b3ff92916af3b458712110bb83976a23471c12fa). It's likely they overlap. Chris's patch changes memory.high reclaim on the allocation side from reclaim once, sleep if there is still overage to reclaim the overage as long as you make forward progress; sleep after 16 no-progress loops if there is still overage Roman's patch describes a problem where allocating threads go to sleep when memory.high is lowered by a wider step. This is exceedingly unlikely after Chris's change. Because after Chris's change, memory.high is reclaimed on the allocation side as aggressively as memory.max. The only difference is that upon failure, one sleeps and the other OOMs. If Roman's issue were present after Chris's change, then we'd also see premature OOM kills when memory.max is lowered by a large step. And I have never seen that happening. So I suggest instead of my fix here, we revert Roman's patch instead, as it should no longer be needed. Thoughts?