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;
Subject: Re: [PATCH v7 0/6] perf record: Track sideband events for all CPUs
 when tracing selected CPUs
To:     Ravi Bangoria <ravi.bangoria@amd.com>, <acme@kernel.org>,
        <irogers@google.com>, <jolsa@kernel.org>, <namhyung@kernel.org>,
        <adrian.hunter@intel.com>
CC:     <peterz@infradead.org>, <mingo@redhat.com>, <mark.rutland@arm.com>,
        <alexander.shishkin@linux.intel.com>, <kan.liang@linux.intel.com>,
        <james.clark@arm.com>, <tmricht@linux.ibm.com>,
        <ak@linux.intel.com>, <anshuman.khandual@arm.com>,
        <linux-kernel@vger.kernel.org>, <linux-perf-users@vger.kernel.org>
References: <20230826032608.107261-1-yangjihong1@huawei.com>
 <bcfabb85-4dc1-e642-4a82-f3b8cc009b0b@amd.com>
From:   Yang Jihong <yangjihong1@huawei.com>
Message-ID: <93abb0e0-1691-a887-f7b2-7591f83a8618@huawei.com>
Date:   Tue, 29 Aug 2023 17:34:59 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
MIME-Version: 1.0
In-Reply-To: <bcfabb85-4dc1-e642-4a82-f3b8cc009b0b@amd.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello,

On 2023/8/29 13:37, Ravi Bangoria wrote:
> On 26-Aug-23 8:56 AM, Yang Jihong wrote:
>> User space tasks can migrate between CPUs, track sideband events for all
>> CPUs.
> 
> I've tested this series with simple test program and it mostly works fine.
> 
> Tested-by: Ravi Bangoria <ravi.bangoria@amd.com>
> 
Thanks for the test.

> Since we are recording sideband data on all cpus, perf will endup recording
> lots of unnecessary data, esp. on large systems. E.g. on a 128 cpu system:
> 
> Without patch:
>    $ sudo ./perf record -C 10 -o perf.data.without -- sleep 10
>    $ du -d1 -ha ./perf.data.without
>    3.0M    ./perf.data.without
> 
>    $ sudo ./perf report --stats -i perf.data.without
>    Aggregated stats:
>               TOTAL events:      47011
>                MMAP events:         51  ( 0.1%)
>                COMM events:       1549  ( 3.3%)
>                EXIT events:        105  ( 0.2%)
>                FORK events:       1544  ( 3.3%)
>              SAMPLE events:      38226  (81.3%)
>               MMAP2 events:       5485  (11.7%)
>               ...
>    cycles:P stats:
>              SAMPLE events:      38226
> 
> With patch:
>    $ sudo ./perf record -C 10 -o perf.data.with -- sleep 10
>    $ du -d1 -ha ./perf.data.with
>    15M     ./perf.data.with
> 
>    $ sudo ./perf report --stats -i perf.data.with
>    Aggregated stats:
>               TOTAL events:     160164
>                MMAP events:         51  ( 0.0%)
>                COMM events:      12879  ( 8.0%)
>                EXIT events:      11192  ( 7.0%)
>                FORK events:      12669  ( 7.9%)
>              SAMPLE events:      38464  (24.0%)
>               MMAP2 events:      84844  (53.0%)
>               ...
>    cycles:P stats:
>              SAMPLE events:      38464
> 
> Number of actual samples are same ~38K. However, the perf.data file is 5x
> bigger because of additional sideband data.

Yes, if record system wide sideband data, the amount of sideband events 
will increase proportionally, which is expected.

> 
> I'm pretty sure we don't need most of those additional data. So, thinking
> loud, should we post-process perf.data file and filter out unnecessary data?
> 

I wonder if we can add a new function in perf inject.
By reading perf.data and comparing tid of SAMPLE events and sideband 
events, we can filter out the sideband data of unmatched tasks.

That's just my idea, or there's another better solution.

Thanks,
Yang