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:     Joel Fernandes <joel@joelfernandes.org>
CC:     "paulmck@kernel.org" <paulmck@kernel.org>,
        "frederic@kernel.org" <frederic@kernel.org>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] rcu-tasks: Make rude RCU-Tasks work well with CPU hotplug
Thread-Topic: [PATCH] rcu-tasks: Make rude RCU-Tasks work well with CPU
 hotplug
Thread-Index: AQHZAOVqO+x1XL1ptUSqdc8Q343Nzq5Qe5uAgAABlKA=
Date:   Sat, 26 Nov 2022 02:43:59 +0000
Message-ID: <PH0PR11MB588009DA52C17BAE7388F99EDA119@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20221125155427.1381933-1-qiang1.zhang@intel.com>
 <Y4F5r9nLDtCrl6df@google.com>
In-Reply-To: <Y4F5r9nLDtCrl6df@google.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?07+g1mLh+/AX344xeaXxpkR9ncosF8OTVW6INtFaEjd8//AKOBTiodBvLFHQ?=
 =?us-ascii?Q?KBzMDwNk3bb/WPjPMtoG8D3qs4e+358t8oAw5Gb/8MCm/g3ZeOEiU5iDDoN9?=
 =?us-ascii?Q?HqdEOJ5bxKyhMk6Et6HjYgopZCR5xWOfj861Z0SdQvSji5ZYaCL2xEHlBIBq?=
 =?us-ascii?Q?6fehHwQbVzNqd2wuJ9jMdS850UtQhaBgetYvQPGMk2pwKXpa5Tl1bHEy9kKF?=
 =?us-ascii?Q?nqxV40bK3NmuH8pi3+k1Nh3fd16OwolOjfO4af+WFgrFU0SaEHymYfNsH30u?=
 =?us-ascii?Q?5lrF82ZTUc94HsrQrC5UR23yy45LxztxP6Tyw/FY5iiuNdQuzZKSBngpf2uw?=
 =?us-ascii?Q?FyT4eT8GBMZxkbcAcI1jJtb5c2+f4eWabnxBYoHafHjy22ziK57tCxBmUkpb?=
 =?us-ascii?Q?gKgn8deg9oZutKqq9zP6yiGvHm/NqDmIUYyMDjwphT25BozHPOuLAdEGd69b?=
 =?us-ascii?Q?rRmJ+3QuWP/jj4sCCwH0pEz7+7jsgKzz621hT2qy6UEDe4PChx80o0moB2gN?=
 =?us-ascii?Q?WwsbumJF1kgUFWHcZE2zGUK+fICIhzifiPQGA+GlM65biz3G228RYf2QnsYR?=
 =?us-ascii?Q?C0N3PENxu3on1xQv95/+QEgaE+7mtEDcA+Y2uOOXwXev80AnrId51V8jPrRJ?=
 =?us-ascii?Q?3yzkVU2L+KkT+XrBgt46T7iFNxr1DyNzjl3XGlrXv3xIKzt+6Os4nkciyECK?=
 =?us-ascii?Q?YL0bjfDQuEqGtmyghIVDgxEeOgNXUazoUOFVHwDhuARWmZhdQft2xTid+JJv?=
 =?us-ascii?Q?RuW/KabWhkyhDY1JhYIi/9B+y4diWAmh/grvRDYPL6DDc+xQjADcHaSVhrh4?=
 =?us-ascii?Q?BmT1uRychf2Aaaxv50pclcHrht1btc4J0f1LpGvE6V/XvXA9vkBp3hCznSgy?=
 =?us-ascii?Q?OS9lXl85rgVSzCVTMgBt9PcLk9Nh3lefTRNjuHbaQoPJALXq6RoY4kGDe0zw?=
 =?us-ascii?Q?T9NM+VAZczLr/wT+EPtR6gxUIpIz+xZXLTBr+AsNwtHxKya3TwuiYmz30ccb?=
 =?us-ascii?Q?vVIa/r9MI80025VKsfdK1LCglG9sWDXeowf4VFSOI6Q5zuFUMWW0x7Y9Eku1?=
 =?us-ascii?Q?5TiI8DTpdQJM7gmchAIpJ4u/rM0zF7IQpi/QTzTBsJhz2wE0WqT1KrVZyQ0h?=
 =?us-ascii?Q?lAyDiUpry4hZv9m8gRdKtlKj0yzSR03LVfu+XQrjBfYdIpWuQol+n37YMinW?=
 =?us-ascii?Q?YZ7NMzCTy0i3g5mrISh4fULVZzb0nnxufc8IQUH0OCSFPXoeCpULaxSbZVuf?=
 =?us-ascii?Q?9F2jBpZMN9bMZQHFIfFkcpviDHw/AIfm7vtrLbG2OaQB3ltnr4LIICfLYFLu?=
 =?us-ascii?Q?Boji37LPwhBOAB4RZaXqcy1iAoUbGcA2vARS+AuzhZkGFViGNjTzc3cHUA0S?=
 =?us-ascii?Q?IJQg+GAUnyQz76xbpV/mIlhVKPbviIpl4VLxsd6aflR4EKXHAibuDJa+1RSB?=
 =?us-ascii?Q?PhBi0HJXgCdJ+O0pyvdBkijEKxyr2lF6dFguwHBiHPT206kq4XTxqnfjNW4+?=
 =?us-ascii?Q?ZPQgXc+/uPRP3L8fxsnn+EIqED+eFoNJFQPTdnPa+xvWDq37f3eSN0u1ikaU?=
 =?us-ascii?Q?wh51OnbYxZ8ooq/rV1pIMjszMsgx2dRtEhuY1tsF?=
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: 0e4eec21-7610-4309-f7b7-08dacf58126a
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Nov 2022 02:43:59.9105
 (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: IGzZamBlbmlMurP4YpmjWCuGX4b7yWokSkdb/HUaNwHDr2YDiOY10cgvADvk01Og+7ehEDOqfYb+bB2ckJoQMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5214
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        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 Fri, Nov 25, 2022 at 11:54:27PM +0800, Zqiang wrote:
> Currently, for the case of num_online_cpus() <=3D 1, return directly,
> indicates the end of current grace period and then release old data.
> it's not accurate, for SMP system, when num_online_cpus() is equal
> one, maybe another cpu that in offline process(after invoke
> __cpu_disable()) is still in the rude RCU-Tasks critical section
> holding the old data, this lead to memory corruption.
>=20
> Therefore, this commit add cpus_read_lock/unlock() before executing
> num_online_cpus().


>I am not sure if this is needed. The only way what you suggest can happen =
is
>if the tasks-RCU protected data is accessed after the num_online_cpus() va=
lue is
>decremented on the CPU going offline.
>
>However, the number of online CPUs value is changed on a CPU other than th=
e
>CPU going offline.
>
>So there's no way the CPU going offline can run any code (it is already
>dead courtesy of CPUHP_AP_IDLE_DEAD). So a corruption is impossible.
>
>Or, did I miss something?

