From:   "Zhang, Qiang1" <qiang1.zhang@intel.com>
To:     "paulmck@kernel.org" <paulmck@kernel.org>
CC:     "frederic@kernel.org" <frederic@kernel.org>,
        "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 v2] rcutorture: Convert schedule_timeout_uninterruptible()
 to mdelay() in rcu_torture_stall()
Thread-Topic: [PATCH v2] rcutorture: Convert
 schedule_timeout_uninterruptible() to mdelay() in rcu_torture_stall()
Thread-Index: AQHZWtris8fTdEhd602c/GqMXMcxwK8EHJQAgAAryuCAAAtBAIAAD3ew
Date:   Tue, 21 Mar 2023 00:35:21 +0000
Message-ID: <PH0PR11MB58807EB54BC051D39300C1DCDA819@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20230320032422.4010801-1-qiang1.zhang@intel.com>
 <7a414721-25fa-485c-91a5-13d3149073fe@paulmck-laptop>
 <PH0PR11MB58806032CE88C9E6BE61059CDA809@PH0PR11MB5880.namprd11.prod.outlook.com>
 <5a887ca6-10e9-4026-b792-164deb80d0a8@paulmck-laptop>
In-Reply-To: <5a887ca6-10e9-4026-b792-164deb80d0a8@paulmck-laptop>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?kMdugRM3K/p/Yd0hCf6Ok6JQ4rvbwE1a07nmnMheDymaaQt3RjfGER9SXaoN?=
 =?us-ascii?Q?pi+TGpFtptloPTYJsHl8d0gaOzAkXpdvjY0EA4aIaciR6LHqp/tmm13GvLNt?=
 =?us-ascii?Q?uq0HPQc/N1X0/eEBTh8BHeBiTcZIHMLUD74u1/JN0AB67noAxN6EJqFFeCVs?=
 =?us-ascii?Q?jmrSmjf6zbcwmUzQ0nGp2Dy+DlPXmIm6PCrvbcU2fYxVogpI4wDh4IJRaeHZ?=
 =?us-ascii?Q?rfTEnkDSeB/+QaYG1fozmV9SzQmtMaTHP9QS9r5MhoH+Glxa0TTxq2YykfQw?=
 =?us-ascii?Q?U0MWskZBLuoT1YHrOeWDHtT9D2xTE6FY6DlzvQpUmlZ164kcmUYmjSSOeAv/?=
 =?us-ascii?Q?eeVhKt8tV00KfN+TEO8HhxzvM1tzlazPbSpUsq4zkbaoaioldH/NBHVKbnKk?=
 =?us-ascii?Q?y90lnJx7eyIuSLyFi+ZBPZlg2ablFdBN3+zGnrgZ5MRzZuQ2u/w+8iF3xytx?=
 =?us-ascii?Q?Xi/VLi08V5osU4P1qKBEjN+ciRpW6jJGufxDRCkj+NZTOL59jxgiH7Ld6OjB?=
 =?us-ascii?Q?QI0YZz3FzbdJLZfEAwx9Ev83BPdxgHOjMpA0Mp9Sb+QouFe/Frx8dy+EnIJK?=
 =?us-ascii?Q?Y0U5uCnQayGdYB/2Uk1H9C3k8DxWCj/Nms9hO8XUzh/j+nR3DgWT5BKwWYPc?=
 =?us-ascii?Q?robO+73i2OlsNVt2CB6qMsv9ORDj65Lk3JNhx8a94QtUrNhelaaLlDSscYez?=
 =?us-ascii?Q?JjNyPKk1n9ixuwXxdZtah1hvECjFYrqZ4YgOv7j9ZBTrH1A7C+lC7sw6jyCx?=
 =?us-ascii?Q?+1mch9/bProEEGUAHAxDcneoMroAF7DTiegU4HpHducrBsHGtubGsIOS7LbK?=
 =?us-ascii?Q?WVfTWGI/niveKGECPyItipiBZscEpeyAYAFClvnzPi4VINOOWKN7KYIi6zyA?=
 =?us-ascii?Q?iXUQHhQQ+vND3LZ5CbDsIyJ8D6enA6z3iAaf96+xMFwjMux6yNZtLipksS05?=
 =?us-ascii?Q?BaSUwrNb6dRqwvKrerqRJCZNY9Z4fttfk45VLp/th1CRfd1gU07oor4A5zQy?=
 =?us-ascii?Q?OGiLDbL8lr1bgsTnWrUItwen3bnHxl3eSSf/fJGyiUjS2bYgzYfqItGUh/gl?=
 =?us-ascii?Q?U6G88tBq7DHsJE6ajCmiBUmjg37vqCIjA/Rd+nmlS/cNzV6oLfjunOeDhvxh?=
 =?us-ascii?Q?7GfgtRhp9Mv4g7EQiOJ+EvliXDtpJNKv6GkkU+SFMiGJf1sEBf82x4JxA1j7?=
 =?us-ascii?Q?0Tz8k/ZU+BWmfag5bCUdbom3i5ZeshoHpO9pupDzjzK1RHWWC2X50HHwx6pV?=
 =?us-ascii?Q?sDpXxSXs14cw2GcY7zFLIkOk1axSkR5vgLrwdlqErpgeOIhtqZsv1Vz84cVr?=
 =?us-ascii?Q?9kSirZkNkoTwCFdlNaY2TsDHfCkqlEp7bwchrBdZsm3PGDGcRNlFRbosSvUy?=
 =?us-ascii?Q?rw0ZewaYUUfYyzXZL63ZOIlahwbdu93jzWuecpYvIj8mu5WQ2Z8QxRHhxOBS?=
 =?us-ascii?Q?vl59LbYCkCuI377Wd8BQ4i2hmzWaWKCV6E3psV6+B1/5KEUPGv2A7Cpkcw/9?=
 =?us-ascii?Q?DeoE1c6ALSzrRLQDr5JRs1yM2TV/CICnU+H+bGSIdGLwcGv4NTH7OjSHLBKP?=
 =?us-ascii?Q?n2fbAIoRpuXWTVgbtUQcoCPVji3brb3y0Ovus1Dh?=
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: e42d678f-16ff-49ef-c7ac-08db29a42790
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Mar 2023 00:35:21.7763
 (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: EDeV0FZASORLCauD/wAvtkXOXK7NiMSAxh5fI72CGcoxmAEzwzhYAg7yAb18qbZQWfyIEnClZFJs7W25YchtXA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN6PR11MB8244
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> > For kernels built with enable PREEMPT_NONE and CONFIG_DEBUG_ATOMIC_SLEE=
P,
> > running the RCU stall tests.
> >=20
> > runqemu kvm slirp nographic qemuparams=3D"-m 1024 -smp 4"
> > bootparams=3D"nokaslr console=3DttyS0 rcutorture.stall_cpu=3D30
> > rcutorture.stall_no_softlockup=3D1 rcutorture.stall_cpu_irqsoff=3D1
> > rcutorture.stall_cpu_block=3D1" -d
> >=20
> > [   10.841071] rcu-torture: rcu_torture_stall begin CPU stall
> > [   10.841073] rcu_torture_stall start on CPU 3.
> > [   10.841077] BUG: scheduling while atomic: rcu_torture_sta/66/0x00000=
00
> > ....
> > [   10.841108] Call Trace:
> > [   10.841110]  <TASK>
> > [   10.841112]  dump_stack_lvl+0x64/0xb0
> > [   10.841118]  dump_stack+0x10/0x20
> > [   10.841121]  __schedule_bug+0x8b/0xb0
> > [   10.841126]  __schedule+0x2172/0x2940
> > [   10.841157]  schedule+0x9b/0x150
> > [   10.841160]  schedule_timeout+0x2e8/0x4f0
> > [   10.841192]  schedule_timeout_uninterruptible+0x47/0x50
> > [   10.841195]  rcu_torture_stall+0x2e8/0x300
> > [   10.841199]  kthread+0x175/0x1a0
> > [   10.841206]  ret_from_fork+0x2c/0x50
> >=20
> > The above calltrace occurs in the local_irq_disable/enable() critical
> > section call schedule_timeout(), and invoke schedule_timeout() also
> > implies a quiescent state, of course it also fails to trigger RCU stall=
,
> > this commit therefore use mdelay() instead of schedule_timeout() to
> > trigger RCU stall.
> >=20
> > Suggested-by: Joel Fernandes <joel@joelfernandes.org>
> > Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> > ---
> >  kernel/rcu/rcutorture.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >=20
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index d06c2da04c34..a08a72bef5f1 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -2472,7 +2472,7 @@ static int rcu_torture_stall(void *args)
> >
> >Right here there is:
> >
> >			if (stall_cpu_block) {
> >
> >In other words, the rcutorture.stall_cpu_block module parameter says to
> >block, even if it is a bad thing to do.  The point of this is to verify
> >the error messages that are supposed to be printed on the console when
> >this happens.
> >
> >  #ifdef CONFIG_PREEMPTION
> >  				preempt_schedule();
> >  #else
> > -				schedule_timeout_uninterruptible(HZ);
> > +				mdelay(jiffies_to_msecs(HZ));
> >
> >So this really needs to stay schedule_timeout_uninterruptible(HZ).
>=20
> But invoke schedule_timeout_uninterruptible(HZ) implies a quiescent state=
,=20
> this will not cause an RCU stall to occur, and still in the RCU read crit=
ical section(PREEMPT_COUNT=3Dy).
>=20
> It didn't happen RCU stall when I tested with the following parameters fo=
r=20
> rcutorture.stall_cpu=3D30
> rcutorture.stall_no_softlockup=3D1
> rcutorture.stall_cpu_irqsoff=3D1
> rcutorture.stall_cpu_block=3D1
>
>Understood.  If you want that RCU CPU stall in a CONFIG_PREEMPTION=3Dn
>kernel, you should not use rcutorture.stall_cpu_block=3D1.
>
>In a CONFIG_PREEMPTION=3Dy kernel, rcutorture.stall_cpu_block=3D1 forces
>the grace period to be stalled on a task rather than a CPU, exercising
>a different part of the RCU CPU stall warning code.
>
>In a CONFIG_PREEMPTION=3Dn kernel, using rcutorture.stall_cpu_block=3D1
>forces the CPU to go through a quiescent state, as you say.  It can
>also cause lockdep and scheduling-while-atomic complaints, depending on
>exactly what type of RCU reader is in effect.
>
>So these are test-the-diagnostics parameters.  The mdelay() instead
>makes rcutorture.stall_cpu_block=3D1 do the same thing as does
>rcutorture.stall_cpu_block=3D0 for CONFIG_PREEMPTION=3Dn kernels, right?

Yes,   maybe  we can increase the description of the stall_cpu_block in ker=
nel-parameters.txt.


>
>							Thanx, Paul
>
> Thanks
> Zqiang
>=20
> >
> >So should there be a change to kernel-parameters.txt to make it
> >more clear that this is intended behavior?

Agree

Thanks
Zqiang

> >
> >						Thanx, Paul
> >
> >  #endif
> >  			} else if (stall_no_softlockup) {
> >  				touch_softlockup_watchdog();
> > --=20
> > 2.25.1
> >=20