From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm2-f12.google.com (mail-wm2-f12.google.com [74.125.225.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8506C30ACEE for ; Sat, 12 Sep 2026 22:00:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.225.140 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789250421; cv=none; b=gSPWfx4WQ4vd/e5VqeKB3blIojAW9OHG6H/LthYmuO77xYHcIra28TtfJoK5ZMKae/CPWihFX3ukMtrcoCwSn0FIdgE6MH8+kMub0SLRtWeIGORflDR3UuR1ZUh0Mm8+ZnSWP71zSVAy8sgn4i5SOy6tbY8jYIKZ1i1UULWEdYc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789250421; c=relaxed/simple; bh=VSjoq7DHQbKtAdXTo08YR1Ci+Z/OuPpj5YnrQdVytbE=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=IG2mFzq59OUqvBHxm0eF5Ok+cUXEdqcO0qThD3lvm3GmNVVFg1i8p6KQmTDJZJQa7pPcB8PahAX3900F+fp4qvh0JLWgZ4ZBPLPBfGpw4G05kCRsIPEapyKcBE16I6N+lK9O1Wsh+V56WzN+r8Hr5MQjPKxSP1mVq3DhXXzE+fI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=Tvpxxhm9; arc=none smtp.client-ip=74.125.225.140 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Tvpxxhm9" Received: by mail-wm2-f12.google.com with SMTP id 5b1f17b1804b1-49cd6185db7so4744105e9.1 for ; Sat, 12 Sep 2026 15:00:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1789250418; x=1789855218; darn=vger.kernel.org; h=content-type:mime-version:references:message-id:in-reply-to:subject :cc:to:from:date:from:to:cc:subject:date:message-id:reply-to :content-type; bh=ShFFlTNrkhtcTY1WEEF9s6hCAewZPx0n3fIe5hLvRZo=; b=Tvpxxhm9rJ8WYRsBiT/xjrzlhS8DbT5GvMsIGw4HPVU5QKIJmMRgcsppKv9nv2phYu Smm/6rEmmKyBtuARBmoOEVXmW5ZriVYBlP7EEQ5PKrRkG742u4jbetZ/Dfqp9yyn60+3 SKhPb2LbNnF9MLFn2nUFE29SIgp+5w/pG9aboVgbVSmhCsSjzx3/3b9Q+/C5gM3QGpnc CIdR2WGwKyN/4T2f6Ds7cHMU4y8vdDgqRtdmwmapMz3PA5SIOaMtgB+8hAkpwk/+HNEF gYeASik22FtEXcjOZFnKZEEZ0z2+femILPQUMSHbk2C14J9qhK4aKSwtCDujKL42xvcG e1XA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1789250418; x=1789855218; h=content-type:mime-version:references:message-id:in-reply-to:subject :cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=ShFFlTNrkhtcTY1WEEF9s6hCAewZPx0n3fIe5hLvRZo=; b=W8292vijOMi5j+rx2wDHXLG4fV1Ag4/a6HbE/x4muBV18IhMyCwO9YnhfCYKVCJgsE Q8waxkb+RetlTQ049f6JiHVwyQZRPsossKKUs2bNrstOlNvCecYKaXvdI4wjY+xzPK0E hGg5q3o3fqkR4D1S33z9276gpOgFaR63kKsHYJb3en1SsmpV3v6BS+27f9x+E4KXR5h2 QZdKOaMUGvoYouBeNuMG3u41e86iE7BXxJ5Bv7G6Nuk819bgTfjw5kqg9Ed82rMFZ2mN Jal5iQa5DupSaTOhlb9rKdcfnvRG92dimVQWSRqIXFvTgNPkuUsUksoDCJIFiZkMgFJM fxlg== X-Forwarded-Encrypted: i=1; AKwUvBw4aCbYLXSVsOaRI+2UtDVfZuN5lBC/ZauSoe9psxbVL0ra5hHxABBHyiq0O5FZHo+N8ebnD0OxQz/vkvY=@vger.kernel.org X-Gm-Message-State: AFuF++nbebrFTM+iA94WYyd8iz+SY6ZXxVgSlnCQqLtdGt/wQZKkE8S6 5BnSK9pHnXepabBEhpMM7n6jay9CyUtcvQNddRgAwf1Fo/ha4VILRrQcFk9s8k0HnA== X-Gm-Gg: AYBFou2DlYwWBmvf3qmixcaIVVOFZDOKtEE7yNH8yXkaU8XDWBUqR5vxY3HxY0QMsrq FxqZlwu/7GuU7d/Bmu/ixb6npbaNeBwKg9D1fezjAl1VDWETMiXo5XLZdlrpT5pKs5uIhG02zuK zv0d11fvRcF/nZpbcKS5u2RtdRqKQdi6TEEe3/ztBYcVxEOya0QJ8WOadFNP8GgxlyRrq4urw3K Z9C44f/l8Vg+u5akZNlOmSzb6/+3YL+Kz0/rEAhnjXxgUoeWST3uA64RyNlTrTzu8rlGJgNP+az KSeuPI5uCdHxn5iXKk2hduLFjPmKKplPFd76S8eKWmNoO2l+AuAXHnFABV3opF93+OZIM2wmWg8 9xyxHu04KdMJ+ob0/+TQI4VpTqqp3vGkJ8TFG6MjZRaoDT5KLmxKrlxZ6Bx96haJjwZpTmf2tkV Zh20vFE6JN6kxGECLBFc+jb07uvcSy0Mf6sjtTO0S3YBBg5iq5NSY9Jo/Y8t8P3wTmCP4ukIUHM 3vEEcFbBiEn9CGtMxHI X-Received: by 2002:a05:600c:3115:b0:49d:1e79:35d6 with SMTP id 5b1f17b1804b1-49e61094b9bmr117724205e9.14.1789250416975; Sat, 12 Sep 2026 15:00:16 -0700 (PDT) Received: from darker.lan (104.157.125.91.dyn.plus.net. [91.125.157.104]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49e64221639sm67265195e9.4.2026.09.12.15.00.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 12 Sep 2026 15:00:16 -0700 (PDT) Date: Sat, 12 Sep 2026 14:59:23 -0700 (PDT) From: Hugh Dickins To: "Vlastimil Babka (SUSE)" cc: Hugh Dickins , Andrew Morton , Ackerley Tng , Alexander Viro , Alexandre Ghiti , Baolin Wang , Barry Song , Binbin Wu , Christian Brauner , Christoph Hellwig , Christoph Lameter , Claudio Imbrenda , David Hildenbrand , JP Kobryn , Jan Kara , Jens Axboe , Johannes Weiner , Kairui Song , Kiryl Shutsemau , Lance Yang , Leonardo Bras , Lorenzo Stoakes , Marcelo Tosatti , Matthew Wilcox , Mel Gorman , Miaohe Lin , Michal Hocko , Minchan Kim , Muchun Song , Oscar Salvador , Peter Zijlstra , Qi Zheng , Rik van Riel , Sebastian Andrzej Siewior , Shakeel Butt , Suren Baghdasaryan , Yang Shi , Yu Zhao , Zach O'Keefe , Zi Yan , linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v2 05/26] mm/fbatch: lru_add_del_folio()+folio_add_lru() after clear_lru() In-Reply-To: Message-ID: <5c943056-cf6b-788f-aa45-b7c0da28ecf2@google.com> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Wed, 9 Sep 2026, Vlastimil Babka (SUSE) wrote: > On 9/9/26 11:51, Hugh Dickins wrote: > > Most callers of folio_test_clear_lru() then proceed to remove the folio > > from its lru, and add it back at the end when they're done (if still in > > use). But isolate_migratepages_block() and check_move_unevictable_pages() > > sometimes decide against, and release immediately with a folio_set_lru(). > > > > Which usually works fine: but there's now a small chance that while they > > held the folio with lru bit cleared, an lru_add fbatch drain came along, > > and had to skip that folio because its lru bit was transiently cleared > > (previously, the lru_add fbatch drain relied on finding lru bit never yet > > set). This risks leaving that folio off lru, unreclaimable until freed. > > So this makes the previous patch a somewhat bisection hazard? I guess it's > acceptable given it's not fatal. Not what I would call a bisection hazard. Yes, the preceding patch is not perfect, but more reviewable that way, and then come corrections to edge cases best considered by themselves. Nobody bisecting unrelated issues would get held up by this gap, and it won't crash any bisections. > > > Fix such cases by trying lru_add_del_folio() (which only takes action and > > returns true if the folio was on an lru_add fbatch), then folio_add_lru() > > Oh ok, that's one detail I didn't realize on the previous patch, and > explains the name of the function. But it's still IMHO confusing. > > > if it succeeded: invalidating the old fbatch slot, appending in a new one. > > > > Signed-off-by: Hugh Dickins > > In general, LGTM. > Reviewed-by: Vlastimil Babka (SUSE) Thanks. > > Nit below: > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index f11491ee9ed5..4e8d5cc34f07 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -8093,17 +8093,19 @@ void check_move_unevictable_folios(struct folio_batch *fbatch) > > folio_clear_unevictable(folio); > > lruvec_add_folio(lruvec, folio); > > pgrescued += nr_pages; > > + } else if (lru_add_del_folio(folio)) { > > + lruvec_unlock_irq(lruvec); > > + folio_add_lru(folio); > > + lruvec = NULL; > > } > > - folio_set_lru(folio); > > + if (lruvec) > > + folio_set_lru(folio); > > } > > > > - if (lruvec) { > > - __count_vm_events(UNEVICTABLE_PGRESCUED, pgrescued); > > - __count_vm_events(UNEVICTABLE_PGSCANNED, pgscanned); > > + if (lruvec) > > lruvec_unlock_irq(lruvec); > > - } else if (pgscanned) { > > - count_vm_events(UNEVICTABLE_PGSCANNED, pgscanned); > > - } > > + count_vm_events(UNEVICTABLE_PGRESCUED, pgrescued); > > + count_vm_events(UNEVICTABLE_PGSCANNED, pgscanned); > > AFAIU this is done because we can no longer rule out that !lruvec means > pgrescued is 0. > But we can still distinguish the cheaper __count_vm_events vs > count_vm_events? Probably all the same on x86, but I hear on arm64 this_cpu* > ops have a cost worth proposing rather elaborate schemes to deal with... Yes, it was just looking a bit baroque to still be deciding whether to use the __count or the count there. Could be done of course, and with "if (pgrescued)" and "if (pgscanned)"; but I haven't noticed anywhere else in the source where we go to such lengths to use __count versus count, and I don't think this is on anyone's hotpath (IIRC this is just SHM_UNLOCK). Now you've got me worried, no, fractionally worried, about Shakeel's recent __count to count fix to NR_MLOCK. I am much more familiar with x86, and have noticed the recent tussles over improving arm64 this_cpus, so that confirms you're right; but I'd look for juicier low-hanging fruit than this, if we're going to embark on an "if (x) __count() else count()" spree. Hugh