Re: [RFC PATCH 3/3] mm/mglru: move lrugen atomic stats off the read-mostly lines

[Ridong Chen <ridong.chen@linux.dev>]

On 11/8/2026 5:14 PM, Barry Song wrote:
> On Thu, Nov 5, 2026 at 9:23 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> lru_gen_eviction() and lru_gen_refault() update lrugen->evicted[] and
>> lrugen->refaulted[] locklessly with atomic_long_add(). The arrays sit at
>> the tail of struct lru_gen_folio, right before enabled/gen/seg/list:
>> with LRU_GEN_STATS=n the file type counters share a cacheline with them.
> 

Hi Barry,

Thanks for the review. Note that the sizes in the changelog were
measured with lockdep enabled.

Offsets below are from pahole on x86_64, LRU_GEN_STATS=n, counted
from the start of struct mem_cgroup_per_node (cacheline aligned), so
line N = bytes N*64 .. N*64+63.

The nesting is:
     mem_cgroup_per_node
       lruvec                at 128
         lrugen              at 128 + 176 = 304  (before the patch)
                             at 128 + 192 = 320  (after the patch)

> So basically, this is causing false sharing, right?
> 

Yes. Before the patch:

field                    offset       line
refaulted[0][file][*]    1608-1639    25    atomic, no lru_lock
enabled/gen/seg          1640-1642    25    read on every LRU add
list                     1648-1663    25    read by shrink_many()

line 25 (1600-1663)
+------------------------+-----------------+------+------------+
| refaulted[0][file][*]  | enabled,gen,seg | pad  | list       |
| 1608-1639              | 1640-1642       |      | 1648-1663  |
+------------------------+-----------------+------+------------+

CPU A (file refault)		CPU B (LRU add)
atomic_long_add(&refaulted[..])
				read lrugen->enabled -> miss
atomic_long_add(&refaulted[..])
				read lrugen->enabled -> miss

CPU B doesn't need refaulted[], but it keeps missing on line 25.

After the patch:

field                    offset       line
max_seq/min_seq/ts       320-375      5
enabled/gen/seg          376-378      5     read-mostly
list                     384-399      6
nr_pages[]               1040-1359    16-21 under lru_lock
evicted[]                1600-1663    25    own line
refaulted[]              1664-1727    26    own line

> I'd prefer a text diagram showing how the false sharing
> occurs, specifically how freq read and freq write end up
> in the same cacheline.
> 
> This will help clarify why rearranging the struct is the
> right approach and how to rearrange it properly.
> 
>>
>> Those fields are read far more often than they are written:
>> lru_gen_add_folio() reads ->enabled on every LRU add, and shrink_many()
>> walks ->list and reads ->gen/->seg under RCU. With frequent file refaults
>> from several CPUs, every refault invalidates that line on the CPUs doing
>> LRU adds.
>>
>> Move enabled/gen/seg/list next to max_seq/min_seq/timestamps, which are
>> also mostly read, and put evicted[]/refaulted[] in their own cacheline
>> group at the end of the struct. This grows struct lru_gen_folio from
>> 1360 to 1408 bytes and struct mem_cgroup_per_node from 2176 to 2240
> 
> Why is the difference exactly 2240 - 2176 = 64 bytes,
> which is one cacheline? I was expecting a value smaller
> than one cacheline :-)
> 

The aligned group makes lru_gen_folio cacheline aligned, and the
padding adds up on the way out:

lru_gen_folio         1360 -> 1408  (+48, hole before evicted[])
lruvec                1624 -> 1728  (+104, lrugen moves from offset
				     176 to 192, plus tail padding to 64)
mem_cgroup_per_node   2048 -> 2112  (+64)

mem_cgroup_per_node is cacheline aligned, so its size only changes
in whole lines. It had 40 bytes of padding after lruvec, which
absorbs part of the 104 bytes, and the rest takes one more line.


> What I'm not convinced about is that nr_pages should be
> updated even more frequently than evicted and refaulted.
> Why aren't they causing a false sharing?
> 

nr_pages[] (lines 16-21) is read and written only under lru_lock.
Its lines move between CPUs only when the lock does, and the new
lock holder needs the updated values anyway, so that is true
sharing. refaulted[] is written by CPUs that don't hold the lock,
so it can steal the line from the lock holder at any time, and the
lock holder doesn't even need it.

> long nr_pages[MAX_NR_GENS][ANON_AND_FILE][MAX_NR_ZONES];
> 
> It would be better to have a text diagram showing the
> potential false sharing, so we can consider the issue
> more comprehensively.
> 
>> bytes on x86_64 (one more cacheline per memcg per node).
>>
>> Tested on an i7-13700HX (24 threads) with turbo disabled and the
>> performance governor. 24 files of 128MiB on ext4 on brd, read with
>> fio (psync, randread, bs=4k, fadvise_hint=random, buffered), all jobs
>> in one memcg with memory.max=1G, so nearly every read refaults.
>> IOPS, median of 5 runs:
>>
>>    jobs      base   patched
>>       1    270537    274303   +1.4%  (sd 3.7%)
>>       8   1461025   1466293   +0.4%
>>      16   2455629   2508001   +2.1%
>>      24   2707720   2823776   +4.3%
>>
>> A custom random pread() test of the same setup, with this patch alone
>> against base over two boots of each kernel, shows +2.4% at 16 threads
>> and +5.9% at 24 threads, and no change at 8 threads or fewer. With
>> anonymous memory on zram, the refaults mostly hit the tier 0 counters
>> on a different line, and there is no measurable change.
>>
>> The gain grows with the number of CPUs refaulting into the same lruvec
>> and needs a fast backing store; with real IO it is likely to be lost in
>> the noise. The patch is mostly meant to keep the lockless counters away
>> from the read-mostly fields regardless of where they end up.
>>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   include/linux/mmzone.h | 18 ++++++++++--------
>>   1 file changed, 10 insertions(+), 8 deletions(-)
>>
> 
> Thanks,
> Barry

-- 
Best regards
Ridong