mm/vmscan.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-)
From: Arnd Bergmann <arnd@arndb.de>
When the -fsanitize=bounds sanitizer is enabled, gcc-16 sometimes runs
into a corner case in the read_ctrl_pos() pos function, where it sees
possible undefined behavior from the 'tier' index overflowing, presumably
in the case that this was called with a negative tier:
In function 'get_tier_idx',
inlined from 'isolate_folios' at mm/vmscan.c:4671:14:
mm/vmscan.c: In function 'isolate_folios':
mm/vmscan.c:4645:29: error: 'pv.refaulted' is used uninitialized [-Werror=uninitialized]
Part of the problem seems to be that read_ctrl_pos() has unusual calling
conventions since commit 37a260870f2c ("mm/mglru: rework type selection")
where passing MAX_NR_TIERS makes it accumulate all tiers but passing a
smaller positive number makes it read a single tier instead.
Avoid this case by splitting read_ctrl_pos() into two separate helpers
that each only do one of the two cases. This avoids the warning as far
as I can tell, and seems a bit easier to understand to me.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is currently the only such warning I get from gcc-16.0.1, and
none from any other version.
I'm not overly happy about having to work around it with a random
code chance, but hopefully the version I ended up with makes
sense regardless.
---
mm/vmscan.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index eaf795c1cfb3..602c955d1f30 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3159,20 +3159,15 @@ struct ctrl_pos {
static void read_ctrl_pos(struct lruvec *lruvec, int type, int tier, int gain,
struct ctrl_pos *pos)
{
- int i;
struct lru_gen_folio *lrugen = &lruvec->lrugen;
int hist = lru_hist_from_seq(lrugen->min_seq[type]);
pos->gain = gain;
- pos->refaulted = pos->total = 0;
-
- for (i = tier % MAX_NR_TIERS; i <= min(tier, MAX_NR_TIERS - 1); i++) {
- pos->refaulted += lrugen->avg_refaulted[type][i] +
- atomic_long_read(&lrugen->refaulted[hist][type][i]);
- pos->total += lrugen->avg_total[type][i] +
- lrugen->protected[hist][type][i] +
- atomic_long_read(&lrugen->evicted[hist][type][i]);
- }
+ pos->refaulted = lrugen->avg_refaulted[type][tier] +
+ atomic_long_read(&lrugen->refaulted[hist][type][tier]);
+ pos->total = lrugen->avg_total[type][tier] +
+ lrugen->protected[hist][type][tier] +
+ atomic_long_read(&lrugen->evicted[hist][type][tier]);
}
static void reset_ctrl_pos(struct lruvec *lruvec, int type, bool carryover)
@@ -4640,6 +4635,24 @@ static int get_tier_idx(struct lruvec *lruvec, int type)
return tier - 1;
}
+static void aggregate_ctrl_pos(struct lruvec *lruvec, int type, int gain,
+ struct ctrl_pos *pos)
+{
+ struct lru_gen_folio *lrugen = &lruvec->lrugen;
+ int hist = lru_hist_from_seq(lrugen->min_seq[type]);
+
+ pos->gain = gain;
+ pos->refaulted = pos->total = 0;
+
+ for (int i = 0; i < MAX_NR_TIERS; i++) {
+ pos->refaulted += lrugen->avg_refaulted[type][i] +
+ atomic_long_read(&lrugen->refaulted[hist][type][i]);
+ pos->total += lrugen->avg_total[type][i] +
+ lrugen->protected[hist][type][i] +
+ atomic_long_read(&lrugen->evicted[hist][type][i]);
+ }
+}
+
static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
{
struct ctrl_pos sp, pv;
@@ -4653,8 +4666,8 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
* Compare the sum of all tiers of anon with that of file to determine
* which type to scan.
*/
- read_ctrl_pos(lruvec, LRU_GEN_ANON, MAX_NR_TIERS, swappiness, &sp);
- read_ctrl_pos(lruvec, LRU_GEN_FILE, MAX_NR_TIERS, MAX_SWAPPINESS - swappiness, &pv);
+ aggregate_ctrl_pos(lruvec, LRU_GEN_ANON, swappiness, &sp);
+ aggregate_ctrl_pos(lruvec, LRU_GEN_FILE, MAX_SWAPPINESS - swappiness, &pv);
return positive_ctrl_err(&sp, &pv);
}
--
2.39.5
On Fri, 13 Feb 2026 13:38:56 +0100 Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> When the -fsanitize=bounds sanitizer is enabled,
Is this an option in current kernels?
> gcc-16 sometimes runs
> into a corner case in the read_ctrl_pos() pos function, where it sees
> possible undefined behavior from the 'tier' index overflowing, presumably
> in the case that this was called with a negative tier:
>
> In function 'get_tier_idx',
> inlined from 'isolate_folios' at mm/vmscan.c:4671:14:
> mm/vmscan.c: In function 'isolate_folios':
> mm/vmscan.c:4645:29: error: 'pv.refaulted' is used uninitialized [-Werror=uninitialized]
>
> Part of the problem seems to be that read_ctrl_pos() has unusual calling
> conventions since commit 37a260870f2c ("mm/mglru: rework type selection")
> where passing MAX_NR_TIERS makes it accumulate all tiers but passing a
> smaller positive number makes it read a single tier instead.
>
> Avoid this case by splitting read_ctrl_pos() into two separate helpers
> that each only do one of the two cases. This avoids the warning as far
> as I can tell, and seems a bit easier to understand to me.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is currently the only such warning I get from gcc-16.0.1, and
> none from any other version.
>
> I'm not overly happy about having to work around it with a random
> code chance, but hopefully the version I ended up with makes
> sense regardless.
Seems a large change just to squish a compiler warning. People might
prefer a simple
- struct ctrl_pos sp, pv;
+ struct ctrl_pos sp, pv = {};
?
On Fri, Feb 13, 2026, at 17:58, Andrew Morton wrote:
> On Fri, 13 Feb 2026 13:38:56 +0100 Arnd Bergmann <arnd@kernel.org> wrote:
>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> When the -fsanitize=bounds sanitizer is enabled,
>
> Is this an option in current kernels?
Yes, this is CONFIG_UBSAN_ARRAY_BOUNDS. The actual warning
only shows up in some configurations with that, so either there
is some other dependency, or an element of chance based on gcc
optimizations.
> Seems a large change just to squish a compiler warning. People might
> prefer a simple
>
> - struct ctrl_pos sp, pv;
> + struct ctrl_pos sp, pv = {};
Right, that would clearly also shut up the warning.
To me this seems less intuitive without an extra comment,
since read_ctrl_pos() is meant to initialize the entire
struct, but please pick whichever you find most readable
here.
Arnd
On Fri, 13 Feb 2026 18:07:04 +0100 "Arnd Bergmann" <arnd@arndb.de> wrote:
> On Fri, Feb 13, 2026, at 17:58, Andrew Morton wrote:
> > On Fri, 13 Feb 2026 13:38:56 +0100 Arnd Bergmann <arnd@kernel.org> wrote:
> >
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> When the -fsanitize=bounds sanitizer is enabled,
> >
> > Is this an option in current kernels?
>
> Yes, this is CONFIG_UBSAN_ARRAY_BOUNDS. The actual warning
> only shows up in some configurations with that, so either there
> is some other dependency, or an element of chance based on gcc
> optimizations.
OK, I'll put a cc:stable on this, as people will want to compile older
kernels with gcc-16.
Aiming for upstreaming into 7.1-rc1 unless it's more urgent than I
think.
> > Seems a large change just to squish a compiler warning. People might
> > prefer a simple
> >
> > - struct ctrl_pos sp, pv;
> > + struct ctrl_pos sp, pv = {};
>
> Right, that would clearly also shut up the warning.
>
> To me this seems less intuitive without an extra comment,
> since read_ctrl_pos() is meant to initialize the entire
> struct, but please pick whichever you find most readable
> here.
Let's see what the MGLRU maintainers have to say.
Hi Andrew and Arnd,
On Fri, Feb 13, 2026 at 11:23 AM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Fri, 13 Feb 2026 18:07:04 +0100 "Arnd Bergmann" <arnd@arndb.de> wrote:
>
> > On Fri, Feb 13, 2026, at 17:58, Andrew Morton wrote:
> > > On Fri, 13 Feb 2026 13:38:56 +0100 Arnd Bergmann <arnd@kernel.org> wrote:
> > >
> > >> From: Arnd Bergmann <arnd@arndb.de>
> > >>
> > >> When the -fsanitize=bounds sanitizer is enabled,
> > >
> > > Is this an option in current kernels?
> >
> > Yes, this is CONFIG_UBSAN_ARRAY_BOUNDS. The actual warning
> > only shows up in some configurations with that, so either there
> > is some other dependency, or an element of chance based on gcc
> > optimizations.
>
> OK, I'll put a cc:stable on this, as people will want to compile older
> kernels with gcc-16.
>
> Aiming for upstreaming into 7.1-rc1 unless it's more urgent than I
> think.
>
> > > Seems a large change just to squish a compiler warning. People might
> > > prefer a simple
> > >
> > > - struct ctrl_pos sp, pv;
> > > + struct ctrl_pos sp, pv = {};
> >
> > Right, that would clearly also shut up the warning.
> >
> > To me this seems less intuitive without an extra comment,
> > since read_ctrl_pos() is meant to initialize the entire
> > struct, but please pick whichever you find most readable
> > here.
>
> Let's see what the MGLRU maintainers have to say.
I went over the various cases of read_ctrl_pos and couldn't find
anything wrong (let me know if I'm mistaken), so this seems like a
mild compiler bug we're trying to work around. Given it's one single
version of gcc, I'm a fan of the simple `struct ctrl_pos sp, pv = {};`
with a comment.
Thanks,
Yuanchu
On Tue, Feb 17, 2026 at 12:55 PM Yuanchu Xie <yuanchu@google.com> wrote:
>
> Hi Andrew and Arnd,
>
> On Fri, Feb 13, 2026 at 11:23 AM Andrew Morton
> <akpm@linux-foundation.org> wrote:
> >
> > On Fri, 13 Feb 2026 18:07:04 +0100 "Arnd Bergmann" <arnd@arndb.de> wrote:
> >
> > > On Fri, Feb 13, 2026, at 17:58, Andrew Morton wrote:
> > > > On Fri, 13 Feb 2026 13:38:56 +0100 Arnd Bergmann <arnd@kernel.org> wrote:
> > > >
> > > >> From: Arnd Bergmann <arnd@arndb.de>
> > > >>
> > > >> When the -fsanitize=bounds sanitizer is enabled,
> > > >
> > > > Is this an option in current kernels?
> > >
> > > Yes, this is CONFIG_UBSAN_ARRAY_BOUNDS. The actual warning
> > > only shows up in some configurations with that, so either there
> > > is some other dependency, or an element of chance based on gcc
> > > optimizations.
> >
> > OK, I'll put a cc:stable on this, as people will want to compile older
> > kernels with gcc-16.
> >
> > Aiming for upstreaming into 7.1-rc1 unless it's more urgent than I
> > think.
> >
> > > > Seems a large change just to squish a compiler warning. People might
> > > > prefer a simple
> > > >
> > > > - struct ctrl_pos sp, pv;
> > > > + struct ctrl_pos sp, pv = {};
> > >
> > > Right, that would clearly also shut up the warning.
> > >
> > > To me this seems less intuitive without an extra comment,
> > > since read_ctrl_pos() is meant to initialize the entire
> > > struct, but please pick whichever you find most readable
> > > here.
> >
> > Let's see what the MGLRU maintainers have to say.
>
> I went over the various cases of read_ctrl_pos and couldn't find
> anything wrong (let me know if I'm mistaken), so this seems like a
> mild compiler bug we're trying to work around. Given it's one single
> version of gcc, I'm a fan of the simple `struct ctrl_pos sp, pv = {};`
> with a comment.
Agreed, that simpler change with a comment seems better to me.
I don't necessarily hate the refactor to split up the single /
aggregate cases, but if we were going to do that I'd probably have
aggregate_ctrl_pos call read_ctrl_pos to avoid duplicating the code,
and I'd want to consider marking read_ctrl_pos inline.
But, it does seem like a large delta just to work around a false
positive warning. The simpler change would be easier to backport to
stable too for example. So on balance I'd prefer that.
>
> Thanks,
> Yuanchu
On Tue, 17 Feb 2026 13:22:01 -0800 Axel Rasmussen <axelrasmussen@google.com> wrote:
> > > Let's see what the MGLRU maintainers have to say.
> >
> > I went over the various cases of read_ctrl_pos and couldn't find
> > anything wrong (let me know if I'm mistaken), so this seems like a
> > mild compiler bug we're trying to work around. Given it's one single
> > version of gcc, I'm a fan of the simple `struct ctrl_pos sp, pv = {};`
> > with a comment.
>
> Agreed, that simpler change with a comment seems better to me.
>
> I don't necessarily hate the refactor to split up the single /
> aggregate cases, but if we were going to do that I'd probably have
> aggregate_ctrl_pos call read_ctrl_pos to avoid duplicating the code,
> and I'd want to consider marking read_ctrl_pos inline.
>
> But, it does seem like a large delta just to work around a false
> positive warning. The simpler change would be easier to backport to
> stable too for example. So on balance I'd prefer that.
Thanks. Did anyone send a v2? If so, I misplaced it!
© 2016 - 2026 Red Hat, Inc.