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:     Frederic Weisbecker <frederic@kernel.org>
CC:     "paulmck@kernel.org" <paulmck@kernel.org>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list
 is empty
Thread-Topic: [PATCH v2] rcu/nocb: Avoid polling when myrdp->nocb_head_rdp
 list is empty
Thread-Index: AQHYekOLcM/pjDOLHUOfGqJWFKxX8a1IkEkAgAEfBTA=
Date:   Sat, 11 Jun 2022 05:29:18 +0000
Message-ID: <PH0PR11MB5880DBE1D62DD56DB45BDE88DAA99@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20220607075057.909070-1-qiang1.zhang@intel.com>
 <20220610120542.GA1811980@lothringen>
In-Reply-To: <20220610120542.GA1811980@lothringen>
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?zH4kiiPj7+s7rmgF5eo7ys1kltOBqJvGfcXL4BQgZkGk5wizlniny2SP9PzE?=
 =?us-ascii?Q?A6dTxA3GRQsAM27uV1qW3fQ8L4AfEO/yXqzzOSXHd5SCZhP43HljcXuPkq3N?=
 =?us-ascii?Q?Rk6LD/VYcd0XbDtVr8hWijicnxUYeO87r5TAbrJXzc8v0dTSbJC9ftpQcpsp?=
 =?us-ascii?Q?ZGmZBb/NUSoqUVyBzDe/O0kqkGhejPkonK+FOXF8fy2Hs8jm7vnxR5+fFW+x?=
 =?us-ascii?Q?a0LoQOkDV3eWBYLT2mxwYIh1pJ5uOGhIY2pMOjQ6TXNkEGYPIpQb2drHDDBl?=
 =?us-ascii?Q?iYYPFYJs7OtXzXntGVT7zsDflSsqmaVq2FFa+eNhelrCUZnMYFsSsQJDSdpE?=
 =?us-ascii?Q?p6mO5+rWFYRDzSy1m9ggjLw0nLpOCZVGMjU0y/Z+NBpt0qyya22KoXDXaQdu?=
 =?us-ascii?Q?gNQWs+Pi+OhFkLbCkYQVy/sLZ9Vf62acBXBnNH4ErUlFB3dn5gHBC86I6nKh?=
 =?us-ascii?Q?o+a9TZh8HXtMfQiVjzY/uhg/Q6b+Papi8dtnT9PgHb+HNiRokXsRbeblVV8T?=
 =?us-ascii?Q?KasxyQgOCskI/cN18OnPzjILWE3eqzwQULaW0Rh1KJ6c0cK+rMZPsOAb3393?=
 =?us-ascii?Q?vs3vqNmpzgXgTDuxS3w3x8LNMkLY3HT8mdablAhB5ND5h8YEUJ+9N/DcUMD9?=
 =?us-ascii?Q?8pVEmG1VPuRM6gnNdQdJATDJcGkyIZiBCoAgXS7/TnmQzkDytrqLeFEE0bQP?=
 =?us-ascii?Q?DaU17OnEoxoJ/+N/s+VqMpJfFwXJZik6Ja4SeIw7LqWsM9o+HyYCykncPvRW?=
 =?us-ascii?Q?/7HZTDgXFBJgrqLR+ABksuLLNnI1kXyUawAgn9FtMWNMb+UZy6veCkLu8d6z?=
 =?us-ascii?Q?BskZyNJed6J9k4fXZx1HWM0xe1oB91URebw8pvw6/ezWv79P3uzA6zzB3UzT?=
 =?us-ascii?Q?DVOXjyu96pejQbDxFKjnGJyX1DElofteJKkxvJ70swdPEw40et5iYbejH/pV?=
 =?us-ascii?Q?y0ef0EJPC/asBD/wMnbuKxpIAWbYxw/XAXp7kZ3IhX7k54u5fbwnrcJziANm?=
 =?us-ascii?Q?5OG9O5+FCYziEDSWYkbvWE88NxxyuB7lctMJmi/E68hqzyIdBhlyVUzLZP4z?=
 =?us-ascii?Q?WQqtk+COzgR8acirqcBU+tCZ+xVZxo5iVgJZsY239AbpFrNqnwkBaPTTAIeL?=
 =?us-ascii?Q?SizA3nLKMVBC+kPnbQTd+l51whuBEs7/UAPqt2PJCOQvhRFj/K+5BM9iE2Rv?=
 =?us-ascii?Q?AiHdXJwrlNwwpwORKHaE1SZmn9PKWYyqEpoIANjibN57/XHUf0dAl14WLbbD?=
 =?us-ascii?Q?99hjzJ0uU7CKkoxcNWqbY6eXRMt121EQZdcSd8YO+vDVGfcmHKMSOtzPBpYN?=
 =?us-ascii?Q?ZkCze46tfw6CJl6sfPvMAONc1H2LuKi/JjhjBsg/nTxXtEAWH+ojGV9fjTp+?=
 =?us-ascii?Q?hOMq1t0NaMoc6aE1lDm1q+RL5P0NnqjAFQmoxNhpFhblx77eBbjokla4B1hh?=
 =?us-ascii?Q?P+3ClR5Snq+BkulVjxjj/mf+SsNrrLFkyc+EroX5HJDu2Sl6PAAGigbAI5B2?=
 =?us-ascii?Q?WE1X8Kt64fUEcQSCySVhvqbBJC7C33WtbRzoCsqD3pWjMRxnYjpAvdcT8Wlp?=
 =?us-ascii?Q?8q38WmFAb2pCEVnnfl/oXH5FtR4q6kIiC9e6ufY2HxDAGMZ6ycOXEknZOct6?=
 =?us-ascii?Q?+Gv06EE39RiF81inCU5uYvsLRtfboOrdu0j4ETOPHAcydlk4DCJp41cwty4w?=
 =?us-ascii?Q?7kfVlEWobM5BxOzIpIcwfIucEzmzrOVjPrtYYeK7rOyqlndzmY1zzSwp9Ucr?=
 =?us-ascii?Q?rzUySEMsHg=3D=3D?=
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: 7134ddcd-3ffd-4406-fca3-08da4b6b551f
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Jun 2022 05:29:18.7933
 (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: 9xISWVg0AzUEPwv2rKjMNno29An5c22N0j+bSSsUCruxI5lCNG1dv29+1RzLA378typHMDDQNalE0S+MPjHCYA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB3886
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        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, Jun 07, 2022 at 03:50:57PM +0800, Zqiang wrote:
> Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog=20
> kthreads enter polling mode. however, due to only insert CPU's rdp=20
> which belong to rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp=20
> served by rcuog kthread have been de-offloaded, these cause the=20
> 'nocb_head_rdp' list served by rcuog kthread is empty, when the=20
> 'nocb_head_rdp' is empty, the rcuog kthread in polling mode not=20
> actually do anything. fix it by exiting polling mode when the=20
> 'nocb_head_rdp'list is empty, otherwise entering polling mode.
>=20
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---
>  v1->v2:
>  Move rcu_nocb_poll flags check from rdp_offload_toggle() to =20
> rcu_nocb_rdp_offload/deoffload(), avoid unnecessary setting of =20
> rdp_gp->nocb_gp_sleep flags, because when rcu_nocb_poll is set  the=20
> rdp_gp->nocb_gp_sleep is not used.
>=20
>  kernel/rcu/tree_nocb.h | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>=20
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h index=20
> fa8e4f82e60c..2a52c9abc681 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -698,10 +698,14 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
>  				   TPS("WakeBypassIsDeferred"));
>  	}
>  	if (rcu_nocb_poll) {
> -		/* Polling, so trace if first poll in the series. */
> -		if (gotcbs)
> -			trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll"));
> -		schedule_timeout_idle(1);
> +		if (list_empty(&my_rdp->nocb_head_rdp)) {
> +			rcu_wait(READ_ONCE(my_rdp->nocb_toggling_rdp));

>I suspect you have based your patch on upstream tree which doesn't seem to=
 have this one yet:
>
>   "rcu/nocb: Add/del rdp to iterate from rcuog itself"
>
>With this patch you can't wait on changes to my_rdp->nocb_toggling_rdp bec=
ause
>nocb_gp_wait() now performs the list_add/list_del itself.
>
>Please rebase your patch on top of latest rcu:dev from Paul's tree. Then a=
ll you need to do is to change the wait side, something like this (untested=
):

Hi Frederic  thanks for your advice.
This is similar to what I said in my previous email, this change is better,=
 I will test it.

Thanks
Zqiang


>
>diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h index fa8e4f8=
2e60c..f36d6be4f372 100644
>--- a/kernel/rcu/tree_nocb.h
>+++ b/kernel/rcu/tree_nocb.h
>@@ -584,6 +584,15 @@ static int nocb_gp_toggle_rdp(struct rcu_data *rdp,
> 	return ret;
> }
>=20
>+static void nocb_gp_sleep(struct rdp *my_rdp, int cpu) {
>+	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
>+	swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
>+					    !READ_ONCE(my_rdp->nocb_gp_sleep));
>+	trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep")); }
>+
>+
> /*
>  * No-CBs GP kthreads come here to wait for additional callbacks to show =
up
>  * or for grace periods to end.
>@@ -701,13 +710,19 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
> 		/* Polling, so trace if first poll in the series. */
> 		if (gotcbs)
> 			trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll"));
>-		schedule_timeout_idle(1);
>+		if (list_empty(&my_rdp->nocb_head_rdp)) {
>+			raw_spin_lock_irqsave(&my_rdp->nocb_gp_lock, flags);
>+			if (!my_rdp->nocb_toggling_rdp)
>+				WRITE_ONCE(my_rdp->nocb_gp_sleep, true);
>+			raw_spin_unlock_irqrestore(&my_rdp->nocb_gp_lock, flags);
>+			/* Wait for any offloading rdp */
>+			rdp_gp_sleep(my_rdp, cpu);
>+		} else {
>+			schedule_timeout_idle(1);
>+		}
> 	} else if (!needwait_gp) {
> 		/* Wait for callbacks to appear. */
>-		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep"));
>-		swait_event_interruptible_exclusive(my_rdp->nocb_gp_wq,
>-				!READ_ONCE(my_rdp->nocb_gp_sleep));
>-		trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("EndSleep"));
>+		rdp_gp_sleep(my_rdp, cpu);
> 	} else {
> 		rnp =3D my_rdp->mynode;
>
> 		trace_rcu_this_gp(rnp, my_rdp, wait_gp_seq, TPS("StartWait"));