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;
MIME-Version: 1.0
References: <20230112214015.1014857-1-namhyung@kernel.org> <Y8Fb45iZ5yp3TUDD@krava>
In-Reply-To: <Y8Fb45iZ5yp3TUDD@krava>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Tue, 17 Jan 2023 00:12:57 -0800
Message-ID: <CAM9d7cjUAOhME8jf=sRbzWFcaGssaXRhhL9rwkBxSLjdPaZg8w@mail.gmail.com>
Subject: Re: [PATCHSET 0/8] perf/core: Prepare sample data for BPF
To:     Jiri Olsa <olsajiri@gmail.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, Song Liu <song@kernel.org>,
        bpf@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Jiri,

On Fri, Jan 13, 2023 at 5:26 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Jan 12, 2023 at 01:40:07PM -0800, Namhyung Kim wrote:
> > Hello,
> >
> > The perf_prepare_sample() is to fill the perf sample data and update the
> > header info before sending it to the ring buffer.  But we want to use it
> > for BPF overflow handler so that it can access the sample data to filter
> > relevant ones.
> >
> > Changes in v2)
> >  * the layout change is merged
> >  * reduce branches using __cond_set  (PeterZ)
> >  * add helpers to set dynamic sample data  (PeterZ)
> >  * introduce perf_prepare_header()  (PeterZ)
> >  * call perf_prepare_sample() before bpf_overflow_handler unconditionally
> >
> > This means the perf_prepare_handler() can be called more than once.  To
> > avoid duplicate work, use the data->sample_flags and save the data size.
> >
> > I also added a few of helpers to set those information accordingly.
> > But it looks some fields like REGS_USER, STACK_USER and AUX are saved in
> > the perf_prepare_sample() so I didn't add the helpers for them.
> >
> > After than we can just check the filtered_sample_type flags begin zero
> > to determine if it has more work.  In that case, it needs to update the
> > data->type since it's possible to miss when PMU driver sets all required
> > sample flags before calling perf_prepare_sample().
> >
> > The code is also available at 'perf/prepare-sample-v2' branch in
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >
> > Thanks,
> > Namhyung
> >
> >
> > Cc: Song Liu <song@kernel.org>
> > Cc: bpf@vger.kernel.org
> >
> >
> > Namhyung Kim (8):
> >   perf/core: Save the dynamic parts of sample data size
> >   perf/core: Add perf_sample_save_callchain() helper
> >   perf/core: Add perf_sample_save_raw_data() helper
> >   perf/core: Add perf_sample_save_brstack() helper
> >   perf/core: Set data->sample_flags in perf_prepare_sample()
> >   perf/core: Do not pass header for sample id init
> >   perf/core: Introduce perf_prepare_header()
> >   perf/core: Call perf_prepare_sample() before running BPF
>
> lgtm, I ran the bpf selftests on top of that and it's ok

Thanks for your review.  I'll add your Acked-by and Tested-by
in the v3. :)

Thanks,
Namhyung