From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from shelob.surriel.com (shelob.surriel.com [96.67.55.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 657841EA90 for ; Thu, 19 Dec 2024 03:14:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=96.67.55.147 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734578092; cv=none; b=TecRbix0UJccQKIzvPWBTdKC3g6lkCPcvZ53DM64xlhyOD7sYke0TQbxWrhIRbASZz6x1t7+an6QX8dYxwUP1pst9sJoec4P7hrYus2YbgZNuwLRzN2w4sde5Uzc+Vfa0RgeqQhqv6fAxTaOZMQatfFacQM1pYpaAqke0RTqEbE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734578092; c=relaxed/simple; bh=WCuoA13fJ0nj/CPUgkvnxHZx5Kzk2Tt+OOD4oN2a930=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=UrGtH9Ag6GbrCDKyh/aT8ew926ca2R+Z6/tRfNfGM0z23suLYAm3KKAWW5v8aiuRsCdGRL18g2x0MgAopq6IGXKltMIXLNlDxG50lUws0DihruTWdNwGt4XiT2klbXr78D3tp9Wpbpe6KJADP2v+6ggcSFm+OXhEXG2X0Rsv83I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=surriel.com; spf=pass smtp.mailfrom=shelob.surriel.com; arc=none smtp.client-ip=96.67.55.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=surriel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=shelob.surriel.com Received: from fangorn.home.surriel.com ([10.0.13.7]) by shelob.surriel.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.97.1) (envelope-from ) id 1tO6yg-0000000074c-1znL; Wed, 18 Dec 2024 22:13:30 -0500 Message-ID: Subject: Re: [PATCH] mm: add maybe_lru_add_drain() that only drains when threshold is exceeded From: Rik van Riel To: Shakeel Butt Cc: Andrew Morton , "Huang, Ying" , Chris Li , Ryan Roberts , David Hildenbrand , "Matthew Wilcox (Oracle)" , linux-kernel@vger.kernel.org, linux-mm@kvack.org, kernel-team@meta.com Date: Wed, 18 Dec 2024 22:13:30 -0500 In-Reply-To: <43o2dqigz6cap75h7y25jz6qbdzoinyq3ntxx4sm5cn3y4dddm@mwyjjygrxhmm> References: <20241218115604.7e56bedb@fangorn> <43o2dqigz6cap75h7y25jz6qbdzoinyq3ntxx4sm5cn3y4dddm@mwyjjygrxhmm> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.54.1 (3.54.1-1.fc41) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Sender: riel@surriel.com On Wed, 2024-12-18 at 12:20 -0800, Shakeel Butt wrote: > On Wed, Dec 18, 2024 at 11:56:04AM -0500, Rik van Riel wrote: > [...] > > =C2=A0 > > +static bool should_lru_add_drain(void) > > +{ > > + struct cpu_fbatches *fbatches =3D > > this_cpu_ptr(&cpu_fbatches); >=20 > You will need either a local_lock or preempt_disable to access the > per > cpu batches. >=20 Why is that? Can the per-cpu batches disappear on us while we're trying to access them, without that local_lock or preempt_disable? I'm not trying to protect against accidentally reading the wrong CPU's numbers, since we could be preempted and migrated to another CPU immediately after returning from should_lru_add_drain, but do want to keep things safe against other potential issues. > > + int pending =3D folio_batch_count(&fbatches->lru_add); > > + pending +=3D folio_batch_count(&fbatches->lru_deactivate); > > + pending +=3D folio_batch_count(&fbatches- > > >lru_deactivate_file); > > + pending +=3D folio_batch_count(&fbatches->lru_lazyfree); > > + > > + /* Don't bother draining unless we have several pages > > pending. */ > > + return pending > SWAP_CLUSTER_MAX; > > +} > > + > > +void maybe_lru_add_drain(void) >=20 > Later it might also make sense to see if other users of > lru_add_drain() > should be fine with maybe_lru_add_drain() as well. Agreed. I think there are a few other users where this could make sense, including munmap. --=20 All Rights Reversed.