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 65BF8C25B75 for ; Wed, 29 May 2024 17:09:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B41676B0095; Wed, 29 May 2024 13:09:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF08E6B0098; Wed, 29 May 2024 13:09:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A07286B0099; Wed, 29 May 2024 13:09:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 86FF06B0095 for ; Wed, 29 May 2024 13:09:07 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 4817A1C0B45 for ; Wed, 29 May 2024 17:09:07 +0000 (UTC) X-FDA: 82172068734.19.9DAA1DF Received: from out-177.mta0.migadu.com (out-177.mta0.migadu.com [91.218.175.177]) by imf20.hostedemail.com (Postfix) with ESMTP id 0C23D1C0019 for ; Wed, 29 May 2024 17:09:03 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=a6US2YVd; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf20.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.177 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717002544; 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=ZCyl+E3MJ9UNj67UrMAAlRB5ugKBQHQxC1sMp2jyKYU=; b=YluCSBt/0yeyoGPVCK434hyfW3Yhpist2KXdN0WBm3Q/rilx5k92QprsufaEignYA35CCD Bv6ZZvYKtkauyFKBUJiNq6x3Fzn+S81je1fQGYjSipli5sF4YlVhZHvUBi6GmdEeblrGrk rwZGnvlkgizKLcUAea4jKYHqvfpQRuA= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=a6US2YVd; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf20.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.177 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717002544; a=rsa-sha256; cv=none; b=gVNRdJeTHN6I6fYjlLwMo9MblN0/owVC2gWrhaZAEvvlG9gvFp8QGJluMHabBn9ILzyybv n2bZWpOEGIHHdUOyhr3NBzGym0Xm3d8QoTW2HLLJPqwFl2MJBZhXGeyXSqJasJzMMwoRTw zEJX66wTg/LT84UuWFKNFyv9LAYrTmU= X-Envelope-To: roman.gushchin@linux.dev DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1717002541; h=from:from:reply-to:subject:subject: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=ZCyl+E3MJ9UNj67UrMAAlRB5ugKBQHQxC1sMp2jyKYU=; b=a6US2YVdl7pQ30ccWBeJx7P8rObolUkRlZPTlrWaVFW9Za+sSLp7IPwgQud2q5KqOH5rCp ZtK/3TzaBlWaS4qYfsU2qINVjL6CTXJC3wmfXtuiKiqNOFlYFisWGLw2fQ6O6lIiqMu6u5 PNMmDV3kyI3Gt9tQFWlfGYej0alrY1U= X-Envelope-To: akpm@linux-foundation.org X-Envelope-To: hannes@cmpxchg.org X-Envelope-To: riel@surriel.com X-Envelope-To: mhocko@kernel.org X-Envelope-To: kernel-team@fb.com X-Envelope-To: linux-mm@kvack.org X-Envelope-To: linux-kernel@vger.kernel.org X-Envelope-To: syzbot+17416257cb95200cba44@syzkaller.appspotmail.com Date: Wed, 29 May 2024 10:08:56 -0700 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Roman Gushchin Cc: Andrew Morton , Johannes Weiner , Rik van Riel , Michal Hocko , Facebook Kernel Team , linux-mm@kvack.org, linux-kernel@vger.kernel.org, syzbot+17416257cb95200cba44@syzkaller.appspotmail.com Subject: Re: [PATCH] mm: vmscan: reset sc->priority on retry Message-ID: <3rtsgmhjxlaafdw7zeasyt4voa77mzpb32s6tsdz4pj4jglq4p@uuekegdnsolw> References: <20240529154911.3008025-1-shakeel.butt@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 0C23D1C0019 X-Stat-Signature: 1k64cm4pp58ynjz1ss4sx5pb6sqcw9cc X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1717002543-544453 X-HE-Meta: U2FsdGVkX1+0jkvSE9pZAA19HgpslbZlj3ve2j4FbWtGUhP4jfwe58vRQMr1oTv/3ZSIhOnLoKrPiikgs3QcH9n2OOwFmQXv7eGM8CEO9TC0Luh1TBH493epzN/tJHUEVALZCMvMigvKrTZautia1tkFKq8WqbVYEOvj5ZfApbI3ysW9A3dPYR/zvdIZYFJBVaw56cJwJGes+Djv5WXTEHGlT4dAQTgU4V6hN8T+Y1T0kXJHoNBCOLnviHk2Jx6MRX/2itSE46PEMYY5Y01Oy/1fSPBmaSXD5l4Gq8DSdbZSZzJ2ynJP1SLUYiBqfG0O34Bz0PVTzelEC5g4BaAmZG0/rGT9LbnTA0aOHlLdPexDW7lhc8uEBQiJylsCKiQNcOul2udWs08DiEsxJiFI+dOagBbC2U2dLyBeM/B5Fn+E82Jqc/tdIDT+ExZWi1LZO8HkT2Szu13uBbn2Bs+or1E1pa8R25U1GsElZbp8NdJnH6YC9biNnXNS43gD3X3DO85BwdEB2QYx43+O1iIJjl3F77soDXWVBdSEtUe6R76sAjN+fv7FGme8a/bLGkQOhdCy9+Fig9Q8RF6FCfoQKhi8Hnm/9vzWRGcI6y9izLkL7+2PyNjrAUXeChjbNk+U+NTqx1ihJEeprMDJuAD5o5zDBAb96c+/F/SPXll+o/eKca3OEt/Nkl8bWQmBDRG4nSyOnDQuwWEIcUv7005nzvk+OmWkLNZGvunnkum35BEQB26fz6HhrGrEh7ooiWPGiVA6NhfN3PIJIKJF/ALYYyVWM4Vn0dEumDAAJgylItSp4vjyFZeNGTkwtERe4Z1NHJ1dwfGvDnYypXVDUnFuu2cJ/1glkf8HWlBeuDfkqe9CtD8x4cC7t2yez8QB+gkJYxUprCbA6IGDJSKDzKPzHXBDi+NTHyFKFL/0ZSH97mqaRbrQnaNiuAHZBkFcoZsdr/y6r6uJLeXjY6UHFM5 RAeMpp+X 4UHXppvp34diwtzE3XFndDTBfWdZpdCdJV0yrFGjMJSRjAwx6INNk8eyU7XZV0x57ivvNBTQa6fHjOR3OUreEpdh2AaytiByHuavTZGsE9plz3OStDC7YfnoQHXF+dUa9/ZQN1iSMRGfLjwzgVxf06BVa02rujRr7A61bBAaWJatPOlKR8FQpTpb63IoF7wDAJRZPnD14qeNaGCv1212GobudnAGxVbtfd8xqNVdXV3jJdbAQuAWhAhF3vg== 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, May 29, 2024 at 09:20:46AM GMT, Roman Gushchin wrote: > On Wed, May 29, 2024 at 08:49:11AM -0700, Shakeel Butt wrote: > > The commit 6be5e186fd65 ("mm: vmscan: restore incremental cgroup > > iteration") added a retry reclaim heuristic to iterate all the cgroups > > before returning an unsuccessful reclaim but missed to reset the > > sc->priority. Let's fix it. > > > > Reported-and-tested-by: syzbot+17416257cb95200cba44@syzkaller.appspotmail.com > > Fixes: 6be5e186fd65 ("mm: vmscan: restore incremental cgroup iteration") > > Signed-off-by: Shakeel Butt > > Reviewed-by: Roman Gushchin > > Good catch! Thanks. > > > --- > > mm/vmscan.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index b9170f767353..731b009a142b 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -6317,6 +6317,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist, > > * meaningful forward progress. Avoid false OOMs in this case. > > */ > > if (!sc->memcg_full_walk) { > > + sc->priority = initial_priority; > > sc->memcg_full_walk = 1; > > goto retry; > > } > > -- > > 2.43.0 > > > > I wonder if it makes sense to refactor things to be more robust like this: Oh I like this as it will make sc->priority values explicit. I hope we don't have any hidden dependency on do-while semantics for this code path. > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index d3ae6bf1b65c7..f150e79f736da 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -6246,7 +6246,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist, > if (!cgroup_reclaim(sc)) > __count_zid_vm_events(ALLOCSTALL, sc->reclaim_idx, 1); > > - do { > + for (sc->priority = initial_priority; sc->priority >= 0; sc->priority--) { > if (!sc->proactive) > vmpressure_prio(sc->gfp_mask, sc->target_mem_cgroup, > sc->priority); > @@ -6265,7 +6265,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist, > */ > if (sc->priority < DEF_PRIORITY - 2) > sc->may_writepage = 1; > - } while (--sc->priority >= 0); > + } > > last_pgdat = NULL; > for_each_zone_zonelist_nodemask(zone, z, zonelist, sc->reclaim_idx, > @@ -6318,7 +6318,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist, > * good, and retry with forcible deactivation if that fails. > */ > if (sc->skipped_deactivate) { > - sc->priority = initial_priority; > sc->force_deactivate = 1; > sc->skipped_deactivate = 0; > goto retry; > @@ -6326,7 +6325,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist, > > /* Untapped cgroup reserves? Don't OOM, retry. */ > if (sc->memcg_low_skipped) { > - sc->priority = initial_priority; > sc->force_deactivate = 0; > sc->memcg_low_reclaim = 1; > sc->memcg_low_skipped = 0;