From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roman Gushchin Subject: Re: [PATCH] mm: memcontrol: prevent starvation when writing memory.high Date: Wed, 13 Jan 2021 10:06:44 -0800 Message-ID: <20210113180620.GA353910@carbon.lan> References: <20210112163011.127833-1-hannes@cmpxchg.org> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=date : from : to : cc : subject : message-id : references : content-type : in-reply-to : mime-version; s=facebook; bh=Tqk3s2gbpL7IEpMXx5QPwfd2m6Xl9/uAFYgdXH/d59k=; b=gNZVLsqO5R99DCECsq9hNLTTDhM4yZS6qMGfIcDhcDT8qV06EDTSouPaP1IoAsC9P5NA UPnV/ryBOR9qfrLkAOG+Goow25pWZptMLDhpGCZxW8j7YRxVITR4yBp/zuPPIDKzYJyU LLFNAwKp2dhEhhHmr6aHWZgmtZL67Bt78NU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.onmicrosoft.com; s=selector2-fb-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Tqk3s2gbpL7IEpMXx5QPwfd2m6Xl9/uAFYgdXH/d59k=; b=QCuZxlsWe2k6VrTbephjDi795JM88LcVOGCyz+zzzKYmF13ftylB8isTI5Ga2tivSXsfSM+quSJdt6o4WgwJozjzDdIVHBPKspn/ct8s4Fp+SHYTFWQ25hTI6qEIJVUqKID3D2IAg4J+8I9Pd43pPVfL9f/+i8J4wb8StieAzqk= Content-Disposition: inline In-Reply-To: <20210112163011.127833-1-hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org> List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Johannes Weiner Cc: Andrew Morton , Tejun Heo , Michal Hocko , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-team-b10kYP2dOMg@public.gmane.org On Tue, Jan 12, 2021 at 11:30:11AM -0500, 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. > > Fixes: 536d3bf261a2 ("mm: memcontrol: avoid workload stalls when lowering memory.high") > Cc: # 5.8+ > Reported-by: Tejun Heo > Signed-off-by: Johannes Weiner > --- > 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 > Acked-by: Roman Gushchin Thanks!