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;
Date:   Mon, 30 Nov 2020 16:46:51 +0000
From:   Will Deacon <will@kernel.org>
To:     James Clark <james.clark@arm.com>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org,
        Mark Rutland <mark.rutland@arm.com>,
        Al Grant <al.grant@arm.com>, Leo Yan <leo.yan@linaro.org>,
        John Garry <john.garry@huawei.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>
Subject: Re: [PATCH] drivers/perf: Enable PID_IN_CONTEXTIDR with SPE
Message-ID: <20201130164650.GA25187@willie-the-truck>
References: <20201130162454.28255-1-james.clark@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201130162454.28255-1-james.clark@arm.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Mon, Nov 30, 2020 at 06:24:54PM +0200, James Clark wrote:
> Enable PID_IN_CONTEXTIDR by default when Arm SPE is enabled.
> This flag is required to get PID data in the SPE trace. Without
> it the perf tool will report 0 for PID which isn't very useful,
> especially when doing system wide profiling or profiling
> applications that fork.

Can perf not figure out the pid some other way? (e.g. by tracing context
switches and correlating that with the SPE data?). Also, how does this
work with pid namespaces?

> There is a small performance overhead when enabling
> PID_IN_CONTEXTIDR, but SPE itself is optional and not enabled by
> default so the impact is minimised.
> 
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Al Grant <al.grant@arm.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: John Garry <john.garry@huawei.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: James Clark <james.clark@arm.com>
> ---
>  drivers/perf/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index 130327ff0b0e..47ede46c3d57 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -125,6 +125,7 @@ config XGENE_PMU
>  config ARM_SPE_PMU
>  	tristate "Enable support for the ARMv8.2 Statistical Profiling Extension"
>  	depends on ARM64
> +	select PID_IN_CONTEXTIDR

Probably better to make PID_IN_CONTEXTIDR 'default y' if SPE is enabled,
rather than selecting it directly. That way, at least people can turn it
off if they don't want it.

Will