From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 A90AF41D106 for ; Mon, 27 Jul 2026 21:14:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785186855; cv=none; b=S6XZDbYzei9jVemJ7UShW8AakIc2MwsaE87KRuOj5us38Bb6cFWkWK8ws9U3z0mhX4yCJ4UqO0Vt6f13D4iUXXYe7PAXZ6Q3wlYg2721YzXmsGupzXOXdXuZ8bB9xRHXKkJ0BD/pWTDP6iUrxq8cBcOE6DQM49WzCpiiXSUTRWU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785186855; c=relaxed/simple; bh=O6GATEpTBgtI673XATA8BQUpYcGylJ1VOaKJIB4nCuQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=P5o7JoQbGMI6umj+MYWWoanxtTITozWEdRPGTMuVC+2eSVyiwJniVWUCcteEZWAlPBgGWfnBZZfgktffDhSzddDk9qLmKtjMDwL9cdMSRoUP/qEwDsI8KkvFyTuCUrhTKvfT2mOXqVcJxRyWkEjbq1Fe3LBAZ3qBrGDBL2WaSkQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KUKMD0jm; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KUKMD0jm" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4295B1F000E9; Mon, 27 Jul 2026 21:14:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1785186854; bh=jDDLZVg0pWK3kfGm3YqYFtcQrrASDE9m+iAc5W2BT/k=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To; b=KUKMD0jmUEbdr5Bl9kcGMvcl/u9Uk8WH6l/lTEGnBJD3pSO9nqcmwjgrtLV9PxxyF Yur/NYH3HkUDWyjRX/iPGaKwEKvXzruecXfO6Lljc7NkrY5nFdKyRKg7dQBsN6/KKu 7y9ABQPuD89Sy7j6jW8YtamYO1iE7nWsVbpKuuqWjulfSym9NPNPEcXj+8swkBaaKw N6+7HmRsc3s0wu3eLIwF8nC5IWnfRNGpKAKFciXh23UWewonMjyN3u/HwYT3a7DseE v+FHqxvOstuimGQFPnMnfP80kklDSJj14Oig6WPPhgDZiM3Vdasp/Zd4XgGLQrvnZu LGCpl15pZ83IQ== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id EFB51CE08BC; Mon, 27 Jul 2026 14:14:13 -0700 (PDT) Date: Mon, 27 Jul 2026 14:14:13 -0700 From: "Paul E. McKenney" To: Andrew Morton Cc: Breno Leitao , David Hildenbrand , Zi Yan , Matthew Brost , Joshua Hahn , Rakie Kim , Byungchul Park , Gregory Price , Ying Huang , Alistair Popple , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH] mm/migrate: report RCU-tasks quiescent states in migrate_pages_batch() Message-ID: <482ba2d2-531e-4926-a03e-0f97a68573df@paulmck-laptop> Reply-To: paulmck@kernel.org References: <20260727-kcompact-v1-1-bdfefddd6874@debian.org> <20260727134950.2d091094dbb9bb3ecb7a3eb6@linux-foundation.org> 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 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20260727134950.2d091094dbb9bb3ecb7a3eb6@linux-foundation.org> On Mon, Jul 27, 2026 at 01:49:50PM -0700, Andrew Morton wrote: > On Mon, 27 Jul 2026 06:50:19 -0700 Breno Leitao wrote: >=20 > > migrate_pages_batch() unmaps each folio before moving it, and every > > unmap runs the mmu_notifier invalidate callbacks. On KVM hosts > > try_to_migrate() ends up in kvm_mmu_notifier_invalidate_range_start() -> > > tdp_mmu_zap_leafs(), which is expensive, so unmapping a large batch kee= ps > > the CPU busy for a long time. > >=20 > > The loop already calls cond_resched(), but on PREEMPTION kernels that is > > a no-op, and involuntary preemption is not a Tasks-RCU quiescent state. > >=20 > > A long batch therefore never reports a quiescent state, and the > > migrating task (e.g. kcompactd) becomes a Tasks-RCU holdout, stalling t= he > > Tasks-RCU grace period for minutes, which is common at Meta fleet: > >=20 > > INFO: rcu_tasks detected stalls on tasks: > > 0000000055349ecc: .. nvcsw: 1157401/1157401 holdout: 1 idle_cpu: -1/5= 6 task:kcompactd0 state:R running task > > Call Trace: > > tdp_mmu_zap_leafs > > tdp_mmu_next_root > > gfn_to_pfn_cache_invalidate_start > > kvm_mmu_notifier_invalidate_range_start > > __mmu_notifier_invalidate_range_start > > try_to_migrate_one > > try_to_migrate > > migrate_pages_batch > > migrate_pages > > compact_zone > > compact_node > > kcompactd > > kthread >=20 > Well I doubt if users of 7.1 kernels and earlier want to see this. So > a Fixes: and a cc:stable are needed. The affected code is quite old > and might even predate the addition of cond_resched_tasks_rcu_qs(). So > I can't begin to suggest a Fixes: target. Maybe omit it and let the > -stable team figure it out ;) >=20 > > --- a/mm/migrate.c > > +++ b/mm/migrate.c > > @@ -1843,7 +1843,7 @@ static int migrate_pages_batch(struct list_head *= =66rom, > > is_thp =3D folio_test_pmd_mappable(folio); > > nr_pages =3D folio_nr_pages(folio); > > =20 > > - cond_resched(); > > + cond_resched_tasks_rcu_qs(); > > =20 >=20 > Totally off-topic but why the heck was that implemented as a macro.=20 > Which invokes another macro and another and another and turtles all the > way down. End result: >=20 > do { do { if (!((false)) && ({ do { __attribute__((__noreturn__)) exte= rn void __compiletime_assert_606(void) __attribute__((__error__("Unsupporte= d access size for {READ,WRITE}_ONCE()."))); if (!((sizeof(((current))->rcu_= tasks_holdout) =3D=3D sizeof(char) || sizeof(((current))->rcu_tasks_holdout= ) =3D=3D sizeof(short) || sizeof(((current))->rcu_tasks_holdout) =3D=3D siz= eof(int) || sizeof(((current))->rcu_tasks_holdout) =3D=3D sizeof(long)) || = sizeof(((current))->rcu_tasks_holdout) =3D=3D sizeof(long long))) __compile= time_assert_606(); } while (0); (*(const volatile __typeof_unqual__(((curre= nt))->rcu_tasks_holdout) *)&(((current))->rcu_tasks_holdout)); })) do { do = { __attribute__((__noreturn__)) extern void __compiletime_assert_607(void) = __attribute__((__error__("Unsupported access size for {READ,WRITE}_ONCE()."= ))); if (!((sizeof(((current))->rcu_tasks_holdout) =3D=3D sizeof(char) || s= izeof(((current))->rcu_tasks_holdout) =3D=3D sizeof(short) || sizeof(((curr= ent))->rcu_tasks_holdout) =3D=3D sizeof(int) || sizeof((( > current))->rcu_tasks_holdout) =3D=3D sizeof(long)) || sizeof(((current))= ->rcu_tasks_holdout) =3D=3D sizeof(long long))) __compiletime_assert_607();= } while (0); do { *(volatile typeof(((current))->rcu_tasks_holdout) *)&(((= current))->rcu_tasks_holdout) =3D (false); } while (0); } while (0); } whil= e (0); ({ __might_resched("mm/migrate.c", 1846, 0); _cond_resched(); }); } = while (0); >=20 >=20 > How much nicer would it be to have >=20 > static inline void cond_resched_tasks_rcu_qs(void) > { > if (some brief and efficient test) > some_slow_uninlined_thing(); > } Sigh. It used to be so simple, then people wanted error checks in READ_ONCE() and WRITE_ONCE(). ;-) The point of rcu_tasks_classic_qs() being a macro was to avoid #include hell. But maybe cond_resched_tasks_rcu_qs() can be a static inline, as shown below. It builds, so it must be perfect, right? On the other hand, does that really help, given that cond_resched_tasks_rcu_qs() gets inlined? Thanx, Paul ------------------------------------------------------------------------ diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index c77b1e02a93cca..9a741ce0588575 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -217,15 +217,15 @@ static inline void exit_tasks_rcu_finish(void) { } /** * cond_resched_tasks_rcu_qs - Report potential quiescent states to RCU * - * This macro resembles cond_resched(), except that it is defined to + * This function resembles cond_resched(), except that it is defined to * report potential quiescent states to RCU-tasks even if the cond_resched= () * machinery were to be shut off, as some advocate for PREEMPTION kernels. */ -#define cond_resched_tasks_rcu_qs() \ -do { \ - rcu_tasks_qs(current, false); \ - cond_resched(); \ -} while (0) +static inline void cond_resched_tasks_rcu_qs(void) +{ + rcu_tasks_qs(current, false); + cond_resched(); +} =20 /** * rcu_softirq_qs_periodic - Report RCU and RCU-Tasks quiescent states