Message-ID: <20250429095335.GH551819@e132581.arm.com>
Date: Tue, 29 Apr 2025 10:53:35 +0100
From: Leo Yan <leo.yan@....com>
To: Ian Rogers <irogers@...gle.com>
Cc: Arnd Bergmann <arnd@...db.de>, Namhyung Kim <namhyung@...nel.org>,
	Yury Norov <yury.norov@...il.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Darren Hart <dvhart@...radead.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	André Almeida <andrealmeid@...lia.com>,
	John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
	James Clark <james.clark@...aro.org>,
	Mike Leach <mike.leach@...aro.org>, Leo Yan <leo.yan@...ux.dev>,
	Yicong Yang <yangyicong@...ilicon.com>,
	Jonathan Cameron <jonathan.cameron@...wei.com>,
	Nathan Chancellor <nathan@...nel.org>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Al Viro <viro@...iv.linux.org.uk>, Kyle Meyer <kyle.meyer@....com>,
	Ben Gainey <ben.gainey@....com>,
	Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
	Kajol Jain <kjain@...ux.ibm.com>,
	Aditya Gupta <adityag@...ux.ibm.com>,
	Eder Zulian <ezulian@...hat.com>,
	Dapeng Mi <dapeng1.mi@...ux.intel.com>,
	Kuan-Wei Chiu <visitorckw@...il.com>, He Zhe <zhe.he@...driver.com>,
	Dirk Gouders <dirk@...ders.net>, Brian Geffon <bgeffon@...gle.com>,
	Ravi Bangoria <ravi.bangoria@....com>,
	Howard Chu <howardchu95@...il.com>,
	Charlie Jenkins <charlie@...osinc.com>,
	Colin Ian King <colin.i.king@...il.com>,
	Dominique Martinet <asmadeus@...ewreck.org>,
	Jann Horn <jannh@...gle.com>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Yang Jihong <yangjihong@...edance.com>,
	Dmitry Vyukov <dvyukov@...gle.com>, Andi Kleen <ak@...ux.intel.com>,
	Graham Woodward <graham.woodward@....com>,
	Ilkka Koskinen <ilkka@...amperecomputing.com>,
	Anshuman Khandual <anshuman.khandual@....com>,
	Zhongqiu Han <quic_zhonhan@...cinc.com>, Hao Ge <gehao@...inos.cn>,
	Tengda Wu <wutengda@...weicloud.com>,
	Gabriele Monaco <gmonaco@...hat.com>,
	Chun-Tse Shao <ctshao@...gle.com>,
	Casey Chen <cachen@...estorage.com>,
	"Dr. David Alan Gilbert" <linux@...blig.org>,
	Li Huafei <lihuafei1@...wei.com>,
	"Steinar H. Gunderson" <sesse@...gle.com>,
	Levi Yun <yeoreum.yun@....com>, Weilin Wang <weilin.wang@...el.com>,
	Thomas Falcon <thomas.falcon@...el.com>,
	Thomas Richter <tmricht@...ux.ibm.com>,
	Andrew Kreimer <algonell@...il.com>,
	Krzysztof Łopatowski <krzysztof.m.lopatowski@...il.com>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Jean-Philippe Romain <jean-philippe.romain@...s.st.com>,
	Junhao He <hejunhao3@...wei.com>,
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
	Xu Yang <xu.yang_2@....com>,
	Steve Clevenger <scclevenger@...amperecomputing.com>,
	Zixian Cai <fzczx123@...il.com>,
	Stephen Brennan <stephen.s.brennan@...cle.com>,
	Yujie Liu <yujie.liu@...el.com>, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, llvm@...ts.linux.dev
Subject: Re: [PATCH v1 09/48] perf tests: Silence -Wshorten-64-to-32 warnings

Hi Ian,

On Mon, Apr 28, 2025 at 09:29:37AM -0700, Ian Rogers wrote:

[...]

> Hi Leo,
> 
> So I sent out cleaning up the kernel headers separately:
> https://lore.kernel.org/lkml/20250403165702.396388-1-irogers@google.com/
> Arnd commented that it would be nicer to do larger changes, but my
> concern was breaking printf modifiers, etc. It looks like those
> patches have lost momentum, not sure what's going to happen about them
> being merged.
> 
> On the fixes vs refactoring. I'm not sure refactoring is the right
> term for the other things in the series. It is basically trying to
> make implicit casts explicit, without doing some boil the ocean
> exercise.

Yeah, I agreed that some "refactoring" is actually fixing bugs.

> I went through the issues in the email this is a reply to:
> https://lore.kernel.org/lkml/CAP-5=fXwAA0PNYAOLNLdHY8g+AyEB0UxmzgX5w-gGj_DZqUxtg@mail.gmail.com/
> Do you want to send the bits you think need prioritizing to the list?

For me, at least two things can be fixed first:

- One is the test case switch-tracking.c is broken on Arm64, your
  change for the compar() function would be fine for me.

- You mentioned the issue in ui/hist.c:
  ret = b->callchain->max_depth - a->callchain->max_depth;

These two issues are similiar and can fixed in the same format.

> > > There are lots of cases like:
> > > ```
> > >                 case ARM_SPE_COUNTER:
> > >                        if (idx == SPE_CNT_PKT_HDR_INDEX_TOTAL_LAT)
> > > -                               decoder->record.latency = payload;
> > > +                               decoder->record.latency = (u32)payload;
> >
> > This would be fine.  Since Arm SPE implements a counter with a maximum
> > of 16 bits, there will never be an overflow when storing the data in a
> > 32-bit unsigned integer.
> 
> The payload was > sizeof(u32), should record.latency be a u16?

Yes, we can change to u16 type for latency.

> My point wasn't so much to complain about adding a u32 cast in the SPE
> code, but that having the cast makes it clear that truncation is
> expected. Adding the casts isn't fixing a bug, but it is making it
> clearer that the code will truncate the payload value (imo).

As Arm SPE has defined the format, I would prefer to refine the code
based on the format:

  #define SPE_CNT_PKT_COUNT(v)    ((v) & GENMASK_ULL(15, 0))

  decoder->record.latency = SPE_CNT_PKT_COUNT(payload);

I can send a minor patch series for this, if not conflict with your
side.

Thanks,
Leo

Powered by blists - more mailing lists