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:   Song Liu <songliubraving@fb.com>
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Song Liu <song@kernel.org>, bpf <bpf@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        "live-patching@vger.kernel.org" <live-patching@vger.kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        Kernel Team <Kernel-team@fb.com>,
        "jolsa@kernel.org" <jolsa@kernel.org>
Subject: Re: [PATCH v4 bpf-next 2/4] ftrace: allow IPMODIFY and DIRECT ops on
 the same function
Thread-Topic: [PATCH v4 bpf-next 2/4] ftrace: allow IPMODIFY and DIRECT ops on
 the same function
Thread-Index: AQHYmmrwWVTrDYAzL0G5ckglE1ecDq2GBfkAgABSmIA=
Date:   Tue, 19 Jul 2022 23:24:35 +0000
Message-ID: <D9D41674-8EFF-4A30-97C5-F2C1B31C1F22@fb.com>
References: <20220718055449.3960512-1-song@kernel.org>
 <20220718055449.3960512-3-song@kernel.org>
 <20220719142856.7d87ea6d@gandalf.local.home>
In-Reply-To: <20220719142856.7d87ea6d@gandalf.local.home>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?iOXl2iDF1A7pkC1gKIyjJRC9eduOxH83QbCiavJLVAbO+mYI/74II0GxhyCK?=
 =?us-ascii?Q?vE9+2Jo18ppBE1G9w0RoSrgjh3aK9kpt8Zqa11V2XtxHMK89tKGHvxEMwifw?=
 =?us-ascii?Q?sebB6T7xu0kx7OqfeUxq8Ns7T21U0TRJx4FRX18R8aVY3629p4YaUpHgi0Zp?=
 =?us-ascii?Q?/Xw55Q73kDHKsIZSbnA53dLIEFJGWI+BxGY233I6bbojQ+UyDEWASLzxqF7f?=
 =?us-ascii?Q?MynlAEqMAf/O0GUrdalVrZp7ZoENNcZWTiJ4mJRXhIZHFZ6mFdErLwvpYwuW?=
 =?us-ascii?Q?qmGgXeQ2wjSb2I1O9mZdcLzRZEoR2zU8RdrfCZu6qXD3GIQOAFcTND4M1RfP?=
 =?us-ascii?Q?oyUObkM0HQO3vjpXl+bK9sRVJTHos7P6YDgeSXwKHlCBiapzDedSBf8egcy3?=
 =?us-ascii?Q?xGQ0Qo5fHAXagswKCTUuR7RfDdSxMakFrgKMFGBEqTEKElFLDe1Q0T1bmJ+f?=
 =?us-ascii?Q?wWelTTMOxZYHwB/UjA3oTAd9NtXyptna3MjL4U9kd/bE93kOGEtW1JvXohlA?=
 =?us-ascii?Q?43iRrdlisPQqRiYV54p7hqlw+5JBUMTfg/zBTb3HPj6+GodS5dfQDXBjwSsD?=
 =?us-ascii?Q?KZLL6cUk29n3AyAI2qD3fT3FRXfdSgxc6dTTSNVU4z4kgM176l2I3cqtacg6?=
 =?us-ascii?Q?NZcEx124peLg6lrr8+NksrcMNeXD5mkK1FThSDtJLWiG+9PeSb9D56NXuQbk?=
 =?us-ascii?Q?aTALRCmfOaVMrq3iihdSn20B0ZOL19pV4Ys4n66lOYozdqO0v+la9Bcg2zio?=
 =?us-ascii?Q?T7XB042fN9hx4eu7UrDY9lsxOpA+v31RYIv09GX1HthsQoIjML3DTyu59HVg?=
 =?us-ascii?Q?phZHOCHTN7BcZ1qTeiTPXTfeGkIwhFrBrTSfIlJDIAq7sZet80iQGDFSJIv4?=
 =?us-ascii?Q?//ghSTqZuo9dVlXUAA0l6KAnIvEY/AD8gXsbz//kGuZM2Z7TyFvANozZHtsr?=
 =?us-ascii?Q?/cKNREeMhVdoUiOtPbwJ5mIhV2vxnpYfnsfiW3jA02wAOy0dVbsdXUCO04qv?=
 =?us-ascii?Q?cC7WAC/5hKq1Njb4qUyGMKZMQuR/J/KouLjCD5mnzn0pJAmXzUTWEx11qJoh?=
 =?us-ascii?Q?3dlJuJ95zNoe3q2cZXiAdI1EZQHTiMBMI6fKhnaJGGDGOtevfRLN8264Twlm?=
 =?us-ascii?Q?QDSyKp/iLDRgrsN0XoHrJw8dCLxpbdJF1TCxGl5ugtIcjYzo7vhxgeXzCE6x?=
 =?us-ascii?Q?Pz92qC/e8z9GANYX6FjapZWm4OUmIlFky1b6TNr4pGC+2gha/USK/Q4HOndn?=
 =?us-ascii?Q?Ffmj0O/9bnrfDZlri4SnCIIn5t+qaAKCzha6O+AKXzeNX4mTuJcCyreL1OOt?=
 =?us-ascii?Q?KJxCHkHLoWIi6SkRCXBx2F1OCgRwqIvCZX5+Q5NEqQxsv98i2h908CbUbiZR?=
 =?us-ascii?Q?aX1KJA7m3/7XEY3a73wydRwBFNKoWY6orOddO4FzDOijkYTlm+vz0XNvodU2?=
 =?us-ascii?Q?Qu8XTPBy/f2q3vkP3/W+0iSfG5NZtxNeaj3TD0iJsEKnH4fjiBcpsDM+9SYM?=
 =?us-ascii?Q?5yQeEdwQ92flKob6vNE1BB4uRtXLLbvQRhcgVe2L3stElyoc4jfMFfz0FN8i?=
 =?us-ascii?Q?k5uHqQ1vN4Ml6/9PDnbT7+FIAd1G0eRd3jKZHwyo3QBSZNO4b/Hnd61xtl5w?=
 =?us-ascii?Q?2TpI7PvPY0wcUOcurwh10fc=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7280954CE2F57749A1D94B03AC55CAFE@namprd15.prod.outlook.com>
