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:   "Xu, Yanfei" <yanfei.xu@intel.com>
To:     Peter Zijlstra <peterz@infradead.org>
CC:     "mingo@redhat.com" <mingo@redhat.com>,
        "acme@kernel.org" <acme@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "alexander.shishkin@linux.intel.com" 
        <alexander.shishkin@linux.intel.com>,
        "jolsa@kernel.org" <jolsa@kernel.org>,
        "namhyung@kernel.org" <namhyung@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-perf-users@vger.kernel.org" <linux-perf-users@vger.kernel.org>
Subject: RE: [PATCH] perf/core: remove a meaningless pair of
 rcu_read_{,un}lock()
Thread-Topic: [PATCH] perf/core: remove a meaningless pair of
 rcu_read_{,un}lock()
Thread-Index: AQHYexxJIp65vUVEWkqJ1eHLcUUC761FpFMw
Date:   Wed, 8 Jun 2022 15:46:49 +0000
Message-ID: <DM6PR11MB4138B6E8265A903DA8F68E4EF0A49@DM6PR11MB4138.namprd11.prod.outlook.com>
References: <20220608090938.2178624-1-yanfei.xu@intel.com>
 <YqBvMWwc1T4ZdKCg@hirez.programming.kicks-ass.net>
In-Reply-To: <YqBvMWwc1T4ZdKCg@hirez.programming.kicks-ass.net>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?aUSUZp337FLgq70jZr2uR5mCx4SYXa1xZJUxOJkXCg1y9uTUEhJHDu7XFMvk?=
 =?us-ascii?Q?47SCJqJ06mK9xcVPeZd23loC46yrPCuCW+KXs1I2LMIeMTRPDUpc3tJGLYn4?=
 =?us-ascii?Q?aOfywzdcUZ5To41zAmU/JIS1+mx1jLyGmcMSuJw5S7h3TMT26qOLE+Lezqte?=
 =?us-ascii?Q?tTSh8Qy1gxzpg5OsGJa7Rb4aXTmB34y2J+Mxtk55YiKgp3eHBZDFCqoy8Z5m?=
 =?us-ascii?Q?Sxl0SFMYeddE4Lvta0YUxHZL/gXLjnIkyOHamryQk/g+nICOZJboskObPX2h?=
 =?us-ascii?Q?4SLX5tl8gjRrO1loRRULDXwuGB1iXYlk5fd5gct4g6sp0aVH+UgVYb8cotrW?=
 =?us-ascii?Q?D785qkMphXqFtohl4oRgdQ07JZFOcCCwqjVDavGiwNymNvfF68763EeksMlS?=
 =?us-ascii?Q?Mc68Ds0rrbGydvm6cOXn0i6qW5CjThIkQWYMbnCKfmnohyDeK5AmV2M4y/wh?=
 =?us-ascii?Q?f2SBmGmxvK2pVt5nkpSTdm35sZgayyOdVXvM33eVRTxksUcMIjyia65dfEIz?=
 =?us-ascii?Q?0KR5s//yBQuR/uYaRppFKZKOiRalmlksHEwEL0F0/RR1GyHyBsjZJUf81KCq?=
 =?us-ascii?Q?DB7rXoklCKzv64vVNubMphfaB8+pTXHQOQE7Ei0N/2HSOS3iz62pVCh7mLLE?=
 =?us-ascii?Q?/MN7ck7TI52ec9tz0Oo5WBD0RunYk+SioHN5+V9J7o3/z95keCYBR/L1aRrv?=
 =?us-ascii?Q?8BvB5oG0q5lhgQZWz1hBL4fwze+y1yEcdBvdMQMY2ERHrodZBw3A/5EbQ2yx?=
 =?us-ascii?Q?92Q2hB0jc32rk6989u2H5zRZvfZNZ/noT2UfR1i69G2Cum0RAbhTF/qZJqSV?=
 =?us-ascii?Q?DF0Zdd75oUDBmeEqSPt0416LgcAuu8Gw0HUluzT1OtYEKDeesM0pBbqnrLAW?=
 =?us-ascii?Q?0MxMiphNWEcYqOgNrrn+S7RP9f0vNqLQS1wHoZevD1ew67FPskk1s67m09wX?=
 =?us-ascii?Q?u+IlNxy7OtSfNeq9CLBoV9b1rhZSWlP8x9a9fojWPTGfHcUgLKcSWi6W6wlC?=
 =?us-ascii?Q?2+jglrIQkCAPXYr7E1f6IgoyjUMfYo7Fi7aUnQMbu1ykMDBalzL55mNW03TM?=
 =?us-ascii?Q?MaeIpo2Q73ckU8aXX3HiQvFm6L9XcJHQTL/heY9dupFW8CNYWbLH14W5Iue8?=
 =?us-ascii?Q?jTZfkYyW4n9zQ+GDIM0hhHy9afHwUK3vo+SyoqtZUyeqdrelYaRS6Qr10ypO?=
 =?us-ascii?Q?UfS2NzEhFgfkr6VMQ20BDoT7BWmXe+5bRnk0h7SuTg2PqmZi6hmIsEOQ45Gp?=
 =?us-ascii?Q?lPj0xSFLsRqmKtlUYbTzCdW/eKdIOxHN9PEDaqStF7tpKAB+3jgmiK+lH923?=
 =?us-ascii?Q?uoYAVYwAu7M5tNDMcNsPfNgUP6zOUB1E0eoTwGJO3PqzIYiBymqp28Hm/V1R?=
 =?us-ascii?Q?INQ1s9eTIqUa8bEp9IhOpbF+cHbVTjUWe4q/1VrUdYPA3wKrA8xlJZVl4gH6?=
 =?us-ascii?Q?R6b17NeY3eDKzL7bEnnRssLjmakODfh7KTuc4hn2IQ8FW3QK5+ZV8MTPO6r9?=
 =?us-ascii?Q?fPpF6rvUO79+k23BmMb6EfIHjdSN4hNeUMcwRqfgLAqnkuA7wD6eKP/ho1Kz?=
 =?us-ascii?Q?CVuT0xifIqcfP1ly5y9852hOewt0C7V28cgVaY3/UNd3NKwSQRtPcP22LQhL?=
 =?us-ascii?Q?nTxQgeW5hgJ1uhHUo3UH2GXbmkgXboGl2EAmmypOa+s3cMZzl2nVx5HsNtl7?=
 =?us-ascii?Q?y3HWPdviVfzwJHot/mBLZg9MMCU2FS8nmU5a5PL1XloCN+OX38p+eIvw7y0j?=
 =?us-ascii?Q?A+j7Em2GeA=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: DM6PR11MB4138.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 68eb997c-f97d-4743-4e81-08da49661996
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Jun 2022 15:46:49.0494
 (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: h0FllOks/1i5lR9CmqdL0QDZJ6Lsb9coPBRkP6lwmChobkkLRZmKlSDF26bC3zM3KsRvI6d55hITdHQIh20kjA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5375
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-3.1 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=unavailable 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


> -----Original Message-----
> From: Peter Zijlstra <peterz@infradead.org>
> Sent: Wednesday, June 8, 2022 5:43 PM
> To: Xu, Yanfei <yanfei.xu@intel.com>
> Cc: mingo@redhat.com; acme@kernel.org; mark.rutland@arm.com;
> alexander.shishkin@linux.intel.com; jolsa@kernel.org; namhyung@kernel.org=
;
> linux-kernel@vger.kernel.org; linux-perf-users@vger.kernel.org
> Subject: Re: [PATCH] perf/core: remove a meaningless pair of
> rcu_read_{,un}lock()
>=20
> On Wed, Jun 08, 2022 at 05:09:38PM +0800, Yanfei Xu wrote:
> > Per the codes, this pair of rcu_read_{,un}lock() protects nothing.
> > Let's remove it.
> >
> > Further, the variable "pmu" is safe as it is in the SRCU read-side
> > critical scope of "pmus_srcu" and perf_pmu_unregister() delete it
> > after calling synchronize_srcu.
> >
> > Signed-off-by: Yanfei Xu <yanfei.xu@intel.com>
> > ---
> >  kernel/events/core.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c index
> > 950b25c3f210..36b0df6feab4 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -11296,9 +11296,7 @@ static struct pmu *perf_init_event(struct
> perf_event *event)
> >  	}
> >
> >  again:
> > -	rcu_read_lock();
> >  	pmu =3D idr_find(&pmu_idr, type);
> > -	rcu_read_unlock();
>=20
> You're mistaken, this is required for the radix tree internal nodes.
> Without it we can't safely traverse the radix tree without full serializa=
tion
> against the modifiers.
Thanks your reminder! And after digging and learning these commits: f9c46d6=
ea5ce ("idr: make idr_find rcu-safe ") and 452a68d0ef34("KVM: hyperv: idr_f=
ind needs RCU protection "). Now I understand it.

Thanks,
Yanfei