Received-SPF: pass (google.com: domain of linux-kernel+bounces-159478-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 26 Apr 2024 11:41:25 +0800
From: Chen Yu <yu.c.chen@intel.com>
To: Sean Christopherson <seanjc@google.com>
CC: Jonathan Corbet <corbet@lwn.net>, Ingo Molnar <mingo@redhat.com>, "Peter
 Zijlstra" <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>, Will Deacon
	<will@kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, "Valentin Schneider"
	<valentin.schneider@arm.com>, Marco Elver <elver@google.com>, Frederic
 Weisbecker <frederic@kernel.org>, David Matlack <dmatlack@google.com>,
	Friedrich Weber <f.weber@proxmox.com>, Ankur Arora
	<ankur.a.arora@oracle.com>, Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2 2/2] sched/core: Drop spinlocks on contention iff
 kernel is preemptible
Message-ID: <ZisiZUWgERCZbc9w@chenyu5-mobl2>
References: <20240312193911.1796717-1-seanjc@google.com>
 <20240312193911.1796717-3-seanjc@google.com>
 <ZioJN6ClnlFIQIBg@chenyu5-mobl2>
 <ZiqJOJEBshsQLl0M@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZiqJOJEBshsQLl0M@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ljwOxsaDvSe3rSMBX+PnCk8NNoYFwhFsqruBw3SS01FKp05WUT7QgZOJYbqH?=
 =?us-ascii?Q?SryRGp7az0Se8tPVetkk2QvgSYZX2mRF6AEAI3QbEssY2kh7eWzAj+7romza?=
 =?us-ascii?Q?E/jZdUUohR3gpdzf6ZGmSXohxOw9apr79MCnaMj3KHr7Y+N1VkuQv2SXbAPp?=
 =?us-ascii?Q?YePkBXa+s0JhirFRaGhkaxW74XHD5yudBG5l/U0YEcX9rDmTp8o6q19ZbLzh?=
 =?us-ascii?Q?2Ag8V9h92WI16AvJ5mKm8zTt7QVYj+I6hu7kdnXxXkC5H1P7RufZoGCEgr1r?=
 =?us-ascii?Q?8W6zEmNQoynUPdIzWlwM+BW0fwMVk28+qBLyWUaXycdmxg0bQygYtTlx7JqF?=
 =?us-ascii?Q?BVWXnUHyjPxL0AQF+g+jlx8uVWI211AEJI4zeSUu/0GA6XufeTVevhVqNlQg?=
 =?us-ascii?Q?oQbqtGY+8RqZBcFtVy5pa4/ws8Dc0XD4at7KJ056E+S0uiwI6P+kdU3YpW7e?=
 =?us-ascii?Q?KuJsrJRl+oK/H4PFCxypstUjpc3BDLvnJjkabaKMzM9YQjzIAvzf9GiGaChT?=
 =?us-ascii?Q?xf1TP1Au4s+QdGcjCzTxdzfBi5v70pivxHyI7D+NpH3QjiJkUVe8SGbJWlWv?=
 =?us-ascii?Q?dZXsY0hP4pNk3xyEcU3u8fkpTx5gS9e6VRTHGe/7lXXPnD3w9eMn1fL24n5U?=
 =?us-ascii?Q?Erktv+fbf7U6zpTESf3JdE4V4cKYBOdhWfUaRceyLM0L/MbhdDUNCFdDci2D?=
 =?us-ascii?Q?0NDsE1xZoqVWUoOsE/7oxzgZjkxgE9N8z5SxQQtvNfY57M30WAx3vHGgn39o?=
 =?us-ascii?Q?sag43Qv8wKLNZxeFvDeiXarplXChA4bobEJcuLAFOtaBxON3ZKm6IzN50VUJ?=
 =?us-ascii?Q?zXcZP/9yYLSsrHtqtEY8rxfy/2+oPLecSBNAGj9eU0JcepP3VQe/TVLTqabD?=
 =?us-ascii?Q?z+WOZoZ9lVpEUO2o0ERCWra3jhtNKCp0npP1m5UupDIsoXosX3mYeqxXrNhz?=
 =?us-ascii?Q?ELG/dx8XsUdsT9gD+6B16S90vxFviN1rGYD5p9d9x+pFM+D7gmj7v7CimDME?=
 =?us-ascii?Q?X3MkqfdebURrvElu8uAh+wtsjo96FlN8Tf3kLA4OU8ejjMY21m0YfCYZ5jm7?=
 =?us-ascii?Q?rhugJIx1GKXJuV98MAtU2NeME9Js3RFkFM+gUIp2NQ7WAP4OJG/Z3RB8CA/1?=
 =?us-ascii?Q?zhQL68MO0O2JOKAA9x4rIlIjaKztlbralHsVHE6ogRIiid5m2zM49LCBnAoE?=
 =?us-ascii?Q?NTDC7dV/q7YE23SNJn3hnX5A0de6R8/rnnuMu6Sx5lV8wX5dGO8DBmNmDXRR?=
 =?us-ascii?Q?+g/V5Lo3Fs3JqM2ZPPiJrz5wgDm4bO+B5l0KaO9itB702pwGNqBJHbQvFwRJ?=
 =?us-ascii?Q?NfTZYSfXrZMQ5Q28REEO3nSdMLgHCV50sPDY8sSeWlSWdBwJP1Ny3VTdoJSv?=
 =?us-ascii?Q?d8Q6kopSBT4NUCM7Mms2DwIWoW7ivAG7GkoQJJldXR+GzTmaC4OLlLNdA5Uw?=
 =?us-ascii?Q?IcfoJC2I18Yg/tVRovP4h/aIi7gQTTT26VhPc+xvuFLoLbXYjX/Gw7FvfyJq?=
 =?us-ascii?Q?IXpgXjNYjiGA/1S9FnFs+LJx8axySrVQPvDGNkl3uvmKYlUABjQSnxWwSRUC?=
 =?us-ascii?Q?v8bRDp8MGKAJY5VU1utWC+/X36oMchLZeqFcZ8qJ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0168b969-3f58-4a0e-877d-08dc65a2c865
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6020.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 03:41:40.2116
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: ECuFY29qgDMNZzRuBNAxn33k2FuS33DR13tzhztm4/mGTctEJmaajtC5IP3g6n3mc+8sn3WiyZBmV5QxJUunzw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6298
X-OriginatorOrg: intel.com

