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>
CC:     "frederic@kernel.org" <frederic@kernel.org>,
        "joel@joelfernandes.org" <joel@joelfernandes.org>,
        "quic_neeraju@quicinc.com" <quic_neeraju@quicinc.com>,
        "rcu@vger.kernel.org" <rcu@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] rcu: Add a warnings in sync_sched_exp_online_cleanup()
Thread-Topic: [PATCH] rcu: Add a warnings in sync_sched_exp_online_cleanup()
Thread-Index: AQHYhQ20/x4TmjWtSEepVJie126Nl61cGSeggAezNQCAADNyMIAABWIAgAAApwA=
Date:   Tue, 28 Jun 2022 00:45:32 +0000
Message-ID: <PH0PR11MB5880C30FB7101CE64485EAF9DAB89@PH0PR11MB5880.namprd11.prod.outlook.com>
References: <20220621012412.2724457-1-qiang1.zhang@intel.com>
 <PH0PR11MB588005D38E08891423310EBBDAB29@PH0PR11MB5880.namprd11.prod.outlook.com>
 <20220627211658.GN1790663@paulmck-ThinkPad-P17-Gen-1>
 <PH0PR11MB5880B496CB57A804B95977A9DAB89@PH0PR11MB5880.namprd11.prod.outlook.com>
 <20220628004022.GU1790663@paulmck-ThinkPad-P17-Gen-1>
