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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,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 2CC38C433E0 for ; Wed, 13 Jan 2021 14:46:59 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 999DC2343E for ; Wed, 13 Jan 2021 14:46:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 999DC2343E Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1B78F8D005E; Wed, 13 Jan 2021 09:46:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 168738D005D; Wed, 13 Jan 2021 09:46:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 057318D005E; Wed, 13 Jan 2021 09:46:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0190.hostedemail.com [216.40.44.190]) by kanga.kvack.org (Postfix) with ESMTP id E24EC8D005D for ; Wed, 13 Jan 2021 09:46:57 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id AB5381F06 for ; Wed, 13 Jan 2021 14:46:57 +0000 (UTC) X-FDA: 77701028874.21.day88_4b012af2751e Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin21.hostedemail.com (Postfix) with ESMTP id 8128518079EB8 for ; Wed, 13 Jan 2021 14:46:57 +0000 (UTC) X-HE-Tag: day88_4b012af2751e X-Filterd-Recvd-Size: 4876 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf36.hostedemail.com (Postfix) with ESMTP for ; Wed, 13 Jan 2021 14:46:56 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1610549215; 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=nt0P3UB52tUXihhKnY0EGJSOwPIXl2w0VfMCeDXigfg=; b=MaYTpfeBCSb+4GfytJrC3yMQ85mJn3wsQrdPMm3X8mJpdVwihUs+pt5UikXfQ4Js75AVXa p+88RPnduouIF3z9abzeMfvM3SCYKBhdqRIhhKakryDlNAwM89t/a65OMRWqoBMjPH7Fg8 YApkvhaPiozFN1PYsTU6iJ2/ww75CGU= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 62A00AB92; Wed, 13 Jan 2021 14:46:55 +0000 (UTC) Date: Wed, 13 Jan 2021 15:46:54 +0100 From: Michal Hocko To: Johannes Weiner 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: <20210113144654.GD22493@dhcp22.suse.cz> References: <20210112163011.127833-1-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210112163011.127833-1-hannes@cmpxchg.org> 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 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? If the writer is time sensitive then there is a trivial way to workaround that and kill it by a signal (timeout 30s echo ....). Btw. this behavior has been considered http://lkml.kernel.org/r/20200710122917.GB3022@dhcp22.suse.cz/ " With this change the reclaim here might be just playing never ending catch up. On the plus side a break out from the reclaim loop would just enforce the limit so if the operation takes too long then the reclaim burden will move over to consumers eventually. So I do not see any real danger. " > 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. > 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. > --- > mm/memcontrol.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 605f671203ef..63a8d47c1cd3 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6275,7 +6275,6 @@ static ssize_t memory_high_write(struct kernfs_open_file *of, > > for (;;) { > unsigned long nr_pages = page_counter_read(&memcg->memory); > - unsigned long reclaimed; > > if (nr_pages <= high) > break; > @@ -6289,10 +6288,10 @@ static ssize_t memory_high_write(struct kernfs_open_file *of, > continue; > } > > - reclaimed = try_to_free_mem_cgroup_pages(memcg, nr_pages - high, > - GFP_KERNEL, true); > + try_to_free_mem_cgroup_pages(memcg, nr_pages - high, > + GFP_KERNEL, true); > > - if (!reclaimed && !nr_retries--) > + if (!nr_retries--) > break; > } > > -- > 2.30.0 > -- Michal Hocko SUSE Labs