Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "paulmck@kernel.org" <paulmck@kernel.org>
CC:     "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: Check the atomic variable
 trc_n_readers_need_end again when wait timeout
Thread-Topic: [PATCH] rcu-tasks: Check the atomic variable
 trc_n_readers_need_end again when wait timeout
Thread-Index: AQHYRCgbTB0vKm9Y50G6+1/r5OpdTqzYSg4AgAA0FUA=
Date:   Wed, 30 Mar 2022 22:30:11 +0000
Message-ID: <PH0PR11MB5880D6B11469266144C58032DA1F9@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20220330112014.2587148-1-qiang1.zhang@intel.com>
 <20220330190556.GL4285@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20220330190556.GL4285@paulmck-ThinkPad-P17-Gen-1>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.401.20
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?kXARLsn2X+rxkZ+BXUbzbE3dZOKd0SVXHT9puWJ2P6qBHai1BjDNYowgGZ0j?=
 =?us-ascii?Q?DIis4Q+ojJYIzB7n2Gk6H0qh8q2b26AXs8dcx6FUfCuMBUrXzVaYvklNjbD4?=
 =?us-ascii?Q?5gZPqSSeAjEXbfORPFAT123e0pikXOe4VEumhs2ZxwVG6+qQ5sOBfJmACUEz?=
 =?us-ascii?Q?X7+7tCBmI0T7xJNMEG2cYuPJY+NaOyZi7gFcVXuWoh/vCOqUzVEX80uq0U7r?=
 =?us-ascii?Q?QDwwVskXBlDM99bA34dpgl+xARvBh43y7lQnCvYIPbw4pN7IOl6QrI4LSqV8?=
 =?us-ascii?Q?YlKncCPgLfhpUvJBiqRVApLPmfEN/lc3fAgaZw44lee16l5YUQqUMyk/rhG+?=
 =?us-ascii?Q?MjMZNkurboCoIxjCZTnU5e0wfxXC3KiNtkkD5GdXRbeOFG0nyYnJru33mIaF?=
 =?us-ascii?Q?N6cBjEFsJk4qtbH3GbO1JihpabycF2fWewtkceHijzB7GXtgegw9cCm0tAyO?=
 =?us-ascii?Q?hKwKlYYK+u2HedJZgBojFfS4dvhXTsRJbYM5HuUSdBoeFLInssaVVs5tgEGe?=
 =?us-ascii?Q?SPL/jfccQ83aKhQlu1cxl6Fb/6NJDbIV9Ng1aE52ezVXWvz9ZsfZCrdWTAfd?=
 =?us-ascii?Q?8nLuOXoxeHEMJr9QKkjyp6AT3k0Uc6ULC/UXuynSZLHbKze5c1aLmyPrKP9M?=
 =?us-ascii?Q?cDCS41rC+tPpXWUO6dhcJTwiwe35q12+p0O87Pe/YIz6J6qA3ZPXAE9srFaK?=
 =?us-ascii?Q?5qPLKuTIVNe5zkjL2t7N63CmF6kph/VjFi3sh70jVLFBYjG8OVGf6tecOnCH?=
 =?us-ascii?Q?ZE9aF3whCjsgKd33z+ryISTrEFlddD2XNYbmdqeHfca+HxjSX7n6NYvlxwMq?=
 =?us-ascii?Q?BJr9zD0yJ7fUHCChl2SvGSLmjDTbAXW4bhDqXvquNxcGzJxGOjVN70bRoySB?=
 =?us-ascii?Q?WM2f3JOXLUBczqLnJKn2BKeqEc+sqBN9rYhqxIox7NNeJSQQm6oO4oqEm5xK?=
 =?us-ascii?Q?GZiF5Bk9gX7AdHNBnUkjRpL7LoAINL31B8jGJOgCrNl1o8qkvs1hYiS/maKG?=
 =?us-ascii?Q?dg2z9xvfvBuJakJGZtPVnHIhdcxbnFP3zNJm9DMVJt91J3LSfyKJ/MmiTz03?=
 =?us-ascii?Q?KFFs68QNGdAbMLrltFgRsOHumYJfRhJePmkigLEANLSHpWGIGEc+4Rb9dgmZ?=
 =?us-ascii?Q?eIFSHTixXxib4o6hX+K+eJ+7YDF0gBlApLyR8NRKRV79RLaZ0JFlDJAAlhH5?=
 =?us-ascii?Q?HH14RfbT6cW789kByWGUoVDE32oZxe9e9fWVyKqnBa7igSG65gzaPcMOklZy?=
 =?us-ascii?Q?ARL4rMaNjKzXWNC0AaBStbvmGwl1861p31FlMETRQrczlZBrzkDWTTfphWnM?=
 =?us-ascii?Q?VQAUDeZlOcEdN0F56SEh2c7dpKPCdR+hYtO8VEA+rO3WQvDZ5t84L2wl1q2W?=
 =?us-ascii?Q?UNCCO6WJtbRRUxqm2pLx6gUfIrDRTtV61VMRyWbYI0OgAUf/LTYjKeJQXM3p?=
 =?us-ascii?Q?ohZu2AAowLS91fXMRBsrYjYhQFi1VJCwfBHQwlRTX64T6B5uyo47Juuosphw?=
 =?us-ascii?Q?GM+c1NDgsO71Kbq6YZYuI2AAoYB9qb0zCvdK5s57y+NLpIby2P7b6bLqmajF?=
 =?us-ascii?Q?D5GXMbGI96MMYzeapl+VWeJzH8+krNqXnolzS5biv8waw7BSaO+r7utXUI1M?=
 =?us-ascii?Q?MBWZEVt39Y9GCZRc4kJBQMFSPbObEfeMswVIU6zvWlxClK70Fw3wjcfOGYJV?=
 =?us-ascii?Q?qONCapnzEWQ1uqKcL8kN0tkXodo3FtpUdYhiVVWwXPIq9k1200Fa6iiiGwn4?=
 =?us-ascii?Q?3alykGI2KA=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: 74125b27-5c18-45d3-6280-08da129cda50
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Mar 2022 22:30:11.2464
 (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: WkY/A3puVNRGpV4W7pJbaTvzWBvaiBibDCrc3ByTUT7HqVGDVaOfihXVvDjo/1ShVvaidXQTUVGLsovI+e6CaA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB3198
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no 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 Wed, Mar 30, 2022 at 07:20:14PM +0800, Zqiang wrote:
> When the trc_wait waitqueue timeout, the atomic variable=20
> trc_n_readers_need_end need to be checked again, perhaps the=20
> conditions have been established at this time, avoid invalid stall=20
> information output.
>
>But are you actually seeing this invalid stall information?  Either way, p=
lease seem my comments and question below.
>
>Please don't get me wrong, we do have similar checks for normal vanilla RC=
U stall warnings, for example, this statement in print_other_cpu_stall() in=
 kernel/rcu/tree_stall.h:
>
>	pr_err("INFO: Stall ended before state dump start\n");
>
>However, the approach used there did benefit from significant real-world e=
xperience with its absence.  ;-)
>
>							Thanx, Paul
>
> Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> ---
>  kernel/rcu/tasks.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h index=20
> 65d6e21a607a..b73a2b362d6b 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -1544,7 +1544,7 @@ static void rcu_tasks_trace_postgp(struct rcu_tasks=
 *rtp)
>  				trc_wait,
>  				atomic_read(&trc_n_readers_need_end) =3D=3D 0,
>  				READ_ONCE(rcu_task_stall_timeout));

>If I understand correctly, this patch is intended to handle a situation wh=
ere the wait_event_idle_exclusive_timeout() timed out, but where the value =
of trc_n_readers_need_end decreased to zero just at this point.

Yes,  this patch is intended to handle a situation.

>If so, please see my question below.  If not, please show me the exact seq=
uence of events leading up to the problem.

> -		if (ret)
> +		if (ret || !atomic_read(&trc_n_readers_need_end))
>  			break;  // Count reached zero.

>Couldn't the value of trc_n_readers_need_end decrease to zero right here, =
still resulting in invalid stall information?

The value of trc_n_readers_need_end decrease to zero right here, indicates =
that the current grace period has been completed.
Even if we go to print some task with condition 't->trc_reader_special.b.ne=
ed_qs' as true,  and these task  will not affect the current grace period. =
Is my understanding correct?

Thanks
Zqiang

>  		// Stall warning time, so make a list of the offenders.
>  		rcu_read_lock();
> --
> 2.25.1
>=20