In-Reply-To: <20220628004022.GU1790663@paulmck-ThinkPad-P17-Gen-1>
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?Lk+4DPgrfJIctOlvTiSoJqsFWra+dQktlAjBWyonuiDu8NGy8Br0cCChxX3s?=
 =?us-ascii?Q?KsEo97V6hLmkkZsDYFTdr4EOwBRl1iIu8Nj6ibjh6tzz0quWK7ETPm9Fg340?=
 =?us-ascii?Q?f4UzTVP4rRJqxKI9mJ9MmKOStYqO5ZRVQtaHcanGr0gCs2sHEPnzr/nrzrJs?=
 =?us-ascii?Q?3yZIJ33lEs+X2oH3d5bCUZHdOd4WNeeGo4EfNqLP4gwDh3xiOAoVK/ITjYGp?=
 =?us-ascii?Q?uTW0QnrxjxH4XrkYMPHD0MIWBQlWzGdgNIMbVy3spu6PlRDWu+y94NuAo0IJ?=
 =?us-ascii?Q?gA4omOR1+/cLOG+62W8682rUWuO3xjcZaz5bnSTyEPXlqLZCP4lhtrfvxcpD?=
 =?us-ascii?Q?2TdlTSRv6KBFuDVQHnpDJlgmNjiA0fgdCqgfT4m0e7J5pYzeEIxFR2PU8X+6?=
 =?us-ascii?Q?FX1op/pc5gNL3Vwdhmx9oQCaNGen56GDd6xBVV43XvXtXUJLg+CSSZNTzvHG?=
 =?us-ascii?Q?VQ9nLy1Foar2ONKFB0CylLCBq5AhP+K7AnORRwwSNMQcHvsqQBk+Y3zaiVRI?=
 =?us-ascii?Q?1K49gsqT1e9s4i4PPLexiiEQqBbTjkTaN6oyM+1TQkaoUkdRxXdgWW7XK+EK?=
 =?us-ascii?Q?CxCe4uzGhAITiqLEQIloesgrbctBMzL3JnL4O7UTceN9DqRD0RyzHt2wgxnB?=
 =?us-ascii?Q?PEbQm4Y0o/6s5GJJKs+AE+5YRqOhFwWey2lN5EDl2YslG6hvmR4tAmMUCckM?=
 =?us-ascii?Q?sJzUUXyeM80BQgds84mFIyTTVgBA3DTYsUsch8v0GKTbrjOXRmUKbkHXvVxG?=
 =?us-ascii?Q?GPJHT/HvMOhNDECJmsLX5weOBCpY7T/bZ/k7oIBIyE0GADoeunBX1e1Ykma3?=
 =?us-ascii?Q?UZFdynudCwAb1kOmLxuovxuqz/GkfItZXmibt30anbH8uBs0llfyyrImTUsD?=
 =?us-ascii?Q?eXmsnTtm9zBfBSntHwZqK6npGciHkjfZ3oBnQsg4FkuQkJ1SbKAvDjAHhGqE?=
 =?us-ascii?Q?05PUHoinpU4oE1EolnC5U6FZxjEYZeZUNCVoD8Wi83K2AsLGO34xf6IS1cD5?=
 =?us-ascii?Q?M/0QcHDp39kwQumayITdR69fd43teX3aqobVyPa2m1whweBHsTFHyS8V3Ay0?=
 =?us-ascii?Q?pfCJ5JoyxqzPv8+zyX+ILdOFj7+Ry9WUtLgAys2ThVPoUjjWXBEa1zrLAec0?=
 =?us-ascii?Q?zKV+sHhEXfnY9nZnHzH27uhAKNZG/+bK9EN0to4IeQOE3lIExYsfa9KOHTws?=
 =?us-ascii?Q?XU+S3SLyuqbgSAoNGhr35L3enU3LlO/LNPpbgXenr8jB+d/v9nfbTj4Aii4O?=
 =?us-ascii?Q?Px0XkIaW/dX7Rh3J7wXt9UDwx614xraNqXezaB+inw6u4I8vEWNFkjNEFwYs?=
 =?us-ascii?Q?PlBYz6ZpdPon0gTIGrGQU7FGwCnjphkzz7xMghUNUGxPJD04WvuZllSfDvjE?=
 =?us-ascii?Q?H8Ek7RR0ExGjR+aBTFJ/CkhTfU20gYcu1tz9U31+qkDhvfTZb0SOLYuDFCLj?=
 =?us-ascii?Q?VNtZ+NIx0jXKPhfrFayZSY+z0xGeWWnc27Dy3dNZKOSscaCmWBxsw9akH5G+?=
 =?us-ascii?Q?o0VYdwLBuwLAuiWK63SbHo1w3B1RUF5GTLKYSfToSpRIZpPfD4EhOW2mkMNn?=
 =?us-ascii?Q?HuTbyzapcuVsz9SRoaP+7j5W+f94iLC/E8U24Y1K?=
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: 9337334a-000f-47c4-089c-08da589f81ab
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jun 2022 00:45:32.4392
 (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: pjeNPBYN+cfKnhj6Z5/2/IwEXx3M6ShwSJhd17LCTdnpvHm/2NZ8rzK5JgaThYYS23UWu7f2ihWwWU/q8K12CA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3915
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.8 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,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 28, 2022 at 12:31:12AM +0000, Zhang, Qiang1 wrote:
> >=20
> > Add Cc=20
> >=20
> > Currently, the sync_sched_exp_online_cleanup() is invoked in
> > cpuhp per-cpu kthreads when CPU is going online, so the CPU id
> > obtained by get_cpu() should always be equal to the CPU id of
> > the passed parameter, that is to say, the smp_call_function_single()
> > never be invoked, if be invoked, there may be problem with cpu-hotplug,
> > this commit add WARN_ON_ONCE() to remind everyone.
> >=20
> > Signed-off-by: Zqiang <qiang1.zhang@intel.com>
> > ---
> >  kernel/rcu/tree_exp.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >=20
> > diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
> > index be667583a554..ae8dcfd4486c 100644
> > --- a/kernel/rcu/tree_exp.h
> > +++ b/kernel/rcu/tree_exp.h
> > @@ -865,6 +865,8 @@ static void sync_sched_exp_online_cleanup(int cpu)
> >  		put_cpu();
> >  		return;
> >  	}
> > +
> > +	WARN_ON_ONCE(my_cpu !=3D cpu);
>=20
> >If we are going to add this sort of warning, why not instead add it
> >to rcutree_online_cpu()?
> >
> >The reason the warning has not been present is the prospect of concurren=
t
> >onlining at boot time, which might have rcutree_online_cpu() invoked
> >from CPU 0 for multiple CPUs at boot.  However, the for_each_online_cpu(=
)
> >loop has recently been removed from rcu_init().
>=20
> The rcutree_online_cpu() is always invoked in per-cpu cpuhp kthreads,=20
> and even if it is called directly through the for_each_online_cpu() loop =
in
> rcu_init(), since the rcu_init() takes precedence over smp initialization=
,
> only the CPU0 is online at this time, still only called on CPU0.
>
>And sync_sched_exp_online_cleanup() is only ever invoked from
>rcutree_online_cpu(), correct?

Yes, currently the sync_sched_exp_online_cleanup() is only invoked from syn=
c_sched_exp_online_cleanup().

>
>							Thanx, Paul

> Thanks
> Zqiang
>=20
> >
> >But I would like to hear what others think.  Would such a warning
> >significantly help debugging?
> >
> >							Thanx, Paul
> >
> >  	/* Quiescent state needed on some other CPU, send IPI. */
> >  	ret =3D smp_call_function_single(cpu, rcu_exp_handler, NULL, 0);
> >  	put_cpu();
> > --=20
> > 2.25.1
> >=20