Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "paulmck@kernel.org" <paulmck@kernel.org>
CC:     "frederic@kernel.org" <frederic@kernel.org>,
        "quic_neeraju@quicinc.com" <quic_neeraju@quicinc.com>,
        "joel@joelfernandes.org" <joel@joelfernandes.org>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 2/2] rcu-tasks: Make synchronize_rcu_tasks_generic()
 no-ops on early booting
Thread-Topic: [PATCH 2/2] rcu-tasks: Make synchronize_rcu_tasks_generic()
 no-ops on early booting
Thread-Index: AQHYlckpO9C5eZ5WlUCBwcIpqAdj3q17AHOAgAIXAkA=
Date:   Thu, 14 Jul 2022 01:53:20 +0000
Message-ID: <PH0PR11MB588067B6DC5195CBE1DDA7BFDA889@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20220712082606.3662616-1-qiang1.zhang@intel.com>
 <20220712082606.3662616-3-qiang1.zhang@intel.com>
 <20220712173709.GX1790663@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20220712173709.GX1790663@paulmck-ThinkPad-P17-Gen-1>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.500.17
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ZhWMrgR9kwBUUhyeVEe5PbhwfOn50EcWpGUu7CUZx4e0xDO8b9Vrl2nSIyQU?=
 =?us-ascii?Q?6S8DPSR74vEbRlVCvSYD1D3xvCbQD7+kTbUryw9AOMI1AxkSAv3Vz2z6WGG6?=
 =?us-ascii?Q?Eb7yMJHqOuGsXgtopHD9HiS1eO6/O7L3IfLi1X2qEORGlChHebqrYio9UOyA?=
 =?us-ascii?Q?ic7O7fID1l0I5mHBmDRNdGXq+nX55WUoPWI53S/jecuKncirABg0zTBydcvx?=
 =?us-ascii?Q?Q4eHjZbYSo4yYLMZfg9Et6S8NwQ58FGmKbVP6z3SguAfJB7v59EDI5GcSRTm?=
 =?us-ascii?Q?agLqL9ZQMrrGE0smAsYaNMye886qHm3OpxKQRiQEwGVAkjVlUXgzqhgXxmSU?=
 =?us-ascii?Q?hyNBPmRrv/NtdMSXlijo6wFYmC/CW5KWdfwargLpMJ/+6u4jGqXA+3CwwkcB?=
 =?us-ascii?Q?X5ESYvXFNhSySHyNtPoCgyZtgspWfAvvcTas0hN5ld/5O86kJ5cGoAuS0y39?=
 =?us-ascii?Q?Eo0P9ThyN5KU5Qd1l+xI/FoVF3is0NKiU4HroFW42bpW5jEGall2qSztOSFo?=
 =?us-ascii?Q?jc/3qwFEPfs3GjqwIywrDUuc1GBx1439WnMZ+3w27z3WX0jcMPJiKUzHb9CI?=
 =?us-ascii?Q?q+fvEA953wR2yBpYN86kGTRFwcGHZGGYa/cBXLgRSCl3KnbvnJk5WrFvU39B?=
 =?us-ascii?Q?RcLP0VmYJC3UiRDfnE6shxE86zi3XV0RlYYeqYXC/iy3whUaEw3GBqtjf/cc?=
 =?us-ascii?Q?UCBhBUhgiwv2pEMoDtgWmmhhRt+tmI5v0bf2g7J9bDsxRbPYO7YBRZL+t4z+?=
 =?us-ascii?Q?TTXLUt2xdFFLVth4IPPOm7FXhhn3fD/QKt7OXcoRTrxwjMB9vqNUOVGLFpcJ?=
 =?us-ascii?Q?OUnmIsgrp+MS96nDcX0ImshYW8CB+w1+eE74udO6CNu6rO+VftN+NGSuW5ZC?=
 =?us-ascii?Q?kJvYTGYMPKD1i7eESz+O2vg+CXO1+lEAdGHnTWri2lPZtInY+gelF90GUgL1?=
 =?us-ascii?Q?kl3safLAXqueRVKa1Xe10aIBHJw1lzrvU+FmYKs671kN9rk5wgbFwGgDf3uw?=
 =?us-ascii?Q?E0vz0zlOLyxlWe+rhESrXqbEKbzPxvBbGu0AbCf5/ElCgFZ+U3+nFvEAZqhL?=
 =?us-ascii?Q?1LL5XWGANeMFXri7AAIGvz37rF5ckDrFJ7wGw2T6Fz2JN/u4B4zMqOIkK6cR?=
 =?us-ascii?Q?on4s9xtD48jEDQQmA3aqAt8fyPMluYzQB13yNNIr/H0iw9O2sKkQVbCvU1Om?=
 =?us-ascii?Q?CPTywpHGlrUJtu0R1SXrqceQv/a+BTycwtp0tn2nNdsWRMRecR4FKrK7MjEy?=
 =?us-ascii?Q?H40syOfSbPiFjKLJgkoFyEbHCeqknzdXAzOuYHBOjglGWvyb3PnSQKlB0YhI?=
 =?us-ascii?Q?1Cr7MlkDgHpy6z2oVL35otFWspFoozHh7lL8fm+tv41e2l4u94KkMM2cVQPZ?=
 =?us-ascii?Q?mI+2yHQ4N9Azr0Rksts/UXFO8/+lioIHFhTPaVTYRqYEql83e2vI77rTuQOZ?=
 =?us-ascii?Q?j8IO0t1MgwcF20iETtRJTjixzi50mJqPDXA+uCwFM725OBqXiSOPxZpC9lJo?=
 =?us-ascii?Q?NK7a3ioKMhmbwb4Lxl1EyvdLUHfraobCouVnpkpMjk1WYiFKZeor9cVXwSPg?=
 =?us-ascii?Q?wca1zT+H8qJKFAsLXNOHMQEW7/naZsUhSCTH20iq?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5880.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fc6b8c8b-007f-4e91-af65-08da653ba0e1
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Jul 2022 01:53:20.2811
 (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: UViuDcVWNdPx7xqWJf9Ut8dSdRmS4YN+F/AOCHAwKx8OyoL+EOD4mlztzGYlL3LD5sZ+E0laUmOfTLxnLV0raw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4876
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On Tue, Jul 12, 2022 at 04:26:06PM +0800, Zqiang wrote:
> When the rcu_scheduler_active variable is RCU_SCHEDULER_INACTIVE and not
> yet converted to RCU_SCHEDULER_INIT, there is only idle task, any legal
> call synchronize_rcu_tasks_generic() is a quiescent state. this commit
> make synchronize_rcu_tasks_generic() no-ops when the rcu_scheduler_active
> variable is RCU_SCHEDULER_INACTIVE.
>=20
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---

>It looks like this would be a good way to provide early boot access
>to synchronize_rcu_tasks(), synchronize_rcu_tasks_rude(), and
>synchronize_rcu_tasks_trace().
>
>But do we really need early boot access to these functions?  As in has
>the below WARN_ON() actually triggered?

when the rcu_scheduler_active variable is RCU_SCHEDULER_INACTIVE,
invoke synchronize_rcu_tasks_generic(), in addition to triggering a warning=
,=20
also need to make it return directly, if not, the rcu_tasks_one_gp() will b=
e
called directly, but due to the rtp structure's -> pregp_func is not initia=
lized,
A null pointer bug will appear.

But like said, I don't see the need to call synchronize_rcu_tasks_generic()=
 on=20
early booting.  maybe this change is  not necessary.

Thanks
Zqiang

>
>And if it has triggered, and in a context that means that these functions
>really are needed during early boot, how should the testing strategy
>change to test these at the relevant portions of the boot sequence?
>
>From what I know, hitting these during early boot would indicate that
>something was removing a trace during early boot, and I know of no way
>to make that happen.  Hence my skepticism.  ;-)
>
>But *if* this was really needed, this looks to be a reasonable way to
>implement it.
>
>							Thanx, Paul

>  kernel/rcu/tasks.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>=20
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 469bf2a3b505..0237e765c28e 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -560,8 +560,9 @@ static int __noreturn rcu_tasks_kthread(void *arg)
>  static void synchronize_rcu_tasks_generic(struct rcu_tasks *rtp)
>  {
>  	/* Complain if the scheduler has not started.  */
> -	WARN_ONCE(rcu_scheduler_active =3D=3D RCU_SCHEDULER_INACTIVE,
> -			 "synchronize_rcu_tasks called too soon");
> +	if (WARN_ONCE(rcu_scheduler_active =3D=3D RCU_SCHEDULER_INACTIVE,
> +			 "synchronize_rcu_tasks called too soon"))
> +		return;
> =20
>  	// If the grace-period kthread is running, use it.
>  	if (READ_ONCE(rtp->kthread_ptr)) {
> --=20
> 2.25.1
>=20