Hi joel

Suppose the system has two cpus

	CPU0                                                                     C=
PU1
					     cpu_stopper_thread
                                                                           =
       take_cpu_down
						    __cpu_disable
							dec __num_online_cpus=20
 rcu_tasks_rude_wait_gp                                      cpuhp_invoke_c=
allback
	num_online_cpus() =3D=3D 1
		return;
       =20
when __num_online_cpus =3D=3D 1, the CPU1 not completely offline.

Thanks
Zqiang

>
>thanks,
>
> - Joel


>=20
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---
>  kernel/rcu/tasks.h | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>=20
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 4a991311be9b..08e72c6462d8 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1033,14 +1033,30 @@ static void rcu_tasks_be_rude(struct work_struct =
*work)
>  {
>  }
> =20
> +static DEFINE_PER_CPU(struct work_struct, rude_work);
> +
>  // Wait for one rude RCU-tasks grace period.
>  static void rcu_tasks_rude_wait_gp(struct rcu_tasks *rtp)
>  {
> +	int cpu;
> +	struct work_struct *work;
> +
> +	cpus_read_lock();
>  	if (num_online_cpus() <=3D 1)
> -		return;	// Fastpath for only one CPU.
> +		goto end;// Fastpath for only one CPU.
> =20
>  	rtp->n_ipis +=3D cpumask_weight(cpu_online_mask);
> -	schedule_on_each_cpu(rcu_tasks_be_rude);
> +	for_each_online_cpu(cpu) {
> +		work =3D per_cpu_ptr(&rude_work, cpu);
> +		INIT_WORK(work, rcu_tasks_be_rude);
> +		schedule_work_on(cpu, work);
> +	}
> +
> +	for_each_online_cpu(cpu)
> +		flush_work(per_cpu_ptr(&rude_work, cpu));
> +
> +end:
> +	cpus_read_unlock();
>  }
> =20
>  void call_rcu_tasks_rude(struct rcu_head *rhp, rcu_callback_t func);
> --=20
> 2.25.1
>=20