Hi Ze,
On Tue, 8 Apr 2025 15:55:53 +0800 Ze Zuo <zuoze1@xxxxxxxxxx> wrote:
Previously, DAMON's physical address space monitoring only supported
memory ranges below 4GB on LPAE-enabled systems. This was due to
the use of 'unsigned long' in 'struct damon_addr_range', which is
32-bit on ARM32 even with LPAE enabled.
Nice finding!
This patch modifies the data types to properly support >4GB physical
address spaces:
1. Changes 'start' and 'end' in 'struct damon_addr_range' from
'unsigned long' to 'unsigned long long' (u64)
2. Updates all related arithmetic operations and comparisons
3. Adjusts print formats from %lu to %llu where needed
4. Maintains backward compatibility for non-LPAE systems
Since the overhead of always using u64 is negligible on 32-bit systems,
should we prefer this simplified approach over the conditional typedef?
Alternative implementation approaches to consider:
1. Introduce damon_addr_t that adapts to CONFIG_PHYS_ADDR_T_64BIT
2. Convert all DAMON address operations to use this type
Just like implementation:
#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef unsigned long long damon_addr_t;
#else
typedef unsigned long damon_addr_t;
#endif
This method could potentially cause minor issues with print formatting
and division operations. We'd appreciate any suggestions for better
approaches. Thank you for your input.
The patch change allows DAMON to work with:
- 32-bit ARM with LPAE (40-bit physical addresses)
- 64-bit ARM systems
- x86_64 physical address monitoring
while preserving existing behavior on 32-bit systems without LPAE.
Again, nice finding and good improvement. Thank you so much for sharing this
nice patch.
But, this doesn't seem like a very small and simple change. I think we can
find the best approach together, by understanding impact of this change for
short term and long term. For that, could you please also share how prevalent
32-bit ARM machines with LPAE are, and what would be your expected usage of
physical address space monitoring on such machines, in both short term and long
term?
Thanks,
SJ
[...]