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/r5OpdTqzYSg4AgAA0FUCAADoYEA==
Date:   Thu, 31 Mar 2022 02:03:26 +0000
Message-ID: <MW5PR11MB58585FA7332315F46FA36CD0DAE19@MW5PR11MB5858.namprd11.prod.outlook.com>
References: <20220330112014.2587148-1-qiang1.zhang@intel.com>
 <20220330190556.GL4285@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR11MB5880D6B11469266144C58032DA1F9@PH0PR11MB5880.namprd11.prod.outlook.com>
In-Reply-To: <PH0PR11MB5880D6B11469266144C58032DA1F9@PH0PR11MB5880.namprd11.prod.outlook.com>
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?vwhO272z2TuYOfbaqqhaZQBP6rcF7amQvf87x0kkyPg/kqvDklPwbQot9Rqi?=
 =?us-ascii?Q?y3I12Bri8fmA2TTlm3hKOsk9PcwAPs/USyFL/SjC1B5maDYny3/x15Ugc26d?=
 =?us-ascii?Q?dyh4OXbSIJi+SM/Guh7KAWMjzbAomchR4knwjKAxzPbRmKOPRUieIZSltpRb?=
 =?us-ascii?Q?BtQgm67XVqs3ipxjRH4lPIyMQ/NZUDtGPynW8+qixJRp1nvpet0nY/RM2gF3?=
 =?us-ascii?Q?eGD9g8+82jziWS8ozuYusN0hlcukT6okVeNwCaFHtTsTF7VZXfgyXwan0OTJ?=
 =?us-ascii?Q?4/2+FOwOAj5kY8/+ptcvOIwJR1AFEytthGpn1lKuXbm8MAJN/JzbhEj22EB3?=
 =?us-ascii?Q?+TGlrXigCRqLqSILHvB2G/Ah8DTpBqEVQCI/2/JKTY2xTIU5UnCjZ5kgDcMF?=
 =?us-ascii?Q?RmdHX5SL1/FJ6p1gySeT+2D4aTWD9/QeLWdGph89Cl10GJKawBmgZVzH/vxB?=
 =?us-ascii?Q?77KR5JKgtURuF/KDN4qG/fGwz/OjuUsxiAMQb9UpWo5vCioSmgETZYxhz9zU?=
 =?us-ascii?Q?EqCSrlcdkSakLu0C2/ZH8b5K7Ua/G0oR/BvuupbZgEKxTWs+UG9B2qv6VS/O?=
 =?us-ascii?Q?/ZPTfmizykkyETyQSXP0tX342aBD7Syj49k+XtIfXKb0sXJX1OaoyUxGTpP0?=
 =?us-ascii?Q?7vXP7oI2exJWad2Uv+jtd54JAmO/ul3NRm8rsHUPkhgFFMFCtB9WkorFymjH?=
 =?us-ascii?Q?QZVDHfqI8VeZ0jkUbOn/K877LE5FD4Sn1wmAnjPHqvP+kSDIqOcXV+tTzt6e?=
 =?us-ascii?Q?ns8WGxZ7cWxyhNfDTjzuXeF3TAs5bBHue8sHKIp5+mw2EA7QQ30naLNWWuGW?=
 =?us-ascii?Q?SY4zt+icAeA+QYgrnrKO9/NfwdguIWogNYEXEK/GIixQ21mA0alcyAZUt/zC?=
 =?us-ascii?Q?/8cnEMGhaPrFLvUgd2b0KEh4Tm3TwlSvSUybhyoqQkM03CIHwsQG9WPzbMqd?=
 =?us-ascii?Q?v77khs1ohJjHcFZG8SxJfJgjhTbUf72EJOo5ScXbZaRgAjq+ubJ3J5XFLgvR?=
 =?us-ascii?Q?0Ris/lCE4gG5WMBPtSkFMh/qjLi1QrKajpUn3sXG9tSmUTWwxDGyETiVfO7l?=
 =?us-ascii?Q?lVHYH94Q9cMfQXjx3q0fptXl9bc+fX7qdr2BrZvgzBi1FMZ+AdJ+N982Ijgp?=
 =?us-ascii?Q?pruUN0tCbf4Ne2vKp5lyhEuGzxmh6Wi6iv47xJuY2L/Ra9ZlMM4oQn0UEzZQ?=
 =?us-ascii?Q?UNI+OecAuWz2rPBDUOrUcmSj1qmPb9EShqo7dKtRBrok3UHDkO+zyQWaNwXy?=
 =?us-ascii?Q?P9K/miwS+L7YKCpoEi5UYExt7nw0//vNLgh1c+aN9qX7vtQZIgckUxVT5Rkr?=
 =?us-ascii?Q?ZIBHfG53nRcgdgzjSbeekWlQQLOkyebkuTmnx1PP5O1lftaH3VP/LZgKYIjQ?=
 =?us-ascii?Q?NikdsAjpJWRzeJ9XjLETZpUZz+xycnjzpGGTXXsZmTD8Owgu/nzpYydQsfvk?=
 =?us-ascii?Q?MCokAhRt6OQbz13+ykN/8/y4/GHfOWNBI1PJuB0Kvs0kBsbw7bR0MrU98Srs?=
 =?us-ascii?Q?Ws7mBBPXV2r9qNaVVOD876ftdwnpRKm6YW8UaULHscCl2k/e6URnIXaT0pnG?=
 =?us-ascii?Q?qJQeNueTnwA/LHuBhl6Sy71Njg82ep25hWWG+/WDf5Uj2EuQ8mn9mqXdFjG/?=
 =?us-ascii?Q?aeha+PimqAhfl05TL5OuoHbdPI4mkFTcJ4MTBIHzu21Kgcd9NfEH2tPWqvFL?=
 =?us-ascii?Q?Nix8r6sLBwy1hyTzSHeMgVaPb+/s0jl/XWv9Wi6D0uha6t0CJClKL344hlGu?=
 =?us-ascii?Q?W+BuYcAXDQ=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: MW5PR11MB5858.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9860910c-7314-4be1-95c8-08da12baa4f5
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Mar 2022 02:03:26.5967
 (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: j8K6gbx/rL4zKxI51xWKCQD8ybbRYTVPZPUi9i/PX/6rTsLoNdwK6rnp7HpmEdMA1vVI7GMLdsuYizXw4ZwoJw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5916
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=20
>real-world experience 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, indicate=
s that the current grace period has been completed.
>>Even if we go to print some task with condition 't->trc_reader_special.b.=
need_qs' as true,  and these task  will not affect the current grace period=
. Is my understanding correct?
>>

Is my commit  information not clear? Is this description more clearly

When the wait_event_idle_exclusive_timeout() timed out, if the value of
trc_n_readers_need_end  decrease to zero just at this point
this indicates that the current grace period is just completed at this poin=
t,
direct break  and  avoid printing stall information.


>>Thanks
>>Zqiang

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