Received-SPF: pass (google.com: domain of linux-kernel+bounces-196004-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
References: <20240528003521.979836-1-ankur.a.arora@oracle.com>
 <20240528003521.979836-17-ankur.a.arora@oracle.com>
 <20240529081404.GI26599@noisy.programming.kicks-ass.net>
User-agent: mu4e 1.4.10; emacs 27.2
From: Ankur Arora <ankur.a.arora@oracle.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ankur Arora <ankur.a.arora@oracle.com>, linux-kernel@vger.kernel.org,
        tglx@linutronix.de, torvalds@linux-foundation.org, paulmck@kernel.org,
        rostedt@goodmis.org, mark.rutland@arm.com, juri.lelli@redhat.com,
        joel@joelfernandes.org, raghavendra.kt@amd.com, sshegde@linux.ibm.com,
        boris.ostrovsky@oracle.com, konrad.wilk@oracle.com,
        Ingo Molnar
 <mingo@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>
Subject: Re: [PATCH v2 16/35] preempt,rcu: warn on PREEMPT_RCU=n, preempt=full
In-reply-to: <20240529081404.GI26599@noisy.programming.kicks-ass.net>
Date: Thu, 30 May 2024 16:04:41 -0700
Message-ID: <8734py6gvq.fsf@oracle.com>
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?N7zy0COAZqi/wSMXQho+7qVdTlXBWk2y/4V5WYnGYgkwgz9Z6Da71LZh5DA9?=
 =?us-ascii?Q?vH4B7BasDzZVpaCwBv6QCYyTA5cr6o2b7+VEiDIszbvwUvIczNhAbJgSVRf7?=
 =?us-ascii?Q?SqxMb1WVYxqoyHshgeFK9faoFj9kZ99gdBIHH1yCSaKmxo9TDPV+FMrTI4LI?=
 =?us-ascii?Q?Xz+nhHzsOgOF+V34ScdUGgS3o8YF5RqHX0H6rab4FCL55cozj1IZXtqyWMCr?=
 =?us-ascii?Q?l3NDzHgKOaYybzvp8CdpU12zVzywInfdQCqMjJUIL+JrFGXFgMo/x299xUBo?=
 =?us-ascii?Q?TuDkL8ZlSD7LAnSxvjiD/gAMr1vr1/7K5rhA0bSoJEiSHRR2M/qQWy0Di8BS?=
 =?us-ascii?Q?qyaekQaNMnb9YNnzUDULec9Tdy484U8E2AT14GzDpHMX5FmLdya5sWYX+QYk?=
 =?us-ascii?Q?GeQCIU9ErinH2lRynY2p0ElcL3U3Qtz3djjnAB4qQdNiRGT2s0l7S9l2v9NA?=
 =?us-ascii?Q?FlddVOKHCWyFrx6ZTT5LM8P190H+TCCmxG9eBfQ3/WnV82YmgzalSGT0FhlK?=
 =?us-ascii?Q?8J65GPGZbUZJF6ghd3+ZCZAMfZZlw2EEU8in3Fy2ppRK6/syfYLDhde9E/ou?=
 =?us-ascii?Q?netcxzrio26SPFHd+uq1bI6qL2f0pzvgYZKgqpcn22vSmxmCx4/7IfxgRY/o?=
 =?us-ascii?Q?2pkNdXLK4dJX0VDKsIQabYc+WZosMqga0K5quaQBXgmdmxA2MgYui3lHGOZx?=
 =?us-ascii?Q?xvtXKhxSd9B9TTtbZL4Fl4F3l1knJBl0Lffao04QeEQQdpwfsuQvV2pMy30H?=
 =?us-ascii?Q?qu3ehqrWZz4PKXRbOEENV7EIb9/xLsc/7TvpduMNgxS12UaMTz0C72fs5GyI?=
 =?us-ascii?Q?WO2NKuuFuHJtLLNfHPubGGzaUvXtG6BPFW/8JsREvsJNFoPnxGMgnJE+LQQn?=
 =?us-ascii?Q?b2og48U5n3/VRNMVQcVvTe/1CMt4muV5hNMg8ex05EBEkfkd516qyZyBoPSh?=
 =?us-ascii?Q?Zqn55HdNPDPwtqg+I7ahaZzqJcFczzPcbyvVL6xjsqEC8QURUKB51F0jrVLU?=
 =?us-ascii?Q?Qm8Qfk20iGSVvLxUqMP9jrUh5tMHyj7HsK3B44WZPVSr/PjeCF0ihTNUgK+7?=
 =?us-ascii?Q?DCxxBOrc+pnMaThTwPCvkonxkFiHgwfdYDDidmNoFX2tmXoaWrf97aTWlzu9?=
 =?us-ascii?Q?bFdRNrjpj42vguvj0Pj1KPubqC8iLdbah1wmmr2Nuer8qDRnz0oqA6qcE+01?=
 =?us-ascii?Q?PlzDyptKm50UTZt3H5qOuevJgALchtXLEPltLn1C4u6CoTLc99oWG9COTPHz?=
 =?us-ascii?Q?CWZEKoMKWSzHqNpCePFNLI1cGybJHzssuhCKhUfMz6mB2RX2xHBZCgnMQN6M?=
 =?us-ascii?Q?yrpgKmYHwK5OBtOCiF7jhef3+6BgvgjFa8f0P2lamFCY6YnSw0oOY0m2/fmX?=
 =?us-ascii?Q?/PirsYkm2FuZJ8Fr3Oz4IrAHFNY87w83u7caXH0DNcVo6VhoMGcQT1HRwq+i?=
 =?us-ascii?Q?GyQ+MU5wZrDUiXv/Gb6KJqnXrGxa2YtrOvtWJBv7l1l2A73dXVIMf7TjRUoA?=
 =?us-ascii?Q?v+mfns5KJ8wEpdEKlChniZ8w5w8Utl5pfX/8rwoFnUxwPjXuF2DPkx9X0bIR?=
 =?us-ascii?Q?T9D0SG4xxR2mmO8aEJEJoizW5e/tOcJMo6U1W17nvnJ30ewmDGX7qrWlygy+?=
 =?us-ascii?Q?bQ=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	dXW9Qx38nxaEXmpawcI/www3Qhpi0i0A+WQXeknksT+1Bw7LLD7Ik9IKXMxcB7RXpewzYq3dDSMS315FsRkta+KL/PCtuI440khZ07Awm5ne1tEqb3LzogE74Kla/Jd7RmAO4piN77CVr8wckvrtqFeOhYUWC14+JFES2v7wozVJDhzprqwahsho0Y7dNNLR8p7TRHKClk6maEjgc8ti2L5hUejBpQKeXKj1q3mH38sY1qNeqYVPXnTPP7AwK34TQixsagPsMeVi08HZhwTPoNNFGN4nP3FpCeNOYKOrHD+TKq9y8sAJkJSFbfCeBhil+WCTaWMQ5Lq33SbYWRj57uK0nIAeCGVOEGUEO/dqbLyIDkjoUay9NMcagCBNUPZPOi9LauNIu0/NNDuXhMm3GmRuHh0MHdZ4Su04Fw/zdJdsorrzM9fUqK+UQo8CjkYAokN68c7NSWZi2/zEYRbsH9kwXta9mbnNcFU4J2oTq70oRkFSLqc8HEWQDPS+zObYJNJvJxsF0Bfy7PCnlaye8DqdKlVp+7ANfN8Q2HegYEodtPA+Vl5H24H6lhYGaKuNnkMZICyiD8cK0fm3Et9bACfEsCKU5cl620/Cob8bRpU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1a8e1a7b-e910-4157-7cc7-08dc80fce8c2
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5418.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 23:04:51.8605
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rKms9Y3iKAm5IkoLyvBLuwbFSmz017gP7bwNBpNkS3NlojCFDlMFU9bm8cb+slxQ8Z3jiAc6PsxfRrOK9zAjdIarYdTnWeWwVw3BVfWuS1w=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4511
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.12.28.16
 definitions=2024-05-30_17,2024-05-30_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 malwarescore=0 bulkscore=0
 suspectscore=0 mlxscore=0 adultscore=0 spamscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2405300175
X-Proofpoint-GUID: lcMtnKSLmPsmGtXJD0wbr6XyEdnbzV_q
X-Proofpoint-ORIG-GUID: lcMtnKSLmPsmGtXJD0wbr6XyEdnbzV_q


Peter Zijlstra <peterz@infradead.org> writes:

> On Mon, May 27, 2024 at 05:35:02PM -0700, Ankur Arora wrote:
>> The combination of PREEMPT_RCU=n and (PREEMPT_AUTO=y, preempt=full)
>> works at cross purposes: the RCU read side critical sections disable
>> preemption, while preempt=full schedules eagerly to minimize
>> latency.
>>
>> Warn if the user is switching to full preemption with PREEMPT_RCU=n.
>>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Juri Lelli <juri.lelli@redhat.com>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Suggested-by: Paul E. McKenney <paulmck@kernel.org>
>> Link: https://lore.kernel.org/lkml/842f589e-5ea3-4c2b-9376-d718c14fabf5@paulmck-laptop/
>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
>> ---
>>  kernel/sched/core.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index d7804e29182d..df8e333f2d8b 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -8943,6 +8943,10 @@ static void __sched_dynamic_update(int mode)
>>  		break;
>>
>>  	case preempt_dynamic_full:
>> +		if (!IS_ENABLED(CONFIG_PREEMPT_RCU))
>> +			pr_warn("%s: preempt=full is not recommended with CONFIG_PREEMPT_RCU=n",
>> +				PREEMPT_MODE);
>> +
>
> Yeah, so I don't believe this is a viable strategy.
>
> Firstly, none of these RCU patches are actually about the whole LAZY
> preempt scheme, they apply equally well (arguably better) to the
> existing PREEMPT_DYNAMIC thing.

Agreed.

> Secondly, esp. with the LAZY thing, you are effectively running FULL at
> all times. It's just that some of the preemptions, typically those of
> the normal scheduling class are somewhat delayed. However RT/DL classes
> are still insta preempt.

Also, agreed.

> Meaning that if you run anything in the realtime classes you're running
> a fully preemptible kernel. As such, RCU had better be able to deal with
> it.

So, RCU can deal with (PREEMPT_RCU=y, PREEMPT_AUTO=y, preempt=none/voluntary/full).
Since that's basically what PREEMPT_DYNAMIC already works with.

The other combination, (PREEMPT_RCU=n, PREEMPT_AUTO,
preempt=none/voluntary) would generally be business as usual, except, as
you say, it is really PREEMPT_RCU=n, preempt=full in disguise.

However, as Paul says __rcu_read_lock(), for PREEMPT_RCU=n is defined as:

static inline void __rcu_read_lock(void)
{
        preempt_disable();
}

So, this combination -- though non standard -- should also work.

The reason for adding the warning was because Paul had warned in
discussions earlier (see here for instance:
https://lore.kernel.org/lkml/842f589e-5ea3-4c2b-9376-d718c14fabf5@paulmck-laptop/)

that the PREEMPT_FULL=y and PREEMPT_RCU=n is basically useless. But at
least in my understanding that's primarily a performance concern not a
correctness concern. But, Paul can probably speak to that more.

  "PREEMPT_FULL=y plus PREEMPT_RCU=n appears to be a useless
  combination.  All of the gains from PREEMPT_FULL=y are more than lost
  due to PREEMPT_RCU=n, especially when the kernel decides to do something
  like walk a long task list under RCU protection.  We should not waste
  people's time getting burned by this combination, nor should we waste
  cycles testing it."


--
ankur