On 2024-04-25 at 09:47:52 -0700, Sean Christopherson wrote:
> On Thu, Apr 25, 2024, Chen Yu wrote:
> > Hi Sean,
> > 
> > On 2024-03-12 at 12:39:11 -0700, Sean Christopherson wrote:
> > > Use preempt_model_preemptible() to detect a preemptible kernel when
> > > deciding whether or not to reschedule in order to drop a contended
> > > spinlock or rwlock.  Because PREEMPT_DYNAMIC selects PREEMPTION, kernels
> > 
> > It took me a while to wonder why PREEMPT_DYNAMIC selects PREEMPTION
> > in Kconfig, then I assume that you mean the static config is CONFIG_PREEMPTION,
> > but the live preemption model is "none" or "voluntary", which makes the
> > static check of CONFIG_PREEMPTION in spin_needbreak() and rwlock_needbreak()
> > invalid?
> 
> Yep, exactly.
> 
> > > diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
> > > index 3fcd20de6ca8..63dd8cf3c3c2 100644
> > > --- a/include/linux/spinlock.h
> > > +++ b/include/linux/spinlock.h
> > > @@ -462,11 +462,10 @@ static __always_inline int spin_is_contended(spinlock_t *lock)
> > >   */
> > >  static inline int spin_needbreak(spinlock_t *lock)
> > >  {
> > > -#ifdef CONFIG_PREEMPTION
> > > +	if (!preempt_model_preemptible())
> > 
> > The old version checks against static CONFIG_PREEMPTION, now we check
> > the live CONFIG_PREEMPTION and static CONFIG_PREEMPT_RT, just wonder
> > if the rt check is needed here?
> 
> It's required, as CONFIG_PREEMPT_RT=y doesn't imply CONFIG_PREEMPT, and
> CONFIG_PREEMPT_RT=y is mutually exclusive with CONFIG_PREEMPT_DYNAMIC.  I.e. a
> CONFIG_PREEMPT_RT=y kernel will look yield:
> 
>   CONFIG_PREEMPT_RT=y
>   CONFIG_PREEMPT_DYNAMIC=n
>   CONFIG_PREEMPT=n
> 
> which in turn generates:
> 
>   static inline bool preempt_model_full(void)
>   {
> 	return IS_ENABLED(CONFIG_PREEMPT);
>   }
> 
> and so just checking preempt_model_full() would incorrectly return false for
> CONFIG_PREEMPT_RT=y.

You are right,  I missunderstood the definition of preempt_model_full(). For my
understanding of this patch:

Reviewed-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Chenyu