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;
Content-Type: text/plain;
        charset=us-ascii
Subject: Re: [tip: perf/core] perf: Use sample_flags for raw_data
From:   Athira Rajeev <atrajeev@linux.vnet.ibm.com>
In-Reply-To: <166434824149.401.4361243714612738808.tip-bot2@tip-bot2>
Date:   Wed, 19 Oct 2022 16:14:03 +0530
Cc:     linux-tip-commits@vger.kernel.org,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>, x86@kernel.org
Message-Id: <824365F4-EA1C-46E1-9C44-D79F1705FD56@linux.vnet.ibm.com>
References: <20220921220032.2858517-2-namhyung@kernel.org>
 <166434824149.401.4361243714612738808.tip-bot2@tip-bot2>
To:     LKML <linux-kernel@vger.kernel.org>,
        Namhyung Kim <namhyung@kernel.org>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk


> On 28-Sep-2022, at 12:27 PM, tip-bot2 for Namhyung Kim <tip-bot2@linutron=
ix.de> wrote:
>=20
> The following commit has been merged into the perf/core branch of tip:
>=20
> Commit-ID:     838d9bb62d132ec3baf1b5aba2e95ef9a7a9a3cd
> Gitweb:        https://git.kernel.org/tip/838d9bb62d132ec3baf1b5aba2e95ef=
9a7a9a3cd
> Author:        Namhyung Kim <namhyung@kernel.org>
> AuthorDate:    Wed, 21 Sep 2022 15:00:32 -07:00
> Committer:     Peter Zijlstra <peterz@infradead.org>
> CommitterDate: Tue, 27 Sep 2022 22:50:24 +02:00
>=20
> perf: Use sample_flags for raw_data
>=20
> Use the new sample_flags to indicate whether the raw data field is
> filled by the PMU driver.  Although it could check with the NULL,
> follow the same rule with other fields.
>=20
> Remove the raw field from the perf_sample_data_init() to minimize
> the number of cache lines touched.
>=20
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20220921220032.2858517-2-namhyung@kernel.=
org

Hi Namhyung,

This commit ("perf: Use sample_flags for raw_data") added
PERF_SAMPLE_RAW check in perf_prepare_sample. To be in sync
while we output sample to memory, do we also need to add
similar check in perf_output_sample ? I am pasting change below.
Please share your thoughts.

From 46d874bc4a915dd710ddbc5198588cbb66d3ea8e Mon Sep 17 00:00:00 2001
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Date: Wed, 19 Oct 2022 13:02:06 +0530
Subject: [PATCH] perf/core: Update sample_flags for raw_data in
 perf_output_sample

commit 838d9bb62d13 ("perf: Use sample_flags for raw_data")
added check for PERF_SAMPLE_RAW in sample_flags in
perf_prepare_sample(). But while copying the sample in memory,
the check for sample_flags is not added in perf_output_sample().
Fix adds the same in perf_output_sample as well.

Fixes: 838d9bb62d13 ("perf: Use sample_flags for raw_data")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4ec3717003d5..daf387c75d33 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7099,7 +7099,7 @@ void perf_output_sample(struct perf_output_handle *ha=
ndle,
 	if (sample_type & PERF_SAMPLE_RAW) {
 		struct perf_raw_record *raw =3D data->raw;
=20
-		if (raw) {
+		if (raw && (data->sample_flags & PERF_SAMPLE_RAW)) {
 			struct perf_raw_frag *frag =3D &raw->frag;
=20
 			perf_output_put(handle, raw->size);
--=20
2.31.1

Thanks
Athira

> ---
> arch/s390/kernel/perf_cpum_cf.c    | 1 +
> arch/s390/kernel/perf_pai_crypto.c | 1 +
> arch/x86/events/amd/ibs.c          | 1 +
> include/linux/perf_event.h         | 5 ++---
> kernel/events/core.c               | 3 ++-
> 5 files changed, 7 insertions(+), 4 deletions(-)
>=20
> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum=
_cf.c
> index f7dd3c8..f043a7f 100644
> --- a/arch/s390/kernel/perf_cpum_cf.c
> +++ b/arch/s390/kernel/perf_cpum_cf.c
> @@ -664,6 +664,7 @@ static int cfdiag_push_sample(struct perf_event *even=
t,
> 		raw.frag.data =3D cpuhw->stop;
> 		raw.size =3D raw.frag.size;
> 		data.raw =3D &raw;
> +		data.sample_flags |=3D PERF_SAMPLE_RAW;
> 	}
>=20
> 	overflow =3D perf_event_overflow(event, &data, &regs);
> diff --git a/arch/s390/kernel/perf_pai_crypto.c b/arch/s390/kernel/perf_p=
ai_crypto.c
> index b38b4ae..6826e2a 100644
> --- a/arch/s390/kernel/perf_pai_crypto.c
> +++ b/arch/s390/kernel/perf_pai_crypto.c
> @@ -366,6 +366,7 @@ static int paicrypt_push_sample(void)
> 		raw.frag.data =3D cpump->save;
> 		raw.size =3D raw.frag.size;
> 		data.raw =3D &raw;
> +		data.sample_flags |=3D PERF_SAMPLE_RAW;
> 	}
>=20
> 	overflow =3D perf_event_overflow(event, &data, &regs);
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index ce5720b..c29a006 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -781,6 +781,7 @@ fail:
> 			},
> 		};
> 		data.raw =3D &raw;
> +		data.sample_flags |=3D PERF_SAMPLE_RAW;
> 	}
>=20
> 	/*
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index f4a1357..e9b151c 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1028,7 +1028,6 @@ struct perf_sample_data {
> 	 * minimize the cachelines touched.
> 	 */
> 	u64				sample_flags;
> -	struct perf_raw_record		*raw;
> 	u64				period;
>=20
> 	/*
> @@ -1040,6 +1039,7 @@ struct perf_sample_data {
> 	union  perf_mem_data_src	data_src;
> 	u64				txn;
> 	u64				addr;
> +	struct perf_raw_record		*raw;
>=20
> 	u64				type;
> 	u64				ip;
> @@ -1078,8 +1078,7 @@ static inline void perf_sample_data_init(struct per=
f_sample_data *data,
> 					 u64 addr, u64 period)
> {
> 	/* remaining struct members initialized in perf_prepare_sample() */
> -	data->sample_flags =3D 0;
> -	data->raw  =3D NULL;
> +	data->sample_flags =3D PERF_SAMPLE_PERIOD;
> 	data->period =3D period;
>=20
> 	if (addr) {
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a91f74d..04e19a8 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7332,7 +7332,7 @@ void perf_prepare_sample(struct perf_event_header *=
header,
> 		struct perf_raw_record *raw =3D data->raw;
> 		int size;
>=20
> -		if (raw) {
> +		if (raw && (data->sample_flags & PERF_SAMPLE_RAW)) {
> 			struct perf_raw_frag *frag =3D &raw->frag;
> 			u32 sum =3D 0;
>=20
> @@ -7348,6 +7348,7 @@ void perf_prepare_sample(struct perf_event_header *=
header,
> 			frag->pad =3D raw->size - sum;
> 		} else {
> 			size =3D sizeof(u64);
> +			data->raw =3D NULL;
> 		}
>=20
> 		header->size +=3D size;