Message-ID: <75e5f32d-a824-44de-b9b0-a2d58dc2c45b@huawei.com>
Date: Thu, 10 Apr 2025 14:28:23 +0800
From: zuoze <zuoze1@...wei.com>
To: SeongJae Park <sj@...nel.org>
CC: <akpm@...ux-foundation.org>, <damon@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	<wangkefeng.wang@...wei.com>
Subject: Re: [RFC PATCH] mm/damon: add full LPAE support for memory monitoring
 above 4GB



在 2025/4/10 1:36, SeongJae Park 写道:
> On Wed, 9 Apr 2025 15:01:39 +0800 zuoze <zuoze1@...wei.com> wrote:
> 
>>
>>
>> 在 2025/4/9 10:50, SeongJae Park 写道:
>>> Hi Ze,
>>>
>>> On Tue, 8 Apr 2025 15:55:53 +0800 Ze Zuo <zuoze1@...wei.com> 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!
>>
>> Thank you for the kind words!
>>
>>>
>>>>
>>>> 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
> [...]
>>>
>>> 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?
>>>
>>
>> Thank you for your feedback. I agree this isn’t a simple change, and
>> the current approach might not be optimal. Let’s work together to find
>> the best solution.
>>
>> As for the prevalence of 32-bit ARM machines with LPAE, these devices
>> are still widely used in our product application. The main goal for
>> enabling DAMON on these devices is to optimize memory usage. During
>> usage, we identified this optimization point, as well as overflow issues
>> with bytes* and other reclaim statistics interfaces.
>>
>> These devices are still in frequent use, and given their large installed
>> base, they are unlikely to be replaced in the short term.
> 
> Thank you for kindly sharing these!  And I agree the devices could still be
> actively used and wouldn't be replaced in the short term.  I believe making
> DAMON supports those devices is really important.
>
> DAMON aims to support multiple address spaces that not limited to only virtual
> address spaces and physical address space but any imaginable case.  I hence
> prefer keeping DAMON core layer independent of the underlying hardware as much
> as possible.  I therefore still hope to continue using 'unsigned long'.
> 
> Of course, 'unsigned long' wouldn't fit all cases.  32-bit ARM with LPAE is a
> great real example, and there might be crazy future use case.  In other words,
> this could be identified as an issue of the operations set layer, which
> directly deals with the given address space.
> 
> I think another approach for this issue is adding a DAMON parameter, say,
> address_unit.  It will represent the factor value that need to be multiplied to
> DAMON-address to get real address on the given address space.  For example, if
> address_unit is 10 and the user sets DAMON monitoring target address range as
> 200-300, it means user wants DAMON to monitor address range 2000-3000 of the
> given address space.  The default value of the parameter would be 1, so that
> old users show no change.  32bit ARM with LPAE users would need to explicitly
> set the parameter but I believe that shouldn't be a big issue?

Regarding the address_unit approach, I have some concerns after checking
the code:

1. Scaling Factor Limitations - While the scaling factor resolves the
damon_addr_range storage issue, actual physical addresses (PAs) would
still require unsigned long long temporaries in many cases.  Different
system with varying iomem regions may require different scaling
factors, making deployment harder than a fixed maximum range.

2. Significant Code Impact & Overhead - Implementing this would require
significant changes with every PA traversal needing rescaling, which
introduces computational overhead. That said, there remains a necessity
to use unsigned long long to store certain variables in structures, such
as sampling_addr in the damon_region structure and sz_tried in the
damos_stat structure.

If I'm misunderstanding any points, please correct me, and feel free to
add any additional concerns.

As an alternative, we could adopt a pattern similar to other subsystems
(e.g., memblock, CMA, resource), which define their own address types.
For example:

  #ifdef CONFIG_PHYS_ADDR_T_64BIT
	typedef unsigned long long damon_addr_t;
  #else
	typedef unsigned long damon_addr_t;
  #endif

This approach would avoid scaling complexity while maintaining
consistency with existing mm code.

What do you think? SJ, I'm happy to help test any approach or discuss
further.

> 
> What do you think about the rough idea, Ze?  If you don't mind, I could
> implement the idea on my own and get your review/tests, or help you
> implementing it on your own.  I don't care who implements it but eagerly want
> to make DAMON supports the real use case well.
> 
> 
> Thanks,
> SJ
> 
> [...]
> 

Powered by blists - more mailing lists