From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id DA09917BA4; Wed, 24 Jan 2024 09:04:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706087046; cv=none; b=AzhkaTmONo6xe+/lxDPeVN3pgUFr6T17tOa72RU341Dybdo2HkdrP8oS5eubtm9opDRrsFLlEZ3pc96etrMfgnOpZCPQDASOPpstLaxwyYTQJ+I2cnJV+qjnrMIFdKwMd07BcN2PxtreweS22REnksU81v6+iiuWMP+AnvjFg+A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706087046; c=relaxed/simple; bh=4FLO9HlQQlJv6Hs845TgL+okgrt3W85KJoDNho2nIIw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=RIa7ccIAujG2krhVsiBD49cQAUz6itVSWToTlZfu9Sedh/JjkMiUVdwGddHRX3kgVOzuMjFGhhyLamWLK6KLRcVnhdy9iLEv3mlM/BCGBFsdfuFSYxiWA+2ysZaiL1NDwaa3GcVFa156x1YbtTD9TmOI/H7aVaRCl454vjkzeyY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BCA411FB; Wed, 24 Jan 2024 01:04:45 -0800 (PST) Received: from [10.57.86.221] (unknown [10.57.86.221]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D389E3F762; Wed, 24 Jan 2024 01:03:58 -0800 (PST) Message-ID: Date: Wed, 24 Jan 2024 09:03:57 +0000 Precedence: bulk X-Mailing-List: linux-perf-users@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v3] perf evlist: Fix evlist__new_default() for > 1 core PMU Content-Language: en-US To: Namhyung Kim Cc: linux-perf-users@vger.kernel.org, irogers@google.com, kan.liang@linux.intel.com, mark.rutland@arm.com, Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Changbin Du , Yang Jihong , linux-kernel@vger.kernel.org References: <20240123102728.239147-1-james.clark@arm.com> <20240123103918.241423-1-james.clark@arm.com> From: James Clark In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 24/01/2024 00:46, Namhyung Kim wrote: > Hi James, > > On Tue, Jan 23, 2024 at 2:39 AM James Clark wrote: >> >> The 'Session topology' test currently fails with this message when >> evlist__new_default() opens more than one event: >> >> 32: Session topology : >> --- start --- >> templ file: /tmp/perf-test-vv5YzZ >> Using CPUID 0x00000000410fd070 >> Opening: unknown-hardware:HG >> ------------------------------------------------------------ >> perf_event_attr: >> type 0 (PERF_TYPE_HARDWARE) >> config 0xb00000000 >> disabled 1 >> ------------------------------------------------------------ >> sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 4 >> Opening: unknown-hardware:HG >> ------------------------------------------------------------ >> perf_event_attr: >> type 0 (PERF_TYPE_HARDWARE) >> config 0xa00000000 >> disabled 1 >> ------------------------------------------------------------ >> sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 5 >> non matching sample_type >> FAILED tests/topology.c:73 can't get session >> ---- end ---- >> Session topology: FAILED! >> >> This is because when re-opening the file and parsing the header, Perf >> expects that any file that has more than one event has the sample ID >> flag set. Perf record already sets the flag in a similar way when there >> is more than one event, so add the same logic to evlist__new_default(). >> >> evlist__new_default() is only currently used in tests, so I don't >> expect this change to have any other side effects. The other tests that >> use it don't save and re-open the file so don't hit this issue. >> >> The session topology test has been failing on Arm big.LITTLE platforms >> since commit 251aa040244a ("perf parse-events: Wildcard most >> "numeric" events") when evlist__new_default() started opening multiple >> events for 'cycles'. >> >> Fixes: 251aa040244a ("perf parse-events: Wildcard most "numeric" events") >> Closes: https://lore.kernel.org/lkml/CAP-5=fWVQ-7ijjK3-w1q+k2WYVNHbAcejb-xY0ptbjRw476VKA@mail.gmail.com/ >> Tested-by: Ian Rogers >> Reviewed-by: Ian Rogers >> Tested-by: Kan Liang >> Signed-off-by: James Clark >> --- >> tools/perf/util/evlist.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> Changes since v2: >> >> * Undo the fact that v2 was accidentally based on v1 instead of >> perf-tools >> >> Changes since v1: >> >> * Reduce scope of evsel variable >> * Add argument label >> * Change summary to be less specific about the failing test >> * Add the closes: tag >> >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >> index 95f25e9fb994..979a6053a84d 100644 >> --- a/tools/perf/util/evlist.c >> +++ b/tools/perf/util/evlist.c >> @@ -106,6 +106,13 @@ struct evlist *evlist__new_default(void) >> evlist = NULL; >> } >> >> + if (evlist->core.nr_entries > 1) { > > I think you need a NULL check for evlist here. > > Thanks, > Namhyung > > Oops yes. Or just return on the error above. >> + struct evsel *evsel; >> + >> + evlist__for_each_entry(evlist, evsel) >> + evsel__set_sample_id(evsel, /*can_sample_identifier=*/false); >> + } >> + >> return evlist; >> } >> >> -- >> 2.34.1 >>