From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 57D81C3DA49 for ; Thu, 18 Jul 2024 11:19:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=fW0+7S1MyDMJOr70o29vJf7jsTrOQxzuP1cCqz9vqps=; b=1pCcI+faRebZSekSqOYgCAKA4p 72/ooqElpi3DidcNWFAAdt6gSH9B7zyJgVjkgFV4j9zd1axFJdTRLKsWh/EarzHvzIkKJI0KQzW9n VuMiXiZU3EjDI1xN5f1i2+hRHqRmanTXWznx8VrdvunHvZjXnHWFkKHUNZqsUy90IWbk3j6oLNfKe GDAeo355TRLq3YdopYT6OdBf4jdWAAwMcItvPPXsLleUvfMI97A6qEtp+Dymm0BNdjsptieGE8eYA NwwxpS2vWUz/SD/XYdjOa/55oVZLhHuxPKcg20I/UTNVkyr79i6BD5BFG8i5JzxGWmkrvjcSbyRSc 2CS5gJ8A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sUPAf-0000000GoaU-0vwS; Thu, 18 Jul 2024 11:19:37 +0000 Received: from mgamail.intel.com ([198.175.65.12]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sUPAJ-0000000GoVP-2vD7 for linux-arm-kernel@lists.infradead.org; Thu, 18 Jul 2024 11:19:17 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1721301556; x=1752837556; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=0AhJ90EdbnHIUUwth+DAtYzqSMzjIxsaALpFpmPbiqM=; b=k/YUTn4bflqckgVdPh8KtTPM5SxpR5U/W9pD9oFVoQApgo3e//Rlb/+k qIddW4PlwRf1IJSfsOdjmZx5sBkhhpltokb4OtuIlsTxEsQLWzrxE68ad Oa6Yq33FPILZLFkOJxrsAJKA+s1dmpb2EpwDZ9+HdAaBmgu2PYmsrhUtx jf1TgOZens2SyfxQcMfaN6g+0pJYufAkpqnOhw78X+0LH/z99aFkOWiH2 giCCKPpokBiQZ9m3yhVSvfKHZQZ9ahXy+GFwvxeb5mpW5Ap/3q+60wSmG 6OzvIKJ9X7PpsgyAjmfT6V96U/u9MKvGyRj6q2YEW17Myo/UVm/D0GxuC Q==; X-CSE-ConnectionGUID: dfS45FC2TY+hFlKoJMf51g== X-CSE-MsgGUID: 9FqWpSZMRpOj6SxbR0x+mQ== X-IronPort-AV: E=McAfee;i="6700,10204,11136"; a="30243882" X-IronPort-AV: E=Sophos;i="6.09,217,1716274800"; d="scan'208";a="30243882" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by orvoesa104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2024 04:19:15 -0700 X-CSE-ConnectionGUID: sYHOG/iRSZO0WsTnDjF1Bw== X-CSE-MsgGUID: QwXKpOj7RLeR8oUvmJ6iyQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,217,1716274800"; d="scan'208";a="88216115" Received: from ahunter6-mobl1.ger.corp.intel.com (HELO [10.0.2.15]) ([10.94.249.84]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Jul 2024 04:19:08 -0700 Message-ID: <14cd68b2-eeee-42e3-87a6-c12d3814bd51@intel.com> Date: Thu, 18 Jul 2024 14:19:03 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH V9 02/13] perf/core: Add aux_pause, aux_resume, aux_start_paused To: Peter Zijlstra Cc: Ingo Molnar , Mark Rutland , Alexander Shishkin , Heiko Carstens , Thomas Richter , Hendrik Brueckner , Suzuki K Poulose , Mike Leach , James Clark , coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, Yicong Yang , Jonathan Cameron , Will Deacon , Arnaldo Carvalho de Melo , Jiri Olsa , Namhyung Kim , Ian Rogers , Andi Kleen , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org References: <20240715160712.127117-1-adrian.hunter@intel.com> <20240715160712.127117-3-adrian.hunter@intel.com> <20240718093846.GJ26750@noisy.programming.kicks-ass.net> Content-Language: en-US From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki In-Reply-To: <20240718093846.GJ26750@noisy.programming.kicks-ass.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240718_041915_825836_F1CB84D9 X-CRM114-Status: GOOD ( 27.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 18/07/24 12:38, Peter Zijlstra wrote: > On Mon, Jul 15, 2024 at 07:07:01PM +0300, Adrian Hunter wrote: >> Hardware traces, such as instruction traces, can produce a vast amount of >> trace data, so being able to reduce tracing to more specific circumstances >> can be useful. >> >> The ability to pause or resume tracing when another event happens, can do >> that. >> >> Add ability for an event to "pause" or "resume" AUX area tracing. >> >> Add aux_pause bit to perf_event_attr to indicate that, if the event >> happens, the associated AUX area tracing should be paused. Ditto >> aux_resume. Do not allow aux_pause and aux_resume to be set together. >> >> Add aux_start_paused bit to perf_event_attr to indicate to an AUX area >> event that it should start in a "paused" state. >> >> Add aux_paused to struct hw_perf_event for AUX area events to keep track of >> the "paused" state. aux_paused is initialized to aux_start_paused. >> >> Add PERF_EF_PAUSE and PERF_EF_RESUME modes for ->stop() and ->start() >> callbacks. Call as needed, during __perf_event_output(). Add > > Why in __perf_event_output() rather than in __perf_event_overflow(). > Specifically, before bpf_overflow_handler(). > > That is, what do we want BPF to be able to do here? To me it seems > strange that BPF would be able to affect this functionality. You want > this pause/resume to happen irrespective of how the rest of the event is > processed, no? The thought was to have the output match up with pause/resume, but it doesn't really make much difference. Putting it before the BPF handler is reasonable. > >> aux_in_pause_resume to struct perf_buffer to prevent races with the NMI >> handler. Pause/resume in NMI context will miss out if it coincides with >> another pause/resume. > > I'm struggling here. That variable is only ever used inside that one > function. So it must be self-recursion. Are you thinking something like: > > swevent_overflow() > ... > event_aux_pause() > > event_overflow() > ... > event_aux_pause() > > ? > > Where two events in the group, one software and one hardware, are both > trying to control the AUX thing? Exactly that yes. > Because I don't think the PT-PMI ever > gets here. No it doesn't. AUX pause/resume is something a non-AUX event in the group does to the AUX event which is the group leader. > >> To use aux_pause or aux_resume, an event must be in a group with the AUX >> area event as the group leader. > > >> @@ -402,6 +411,15 @@ struct pmu { >> * >> * ->start() with PERF_EF_RELOAD will reprogram the counter >> * value, must be preceded by a ->stop() with PERF_EF_UPDATE. >> + * >> + * ->stop() with PERF_EF_PAUSE will stop as simply as possible. Will not >> + * overlap another ->stop() with PERF_EF_PAUSE nor ->start() with >> + * PERF_EF_RESUME. >> + * >> + * ->start() with PERF_EF_RESUME will start as simply as possible but >> + * only if the counter is not otherwise stopped. Will not overlap >> + * another ->start() with PERF_EF_RESUME nor ->stop() with >> + * PERF_EF_PAUSE. >> */ >> void (*start) (struct perf_event *event, int flags); >> void (*stop) (struct perf_event *event, int flags); > > Notably, they *can* race with ->stop/start without EF_PAUSE/RESUME. Yes that would be worth adding to the comments.