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:     "paulmck@kernel.org" <paulmck@kernel.org>,
        Frederic Weisbecker <frederic@kernel.org>
CC:     "quic_neeraju@quicinc.com" <quic_neeraju@quicinc.com>,
        "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] rcu: Fix race in set and clear TICK_DEP_BIT_RCU_EXP
 bitmask
Thread-Topic: [PATCH] rcu: Fix race in set and clear TICK_DEP_BIT_RCU_EXP
 bitmask
Thread-Index: AQHZFGUUOrkEkQdHgkyyhC/0ix1REa54x0mAgA+aWACAAAoOgIAA8kSw
Date:   Sun, 1 Jan 2023 09:41:57 +0000
Message-ID: <PH0PR11MB588001E32A306F2166D55622DAF69@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20221220112520.3596920-1-qiang1.zhang@intel.com>
 <20221221200849.GG4001@paulmck-ThinkPad-P17-Gen-1>
 <20221231182508.GB205110@lothringen>
 <20221231190107.GX4001@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20221231190107.GX4001@paulmck-ThinkPad-P17-Gen-1>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?DlAhNdlHUiC+OkwmzBnhXHCQR66BvugptHkMdtCPTUOZD9mWI4WFLGftg/YH?=
 =?us-ascii?Q?tlUw3EEbznQWquKqMT5k0/NyV3fNhUG3PPs4NgIXUzmYQdBFCrp1wio6Px6M?=
 =?us-ascii?Q?zHnTGiL2jVCLcc9NpiQrp2z3NWEQ33TJUPTBbTTGUrP9vgv4KwD7KWkzYuBT?=
 =?us-ascii?Q?o+fZkrZfgaivDqOvW8XRhGdfnsVE3tkzId/oT1afZB53wwT2jBQQ4TKkk+Ey?=
 =?us-ascii?Q?Tnl14EnNGon+KXNTegYB59yPkUNdgAS3RBZbagokz1cH/rTcrSIESIdQtyYV?=
 =?us-ascii?Q?qYweP10DuC3Ps4IkL41Pf1mXY0ycz3NKeZ0Ji0BxuzVlx1I6PmoMSiX4XOKs?=
 =?us-ascii?Q?nurisfwh13s7YQ7D3YY40d5FTdOQyDjuy5i9PFeeFIqr1QAbb3MOjGTPjhZw?=
 =?us-ascii?Q?nNHqRl/s+VKJ79swgmB+zUQpRZvfYGmiQlImk96orKk6aKVn0q5lQiDP8icz?=
 =?us-ascii?Q?uHqUqZ5qmgbdaqMT3O/TlhpJZiTnaGSCjAptYFQp3+0gRYbc5asa/rHdWHST?=
 =?us-ascii?Q?Y/BefwvdnsFReBOzK1u79w8F8aBW0YqZ4o9Zz2ucIl2fz9Dj0QpLX/OBu9i5?=
 =?us-ascii?Q?8f8K/zoQp558IUhUOamWSqoLuQiS/bnWO4P8sMHUOBA4EW9aWEWTdH/Fceqo?=
 =?us-ascii?Q?ifm3IHCvBgDI3mirxAX964/69Figu3G7q9vKNifszSbj3+tP2dm3KnA28dkK?=
 =?us-ascii?Q?do1GlVAmxXAtmJvGMpGslKHyEkvupeWljym0AsVwfPBiDUNV3/9vPzlDXqft?=
 =?us-ascii?Q?CoT++rw2PouE+aO7dtldiR7Qw+nyjQt0hW3SDBXvePtAmOSEd36VLbQgGVSE?=
 =?us-ascii?Q?H2b1UU6CKg4yQc79VjyRF9ynsC48kP4xScTxenn25W7673wxM5l/5YVW1UHW?=
 =?us-ascii?Q?qyBtcTvjFKtGZi58czf1hOsUsMaF+DhBlHlMsDCep7r3SWvnDQnkB9NFP0K7?=
 =?us-ascii?Q?z+wte9sirFk4t+Dd4A9DHt0xvR/akqqR7WaVT4NslW67xH/zNyhqB9Hb/ZEL?=
 =?us-ascii?Q?hOVaOYW8hn2l/cWkFyo9+NgP5MKOv7VvHazLJbACrGfDtU1ahPYXuIlRzoA9?=
 =?us-ascii?Q?azSbjvKNpQ+Khbuqk8aXQu32M6GmynvWbO7cLBMb2s5qKRSRwem1hltYb3S3?=
 =?us-ascii?Q?WKtutgiUWqqS+ztTUg1yW+v128C8/YYoUtUictU08fvl6BbPd+n3WY6I9qX8?=
 =?us-ascii?Q?GPx2YyiHOZ7KnZIRCB1zdGo6jdq/LXDxXfaa3FOBqEyPzxLOOYI0R15Fb6e+?=
 =?us-ascii?Q?JDsRVqclqthek39oePn1bshAPpr4k5PphojlHqJ8HO5PgjoMYZbNOsRw77Jb?=
 =?us-ascii?Q?ORY0JrNuSnI6xPILiZ7zXjhQS1u9WRpdFxWvjQmSBnmnY1zvX0J2/eIwFSnn?=
 =?us-ascii?Q?8J3/ATDwkYrWb7UI9lHIPOjUyWcvbzlao1DgD0kE8Y9Jj93tGI2lsHHHyA0P?=
 =?us-ascii?Q?OV8H/4E8fUdFsLoKzVoE3Oo3IlWo/fXB9WYOZoh/RFKGn4oT7V9ISFWm0MsZ?=
 =?us-ascii?Q?RHLw0N9pKGkuz8ifxHN7G2k4ElM0L3ZvYf3ztD6z3dKhde94lfJs1d6z8/Xy?=
 =?us-ascii?Q?GX6yVk4K6SqmCRGeIWPYIwvIWyFYDkeLAGMV5qRj?=
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: d87179ce-98b3-4d9d-c34a-08daebdc6cd4
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jan 2023 09:41:57.7093
 (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: hy1mH7xKrRvuC1MYVTfBDZre2cJhy+Ri+pGrqKqbk/u2Knt9mFSvaC6WzrOA04NahPKX/37ZcW0XWjkqnOh3sA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4717
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,
        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 Sat, Dec 31, 2022 at 07:25:08PM +0100, Frederic Weisbecker wrote:
> On Wed, Dec 21, 2022 at 12:08:49PM -0800, Paul E. McKenney wrote:
> > diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h index=20
> > 249c2967d9e6c..7cc4856da0817 100644
> > --- a/kernel/rcu/tree_exp.h
> > +++ b/kernel/rcu/tree_exp.h
> > @@ -594,6 +594,7 @@ static void synchronize_rcu_expedited_wait(void)
> >  	struct rcu_data *rdp;
> >  	struct rcu_node *rnp;
> >  	struct rcu_node *rnp_root =3D rcu_get_root();
> > +	unsigned long flags;
> > =20
> >  	trace_rcu_exp_grace_period(rcu_state.name, rcu_exp_gp_seq_endval(), T=
PS("startwait"));
> >  	jiffies_stall =3D rcu_exp_jiffies_till_stall_check();
> > @@ -602,17 +603,17 @@ static void synchronize_rcu_expedited_wait(void)
> >  		if (synchronize_rcu_expedited_wait_once(1))
> >  			return;
> >  		rcu_for_each_leaf_node(rnp) {
> > +			raw_spin_lock_irqsave_rcu_node(rnp, flags);
> >  			mask =3D READ_ONCE(rnp->expmask);
> >  			for_each_leaf_node_cpu_mask(rnp, cpu, mask) {
> >  				rdp =3D per_cpu_ptr(&rcu_data, cpu);
> >  				if (rdp->rcu_forced_tick_exp)
> >  					continue;
> >  				rdp->rcu_forced_tick_exp =3D true;
> > -				preempt_disable();
> >  				if (cpu_online(cpu))
> >  					tick_dep_set_cpu(cpu, TICK_DEP_BIT_RCU_EXP);
> > -				preempt_enable();
> >  			}
> > +			raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> >  		}
> >  		j =3D READ_ONCE(jiffies_till_first_fqs);
> >  		if (synchronize_rcu_expedited_wait_once(j + HZ))
>=20
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
>
>Thank you!
>
> BTW why are we forcing the tick on the whole node?
>
>Now that you mention it, that would be more precise.
>
> And shouldn't we set the tick dependency from rcu_exp_handler() instead?
>
>Because it never occurred to me to check whether this could be invoked fro=
m an interrupt handler?  ;-)
>
>But that does sound like it might be a better approach.
>
>Zqiang, would you be willing to look into this?


Yes,    and I have a question,  we forcing the tick dependency because the =
expedited grace period
is not end for the first time, this means that it is not to set the tick de=
pendency every time.
if we set the tick dependency in rcu_exp_handler(), does this mean that eve=
ry time the expedited
grace period starts the tick dependency will be set unconditionally ?

Thoughts ?

Thanks
Zqiang

>
>							Thanx, Paul