Received: by 2002:a05:6a10:f3d0:0:0:0:0 with SMTP id a16csp2325071pxv; Sun, 11 Jul 2021 09:30:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy2/nTIzJ5XzeFxAIXE1oMWEkgKBNnwr6gPwcDX5V5CFTx9Z55HVHESkjr4BrdWHCqfyhcw X-Received: by 2002:a6b:5f0b:: with SMTP id t11mr32318452iob.111.1626021059463; Sun, 11 Jul 2021 09:30:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1626021059; cv=none; d=google.com; s=arc-20160816; b=M5eg6VPtqMQXWWYHGLXEXbvkJWJg/WfnCtOlEzZjOMM/0JlgulXkbj2cCDIy5rJD6k DscGdX9a9JIXplbVcJKQQIZ48YN9WRPPq8mSP0Jqho8IXmsJRth/f9TjbIbamOzmQoRk VooqMgvCYrOeRd2Bw0wMYMMr6B1q+v3RahAxXzTeRIQtZtR9O4L0uy6Fzei6eLfqckwF b3f19BxnrN+Fhk2MJr4IBaRqzh4Htr/bHa3OcjCRSOTBnjpl1wj0CF9rl/tcAi4tWagz fh39Yt55HWAosqDqID5rEham7vBfKRRvgaSU9Dy7TkNI769mU4yvdcAfMqvHqQY8VwAZ enMA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dkim-signature; bh=n3YRHkj0c58PDufExs5GW5EqSsqejMDzvuIBPPdMvOg=; b=yZrIfHDsr4gnhpKW6COPM3KGoxVplX9QfRcLN6U5XBVAHzgJiU8/jusSn0/ZDAd6Gu p5JfOI2zL6kX9PDRcWEHZTRrFCW/3y1QAUItBc9ee+Yn20a+xLYIbSP++HiRAtPsdk6g xDPsBiPJAQcmkTHC4W2vv8cIqf6ypdxxsoit/g1NDg36XZ6dw/GxNnneeVLrN3QL+GGQ RhrixjHIKAXeVy+UUrxoijN/oTTsVDh+JuyB9KMN/q8xQQU0dXwYki5sB2Sy2g6+DolG oRzi1fY3AgGFjxchn6BH+mV90N6ryYbZrde0bYqZiIwsfg6Mw1cMNZ2XM75roZijkXD/ sQhg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@bytedance-com.20150623.gappssmtp.com header.s=20150623 header.b=cftgdsHy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=bytedance.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r193si3381767ior.11.2021.07.11.09.30.47; Sun, 11 Jul 2021 09:30:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@bytedance-com.20150623.gappssmtp.com header.s=20150623 header.b=cftgdsHy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=bytedance.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235839AbhGKPb7 (ORCPT + 99 others); Sun, 11 Jul 2021 11:31:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55440 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235840AbhGKPb6 (ORCPT ); Sun, 11 Jul 2021 11:31:58 -0400 Received: from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com [IPv6:2607:f8b0:4864:20::1042]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 10A49C0613E8 for ; Sun, 11 Jul 2021 08:29:12 -0700 (PDT) Received: by mail-pj1-x1042.google.com with SMTP id cu14so3262593pjb.0 for ; Sun, 11 Jul 2021 08:29:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=n3YRHkj0c58PDufExs5GW5EqSsqejMDzvuIBPPdMvOg=; b=cftgdsHyZHCgsdByowpFdQGqA8FjOTGeVyz2P4bO1ZPaUO0n6CJX5jUm0hkxED8zJU xq0gPh0zyTrbhWUCVnYCl9bXBNqoDAEEFBtUYF9VTbhlnbXwl7oof186KaSl6kpWyLdC TPuQygx25GH/YQdmFAbmgOvPIn0iGuugXMqezVWieGdmwgTAI2lb39QeTGSKIGO6ZukQ QemdQWgetrSXQJ/NuI0UGINfnUhbQW6KvuPJlat/l9Rfe1IgacNIagJ1La0M6T0Ivy84 qPq40yPnzuWfNAjtVjtqbmf+/FATYadA9qE7b2wNo75WEtZ7/r3epXFna48S+AgdmBri xM9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=n3YRHkj0c58PDufExs5GW5EqSsqejMDzvuIBPPdMvOg=; b=g6YeJK910DOVoJJKb1zaTZYunpTjqcUVsRPOVJPHXerS5Sw8V53KwSXj1vb6DfQ5Sj Y7mpChnoWY9Z4C8d5KXzhDRIxKujY07vAgxvMDFJp/fbwyRnZVDSMjY2exjX4qR7j4pC JzOHxvu+q9GQNCZ4Xwz3nDMyQqAujygC5RG5e3f8kXBr6atY7Ku4gIlEQVcaOIykHmLC akmDDNVULMds8HhU7cTiTIeLTXDuFQBRLydUnIRlA2lY+p77AnIqsKOuJaX0ieePVZcp ZiDtRoJaqTWDLBuRLpGVQobT9Bjun1VhPixhuYT/36LtRcjmnX+mlNcVmN77BTjgcNSx iB2g== X-Gm-Message-State: AOAM533CawmpE6M1nlDgXGRyiuw2FS8u/+PfZ4794bSgP8Sj3Q4fhJyM qStX7VjHTo1V60avUoXcQdjfEA== X-Received: by 2002:a17:90a:7d13:: with SMTP id g19mr49022034pjl.163.1626017351507; Sun, 11 Jul 2021 08:29:11 -0700 (PDT) Received: from ?IPv6:240e:38a:3604:2400:8585:964d:c4ab:ba4c? ([240e:38a:3604:2400:8585:964d:c4ab:ba4c]) by smtp.gmail.com with ESMTPSA id j20sm9964157pfc.203.2021.07.11.08.28.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 11 Jul 2021 08:29:10 -0700 (PDT) Subject: Re: [PATCH -tip v8 11/13] x86/unwind: Recover kretprobe trampoline entry To: Masami Hiramatsu Cc: Peter Zijlstra , Steven Rostedt , Josh Poimboeuf , Ingo Molnar , X86 ML , Daniel Xu , linux-kernel@vger.kernel.org, bpf@vger.kernel.org, kuba@kernel.org, mingo@redhat.com, ast@kernel.org, Thomas Gleixner , Borislav Petkov , kernel-team@fb.com, yhs@fb.com, linux-ia64@vger.kernel.org, Abhishek Sagar , Andrii Nakryiko References: <162399992186.506599.8457763707951687195.stgit@devnote2> <162400002631.506599.2413605639666466945.stgit@devnote2> <20210706004257.9e282b98f447251a380f658f@kernel.org> <20210706111136.7c5e9843@oasis.local.home> <20210707191510.cb48ca4a20f0502ce6c46508@kernel.org> <20210707194530.766a9c8364f3b2d7714ca590@kernel.org> <20210707222925.87ecc1391d0ab61db3d8398e@kernel.org> <3fc578e0-5b26-6067-d026-5b5d230d6720@bytedance.com> <20210711230909.dac1ff010a94831d5e9c25cd@kernel.org> From: Matt Wu Message-ID: <9c160404-ad6d-816a-93ed-91bb6e7c26a9@bytedance.com> Date: Sun, 11 Jul 2021 23:28:49 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210711230909.dac1ff010a94831d5e9c25cd@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021/7/11 PM10:09, Masami Hiramatsu wrote: > On Wed, 7 Jul 2021 22:42:47 +0800 > Matt Wu wrote: > >> On 2021/7/7 PM9:29, Masami Hiramatsu wrote: >>> On Wed, 7 Jul 2021 19:45:30 +0900 >>> Masami Hiramatsu wrote: >>> >>>> On Wed, 7 Jul 2021 12:20:57 +0200 >>>> Peter Zijlstra wrote: >>>> >>>>> On Wed, Jul 07, 2021 at 07:15:10PM +0900, Masami Hiramatsu wrote: >>>>> >>>>>> I actually don't want to keep this feature because no one use it. >>>>>> (only systemtap needs it?) >>>>> >>>>> Yeah, you mentioned systemtap, but since that's out-of-tree I don't >>>>> care. Their problem. >>> >>> Yeah, maybe it is not hard to update. >>> >>>>> >>>>>> Anyway, if we keep the idea-level compatibility (not code level), >>>>>> what we need is 'void *data' in the struct kretprobe_instance. >>>>>> User who needs it can allocate their own instance data for their >>>>>> kretprobes when initialising it and sets in their entry handler. >>>>>> >>>>>> Then we can have a simple kretprobe_instance. >>>>> >>>>> When would you do the alloc? When installing the retprobe, but that >>>>> might be inside the allocator, which means you can't call the allocator >>>>> etc.. :-) >>>> >>>> Yes, so the user may need to allocate a pool right before register_kretprobe(). >>>> (whether per-kretprobe or per-task or global pool, that is user's choice.) >>>> >>>>> >>>>> If we look at struct ftrace_ret_stack, it has a few fixed function >>>>> fields. The calltime one is all that is needed for the kretprobe >>>>> example code. >>>> >>>> kretprobe consumes 3 fields, a pointer to 'struct kretprobe' (which >>>> stores callee function address in 'kretprobe::kp.addr'), a return >>>> address and a frame pointer (*). >>> > Oops, I forgot to add "void *data" for storing user data. >>> >> >> Should use "struct kretprobe_holder *rph", since "struct kretprobe" belongs >> to 3rd-party module (which might be unloaded any time). > > Good catch. Yes, instead of 'struct kretprobe', we need to use the holder. > >> User's own pool might not work if the module can be unloaded. Better manage >> the pool in kretprobe_holder, which needs no changes from user side. > > No, since the 'data' will be only refered from user handler. If the kretprobe > is released, then the kretprobe_holder will clear the refernce to the 'struct > kretprobe'. Then, the user handler is never called. No one access the 'data'. Indeed, there is no race of "data" accessing, since unregister_kretprobes() is taking care of it. This implementation just increases the complexity of caller to keep track of all allocated instances and release them after unregistration. But guys are likely to use kmalloc in pre-handler and kfree in post-handler, which will lead to memory leaks.