[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250510112419.749652a7@pumpkin>
Date: Sat, 10 May 2025 11:24:19 +0100
From: David Laight <david.laight.linux@...il.com>
To: WangYuli <wangyuli@...ontech.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, yuzhao@...gle.com, stevensd@...omium.org,
kaleshsingh@...gle.com, zhanjun@...ontech.com, niecheng1@...ontech.com,
guanwentao@...ontech.com, Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH] mm: vmscan: Avoid signedness error for GCC 5.4
On Wed, 7 May 2025 12:06:11 +0800
WangYuli <wangyuli@...ontech.com> wrote:
> Hi Andrew Morton,
>
> On 2025/5/7 07:24, Andrew Morton wrote:
> > On Wed, 7 May 2025 00:02:38 +0800 WangYuli <wangyuli@...ontech.com> wrote:
> >
> > Make `tier' unsigned? After all, negative tier numbers are nonsensical.
>
> That point is well taken.
>
> However, I've noticed that variables named "tier" seem to be commonly
> defined as int rather than unsigned int throughout the mm subsystem, and
> perhaps even the wider kernel code.
>
> I was wondering if changing just this one instance might feel a little
> inconsistent?
>
> Perhaps a possible approach for now could be to change this line to for
> (i = tier % MAX_NR_TIERS; i <= min_t(int, tier, MAX_NR_TIERS - 1); i++)
> {, which would allow us to keep the signed int type for the tier
> variable itself.
Remember that min_t(int, a, b) is just min((int)a, (int)b) and you really
wouldn't put casts like that in code.
Even if a cast can't be avoided only one side would normally need it.
There really ought to be a 'duck shoot' against min_t().
I'm trying very hard to stop any more being added.
David
>
> Regarding the potential for a more comprehensive change in the future to
> redefine all these "tier" variables and related ones as unsigned int, I
> would be very grateful for your guidance on whether that's a direction
> we should consider.
>
> But actually, whether it's signed or not likely won't affect its normal
> operation...
>
> Thanks,
>
Powered by blists - more mailing lists