MIME-Version: 1.0
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5109.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e9e076f2-2482-4f14-c412-08da69ddd794
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Jul 2022 23:24:35.1574
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: q3WJTWKRktA8Dbc565gI7ic2xl225Ae69jLsKpwTxYVbZ/5f+91QiUO7omXWZ0beHepzwDmrtccHDgfN53X5Nw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR15MB3520
X-Proofpoint-ORIG-GUID: MfuIDrrPAajzbWpaycKHwSBzag4Pkr_h
X-Proofpoint-GUID: MfuIDrrPAajzbWpaycKHwSBzag4Pkr_h
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-19_10,2022-07-19_01,2022-06-22_01
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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 Jul 19, 2022, at 11:28 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> /**
>> * register_ftrace_function - register a function for profiling
>> * @ops:	ops structure that holds the function for profiling.
>> @@ -8016,17 +8192,29 @@ int ftrace_is_dead(void)
>> * recursive loop.
>> */
>> int register_ftrace_function(struct ftrace_ops *ops)
>> +	__releases(&direct_mutex)
>> {
>> +	bool direct_mutex_locked = false;
>> 	int ret;
>> 
>> 	ftrace_ops_init(ops);
>> 
> 
> I agree with Petr.
> 
> Just grab the direct_mutex_lock here.
> 
> 	mutex_lock(&direct_mutex);

Actually, we cannot blindly lock direct_mutex here, as 
register_ftrace_direct() already locks it before calling 
register_ftrace_function(). We still need the if (IPMODIFY)
check. 

Thanks,
Song

> 
>> +	ret = prepare_direct_functions_for_ipmodify(ops);
>> +	if (ret < 0)
>> +		return ret;
>