From: SeongJae Park <sj@kernel.org>
Add support of addr_unit for DAMOS_PAGEOUT action handling from the
DAMOS operation implementation for the physical address space.
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Quanmin Yan <yanquanmin1@huawei.com>
Reviewed-by: SeongJae Park <sj@kernel.org>
---
mm/damon/paddr.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index d497373c2bd2..826c2064dbfd 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -135,10 +135,11 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
return false;
}
-static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s,
+static unsigned long damon_pa_pageout(struct damon_region *r,
+ unsigned long addr_unit, struct damos *s,
unsigned long *sz_filter_passed)
{
- unsigned long addr, applied;
+ phys_addr_t addr, applied;
LIST_HEAD(folio_list);
bool install_young_filter = true;
struct damos_filter *filter;
@@ -159,8 +160,8 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s,
damos_add_filter(s, filter);
}
- addr = r->ar.start;
- while (addr < r->ar.end) {
+ addr = damon_pa_phys_addr(r->ar.start, addr_unit);
+ while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
folio = damon_get_folio(PHYS_PFN(addr));
if (damon_pa_invalid_damos_folio(folio, s)) {
addr += PAGE_SIZE;
@@ -170,7 +171,7 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s,
if (damos_pa_filter_out(s, folio))
goto put_folio;
else
- *sz_filter_passed += folio_size(folio);
+ *sz_filter_passed += folio_size(folio) / addr_unit;
folio_clear_referenced(folio);
folio_test_clear_young(folio);
@@ -189,7 +190,7 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s,
applied = reclaim_pages(&folio_list);
cond_resched();
s->last_applied = folio;
- return applied * PAGE_SIZE;
+ return applied * PAGE_SIZE / addr_unit;
}
static inline unsigned long damon_pa_mark_accessed_or_deactivate(
@@ -302,9 +303,11 @@ static unsigned long damon_pa_apply_scheme(struct damon_ctx *ctx,
struct damon_target *t, struct damon_region *r,
struct damos *scheme, unsigned long *sz_filter_passed)
{
+ unsigned long aunit = ctx->addr_unit;
+
switch (scheme->action) {
case DAMOS_PAGEOUT:
- return damon_pa_pageout(r, scheme, sz_filter_passed);
+ return damon_pa_pageout(r, aunit, scheme, sz_filter_passed);
case DAMOS_LRU_PRIO:
return damon_pa_mark_accessed(r, scheme, sz_filter_passed);
case DAMOS_LRU_DEPRIO:
--
2.43.0On Fri, 22 Aug 2025 17:34:11 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
> From: SeongJae Park <sj@kernel.org>
>
> Add support of addr_unit for DAMOS_PAGEOUT action handling from the
> DAMOS operation implementation for the physical address space.
[...]
> - return applied * PAGE_SIZE;
> + return applied * PAGE_SIZE / addr_unit;
> }
Kernel test robot [1] found this can cause __udivdi3 linking issue. Andrew,
could you please add the below attached fixup?
[1] https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
Thanks,
SJ
[...]
==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
From hackermail Thu Jan 1 00:00:00 1970
From: SeongJae Park <sj@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: SeongJae Park <sj@kernel.org>
Cc: damon@lists.linux.dev
Cc: kernel-team@meta.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org
Date: Mon, 25 Aug 2025 07:41:33 -0700
Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
return value
Otherwise, __udidi3 linking problem happens on certain configs.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/paddr.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 5fad2f9a99a0..09c87583af6c 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
return false;
}
+/* convert physical address to core-layer address */
+static unsigned long damon_pa_core_addr(phys_addr_t pa,
+ unsigned long addr_unit)
+{
+#ifdef __i386__
+ do_div(pa, addr_unit);
+ return pa;
+#else
+ return pa / addr_unit;
+#endif
+}
+
static unsigned long damon_pa_pageout(struct damon_region *r,
unsigned long addr_unit, struct damos *s,
unsigned long *sz_filter_passed)
@@ -190,7 +202,7 @@ static unsigned long damon_pa_pageout(struct damon_region *r,
applied = reclaim_pages(&folio_list);
cond_resched();
s->last_applied = folio;
- return applied * PAGE_SIZE / addr_unit;
+ return damon_pa_core_addr(applied * PAGE_SIZE, addr_unit);
}
static inline unsigned long damon_pa_mark_accessed_or_deactivate(
--
2.39.5
在 2025/8/25 23:12, SeongJae Park 写道:
> On Fri, 22 Aug 2025 17:34:11 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>
>> From: SeongJae Park <sj@kernel.org>
>>
>> Add support of addr_unit for DAMOS_PAGEOUT action handling from the
>> DAMOS operation implementation for the physical address space.
> [...]
>> - return applied * PAGE_SIZE;
>> + return applied * PAGE_SIZE / addr_unit;
>> }
> Kernel test robot [1] found this can cause __udivdi3 linking issue. Andrew,
> could you please add the below attached fixup?
>
> [1] https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
>
>
> Thanks,
> SJ
>
> [...]
>
> ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
> From hackermail Thu Jan 1 00:00:00 1970
> From: SeongJae Park <sj@kernel.org>
> To: Andrew Morton <akpm@linux-foundation.org>
> Cc: SeongJae Park <sj@kernel.org>
> Cc: damon@lists.linux.dev
> Cc: kernel-team@meta.com
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org
> Date: Mon, 25 Aug 2025 07:41:33 -0700
> Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
> return value
>
> Otherwise, __udidi3 linking problem happens on certain configs.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
> mm/damon/paddr.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 5fad2f9a99a0..09c87583af6c 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
> return false;
> }
>
> +/* convert physical address to core-layer address */
> +static unsigned long damon_pa_core_addr(phys_addr_t pa,
> + unsigned long addr_unit)
> +{
> +#ifdef __i386__
Can we use the following condition instead?
#if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
Thanks,
Quanmin Yan
> + do_div(pa, addr_unit);
> + return pa;
> +#else
> + return pa / addr_unit;
> +#endif
> +}
> +
> static unsigned long damon_pa_pageout(struct damon_region *r,
> unsigned long addr_unit, struct damos *s,
> unsigned long *sz_filter_passed)
> @@ -190,7 +202,7 @@ static unsigned long damon_pa_pageout(struct damon_region *r,
> applied = reclaim_pages(&folio_list);
> cond_resched();
> s->last_applied = folio;
> - return applied * PAGE_SIZE / addr_unit;
> + return damon_pa_core_addr(applied * PAGE_SIZE, addr_unit);
> }
>
> static inline unsigned long damon_pa_mark_accessed_or_deactivate(
On Tue, 26 Aug 2025 11:16:56 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>
> 在 2025/8/25 23:12, SeongJae Park 写道:
> > On Fri, 22 Aug 2025 17:34:11 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
[...]
> > ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
> > From hackermail Thu Jan 1 00:00:00 1970
> > From: SeongJae Park <sj@kernel.org>
> > To: Andrew Morton <akpm@linux-foundation.org>
> > Cc: SeongJae Park <sj@kernel.org>
> > Cc: damon@lists.linux.dev
> > Cc: kernel-team@meta.com
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-mm@kvack.org
> > Date: Mon, 25 Aug 2025 07:41:33 -0700
> > Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
> > return value
> >
> > Otherwise, __udidi3 linking problem happens on certain configs.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
> > Signed-off-by: SeongJae Park <sj@kernel.org>
> > ---
> > mm/damon/paddr.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 5fad2f9a99a0..09c87583af6c 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
> > return false;
> > }
> >
> > +/* convert physical address to core-layer address */
> > +static unsigned long damon_pa_core_addr(phys_addr_t pa,
> > + unsigned long addr_unit)
> > +{
> > +#ifdef __i386__
>
> Can we use the following condition instead?
>
> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
To my understanding, this issue happens only on i386, not every 32bit
architectures. So I think i386 specific condition is better.
Thanks,
SJ
[...]
Hi SJ,
在 2025/8/26 11:21, SeongJae Park 写道:
> [...]
>
>>> ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
>>> From hackermail Thu Jan 1 00:00:00 1970
>>> From: SeongJae Park <sj@kernel.org>
>>> To: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: SeongJae Park <sj@kernel.org>
>>> Cc: damon@lists.linux.dev
>>> Cc: kernel-team@meta.com
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-mm@kvack.org
>>> Date: Mon, 25 Aug 2025 07:41:33 -0700
>>> Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
>>> return value
>>>
>>> Otherwise, __udidi3 linking problem happens on certain configs.
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
>>> Signed-off-by: SeongJae Park <sj@kernel.org>
>>> ---
>>> mm/damon/paddr.c | 14 +++++++++++++-
>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
>>> index 5fad2f9a99a0..09c87583af6c 100644
>>> --- a/mm/damon/paddr.c
>>> +++ b/mm/damon/paddr.c
>>> @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
>>> return false;
>>> }
>>>
>>> +/* convert physical address to core-layer address */
>>> +static unsigned long damon_pa_core_addr(phys_addr_t pa,
>>> + unsigned long addr_unit)
>>> +{
>>> +#ifdef __i386__
>> Can we use the following condition instead?
>>
>> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
> To my understanding, this issue happens only on i386, not every 32bit
> architectures. So I think i386 specific condition is better.
I understand. However, the aforementioned general condition is essential,
and we should propose a new patch to address this. After introducing addr_unit,
any 32-bit architecture should support monitoring of 64-bit phys_addr_t.
What do you think should be our next step? Looking forward to your reply.
Thanks,
Quanmin Yan
[...]
On Tue, 26 Aug 2025 12:51:17 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
> Hi SJ,
>
> 在 2025/8/26 11:21, SeongJae Park 写道:
> > [...]
> >
> >>> ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
> >>> From hackermail Thu Jan 1 00:00:00 1970
> >>> From: SeongJae Park <sj@kernel.org>
> >>> To: Andrew Morton <akpm@linux-foundation.org>
> >>> Cc: SeongJae Park <sj@kernel.org>
> >>> Cc: damon@lists.linux.dev
> >>> Cc: kernel-team@meta.com
> >>> Cc: linux-kernel@vger.kernel.org
> >>> Cc: linux-mm@kvack.org
> >>> Date: Mon, 25 Aug 2025 07:41:33 -0700
> >>> Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
> >>> return value
> >>>
> >>> Otherwise, __udidi3 linking problem happens on certain configs.
> >>>
> >>> Reported-by: kernel test robot <lkp@intel.com>
> >>> Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
> >>> Signed-off-by: SeongJae Park <sj@kernel.org>
> >>> ---
> >>> mm/damon/paddr.c | 14 +++++++++++++-
> >>> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> >>> index 5fad2f9a99a0..09c87583af6c 100644
> >>> --- a/mm/damon/paddr.c
> >>> +++ b/mm/damon/paddr.c
> >>> @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
> >>> return false;
> >>> }
> >>>
> >>> +/* convert physical address to core-layer address */
> >>> +static unsigned long damon_pa_core_addr(phys_addr_t pa,
> >>> + unsigned long addr_unit)
> >>> +{
> >>> +#ifdef __i386__
> >> Can we use the following condition instead?
> >>
> >> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
> > To my understanding, this issue happens only on i386, not every 32bit
> > architectures. So I think i386 specific condition is better.
>
> I understand. However, the aforementioned general condition is essential,
> and we should propose a new patch to address this. After introducing addr_unit,
> any 32-bit architecture should support monitoring of 64-bit phys_addr_t.
The issue is that we cannot divide 64bit values with plain '/' operator on
"i386", and hence this patch makes it to use do_div() instead of '/' on "i386".
No such or other problems at supporting 64-bit phys_addr_t is found on other
32bit architectures, to my understanding. My understanding is that at least
you confirmed that on your arm-based test environment. So we don't need a new
patch to my understanding.
Am I missing somthing?
Thanks,
SJ
[...]
在 2025/8/26 22:21, SeongJae Park 写道:
> On Tue, 26 Aug 2025 12:51:17 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>
>> Hi SJ,
>>
>> 在 2025/8/26 11:21, SeongJae Park 写道:
>>> [...]
>>>
>>>>> ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
>>>>> From hackermail Thu Jan 1 00:00:00 1970
>>>>> From: SeongJae Park <sj@kernel.org>
>>>>> To: Andrew Morton <akpm@linux-foundation.org>
>>>>> Cc: SeongJae Park <sj@kernel.org>
>>>>> Cc: damon@lists.linux.dev
>>>>> Cc: kernel-team@meta.com
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: linux-mm@kvack.org
>>>>> Date: Mon, 25 Aug 2025 07:41:33 -0700
>>>>> Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
>>>>> return value
>>>>>
>>>>> Otherwise, __udidi3 linking problem happens on certain configs.
>>>>>
>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
>>>>> Signed-off-by: SeongJae Park <sj@kernel.org>
>>>>> ---
>>>>> mm/damon/paddr.c | 14 +++++++++++++-
>>>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
>>>>> index 5fad2f9a99a0..09c87583af6c 100644
>>>>> --- a/mm/damon/paddr.c
>>>>> +++ b/mm/damon/paddr.c
>>>>> @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
>>>>> return false;
>>>>> }
>>>>>
>>>>> +/* convert physical address to core-layer address */
>>>>> +static unsigned long damon_pa_core_addr(phys_addr_t pa,
>>>>> + unsigned long addr_unit)
>>>>> +{
>>>>> +#ifdef __i386__
>>>> Can we use the following condition instead?
>>>>
>>>> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
>>> To my understanding, this issue happens only on i386, not every 32bit
>>> architectures. So I think i386 specific condition is better.
>> I understand. However, the aforementioned general condition is essential,
>> and we should propose a new patch to address this. After introducing addr_unit,
>> any 32-bit architecture should support monitoring of 64-bit phys_addr_t.
> The issue is that we cannot divide 64bit values with plain '/' operator on
> "i386", and hence this patch makes it to use do_div() instead of '/' on "i386".
> No such or other problems at supporting 64-bit phys_addr_t is found on other
> 32bit architectures, to my understanding. My understanding is that at least
> you confirmed that on your arm-based test environment. So we don't need a new
> patch to my understanding.
>
> Am I missing somthing?
This is because I seem to have made a mistake earlier: I adjusted the local
compilation toolchain. When the __udivdi3 issue mentioned above occurred, it
reminded me of a potential problem. After switching to a completely new environment
for testing, I discovered the __aeabi_uldivmod linking error in arm, which is similar
to the __udivdi3 issue.🙁 To prevent similar environment-related problems in the
future, I suggest adjusting the condition to the following:
#if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
Please consider approving this fix.
Best regards,
Quanmin Yan
[...]
On Wed, 27 Aug 2025 10:21:41 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>
> 在 2025/8/26 22:21, SeongJae Park 写道:
> > On Tue, 26 Aug 2025 12:51:17 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
> >
> >> Hi SJ,
> >>
> >> 在 2025/8/26 11:21, SeongJae Park 写道:
> >>> [...]
> >>>
> >>>>> ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
> >>>>> From hackermail Thu Jan 1 00:00:00 1970
> >>>>> From: SeongJae Park <sj@kernel.org>
> >>>>> To: Andrew Morton <akpm@linux-foundation.org>
> >>>>> Cc: SeongJae Park <sj@kernel.org>
> >>>>> Cc: damon@lists.linux.dev
> >>>>> Cc: kernel-team@meta.com
> >>>>> Cc: linux-kernel@vger.kernel.org
> >>>>> Cc: linux-mm@kvack.org
> >>>>> Date: Mon, 25 Aug 2025 07:41:33 -0700
> >>>>> Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
> >>>>> return value
> >>>>>
> >>>>> Otherwise, __udidi3 linking problem happens on certain configs.
> >>>>>
> >>>>> Reported-by: kernel test robot <lkp@intel.com>
> >>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
> >>>>> Signed-off-by: SeongJae Park <sj@kernel.org>
> >>>>> ---
> >>>>> mm/damon/paddr.c | 14 +++++++++++++-
> >>>>> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> >>>>> index 5fad2f9a99a0..09c87583af6c 100644
> >>>>> --- a/mm/damon/paddr.c
> >>>>> +++ b/mm/damon/paddr.c
> >>>>> @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
> >>>>> return false;
> >>>>> }
> >>>>>
> >>>>> +/* convert physical address to core-layer address */
> >>>>> +static unsigned long damon_pa_core_addr(phys_addr_t pa,
> >>>>> + unsigned long addr_unit)
> >>>>> +{
> >>>>> +#ifdef __i386__
> >>>> Can we use the following condition instead?
> >>>>
> >>>> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
> >>> To my understanding, this issue happens only on i386, not every 32bit
> >>> architectures. So I think i386 specific condition is better.
> >> I understand. However, the aforementioned general condition is essential,
> >> and we should propose a new patch to address this. After introducing addr_unit,
> >> any 32-bit architecture should support monitoring of 64-bit phys_addr_t.
> > The issue is that we cannot divide 64bit values with plain '/' operator on
> > "i386", and hence this patch makes it to use do_div() instead of '/' on "i386".
> > No such or other problems at supporting 64-bit phys_addr_t is found on other
> > 32bit architectures, to my understanding. My understanding is that at least
> > you confirmed that on your arm-based test environment. So we don't need a new
> > patch to my understanding.
> >
> > Am I missing somthing?
>
> This is because I seem to have made a mistake earlier: I adjusted the local
> compilation toolchain. When the __udivdi3 issue mentioned above occurred, it
> reminded me of a potential problem. After switching to a completely new environment
> for testing, I discovered the __aeabi_uldivmod linking error in arm, which is similar
> to the __udivdi3 issue.🙁
Thank you for sharing this. Then I agree the current fixup is insufficient.
Andrew, could you please drop this patch series from mm tree for now? I will
further discuss with Quanmin about the proper fix and post next version of this
series with the proper fixup.
> To prevent similar environment-related problems in the
> future, I suggest adjusting the condition to the following:
>
> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
>
> Please consider approving this fix.
I'm yet in travel, and I'd like to take more time on thinking about the proper
fix. Quanmin, could you share more details about your test setups, both for
the compiling success case and failing case?
Thanks,
SJ
[...]
Hi SJ,
在 2025/8/27 10:42, SeongJae Park 写道:
> On Wed, 27 Aug 2025 10:21:41 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>
>> 在 2025/8/26 22:21, SeongJae Park 写道:
>>> On Tue, 26 Aug 2025 12:51:17 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>>>
>>>> Hi SJ,
>>>>
>>>> 在 2025/8/26 11:21, SeongJae Park 写道:
>>>>> [...]
>>>>>
>>>>>>> ==== Attachment 0 (0001-mm-damon-paddr-use-do_div-on-i386-for-damon_pa_pageo.patch) ====
>>>>>>> From hackermail Thu Jan 1 00:00:00 1970
>>>>>>> From: SeongJae Park <sj@kernel.org>
>>>>>>> To: Andrew Morton <akpm@linux-foundation.org>
>>>>>>> Cc: SeongJae Park <sj@kernel.org>
>>>>>>> Cc: damon@lists.linux.dev
>>>>>>> Cc: kernel-team@meta.com
>>>>>>> Cc: linux-kernel@vger.kernel.org
>>>>>>> Cc: linux-mm@kvack.org
>>>>>>> Date: Mon, 25 Aug 2025 07:41:33 -0700
>>>>>>> Subject: [PATCH 1/3] mm/damon/paddr: use do_div() on i386 for damon_pa_pageout()
>>>>>>> return value
>>>>>>>
>>>>>>> Otherwise, __udidi3 linking problem happens on certain configs.
>>>>>>>
>>>>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202508241831.EKwdwXZL-lkp@intel.com/
>>>>>>> Signed-off-by: SeongJae Park <sj@kernel.org>
>>>>>>> ---
>>>>>>> mm/damon/paddr.c | 14 +++++++++++++-
>>>>>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
>>>>>>> index 5fad2f9a99a0..09c87583af6c 100644
>>>>>>> --- a/mm/damon/paddr.c
>>>>>>> +++ b/mm/damon/paddr.c
>>>>>>> @@ -135,6 +135,18 @@ static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
>>>>>>> return false;
>>>>>>> }
>>>>>>>
>>>>>>> +/* convert physical address to core-layer address */
>>>>>>> +static unsigned long damon_pa_core_addr(phys_addr_t pa,
>>>>>>> + unsigned long addr_unit)
>>>>>>> +{
>>>>>>> +#ifdef __i386__
>>>>>> Can we use the following condition instead?
>>>>>>
>>>>>> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
>>>>> To my understanding, this issue happens only on i386, not every 32bit
>>>>> architectures. So I think i386 specific condition is better.
>>>> I understand. However, the aforementioned general condition is essential,
>>>> and we should propose a new patch to address this. After introducing addr_unit,
>>>> any 32-bit architecture should support monitoring of 64-bit phys_addr_t.
>>> The issue is that we cannot divide 64bit values with plain '/' operator on
>>> "i386", and hence this patch makes it to use do_div() instead of '/' on "i386".
>>> No such or other problems at supporting 64-bit phys_addr_t is found on other
>>> 32bit architectures, to my understanding. My understanding is that at least
>>> you confirmed that on your arm-based test environment. So we don't need a new
>>> patch to my understanding.
>>>
>>> Am I missing somthing?
>> This is because I seem to have made a mistake earlier: I adjusted the local
>> compilation toolchain. When the __udivdi3 issue mentioned above occurred, it
>> reminded me of a potential problem. After switching to a completely new environment
>> for testing, I discovered the __aeabi_uldivmod linking error in arm, which is similar
>> to the __udivdi3 issue.🙁
> Thank you for sharing this. Then I agree the current fixup is insufficient.
>
> Andrew, could you please drop this patch series from mm tree for now? I will
> further discuss with Quanmin about the proper fix and post next version of this
> series with the proper fixup.
>
>> To prevent similar environment-related problems in the
>> future, I suggest adjusting the condition to the following:
>>
>> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
>>
>> Please consider approving this fix.
> I'm yet in travel, and I'd like to take more time on thinking about the proper
> fix. Quanmin, could you share more details about your test setups, both for
> the compiling success case and failing case?
Apologies for disturbing you during your travels. Please allow me to explain:
When CONFIG_PHYS_ADDR_T_64BIT is enabled on "i386" [1], the phys_addr_t type
becomes 64-bit, requiring the use of the do_div function. We are in agreement
on this point.
On arm32, if LPAE (which we intend to support in this series) is enabled,
CONFIG_PHYS_ADDR_T_64BIT will also be enabled. In this case, pa / addr_unit
will involve 64-bit division and similarly require the do_div function.
Obviously, such link errors should normally occur under these circumstances.
Unfortunately, the expected anomalies did not manifest in my previous tests.
This may be related to some incorrect adjustments I had made to my local build
environment quite some time ago — though I cannot be entirely certain. That
said, I have since cleaned up the old configurations and ensured the current
environment is clean and normal. For now, we have confirmed the actual problem
and its root cause, shall we focus on fixing it?
In summary, after introducing addr_unit, we expect that any 32-bit architecture
should support monitoring of 64-bit phys_addr_t. Therefore, we can consider the
following adjustment:
#if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
Or at least adjust it to:
#if defined(__i386__) || (defined(__arm__) && defined(CONFIG_PHYS_ADDR_T_64BIT))
I have thoroughly re-validated the feature functionality today and confirmed the
correctness of the aforementioned modifications. Therefore, could I kindly ask
you to consider the aforementioned modifications when you have some free time?
[1]https://download.01.org/0day-ci/archive/20250824/202508241831.EKwdwXZL-lkp@intel.com/config
Thanks,
Quanmin Yan
[...]
On Wed, 27 Aug 2025 19:37:38 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
> Hi SJ,
>
> 在 2025/8/27 10:42, SeongJae Park 写道:
> > On Wed, 27 Aug 2025 10:21:41 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
> >
> >> 在 2025/8/26 22:21, SeongJae Park 写道:
> >>> On Tue, 26 Aug 2025 12:51:17 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
> >>>
> >>>> Hi SJ,
> >>>>
> >>>> 在 2025/8/26 11:21, SeongJae Park 写道:
[...]
> >> Please consider approving this fix.
> > I'm yet in travel, and I'd like to take more time on thinking about the proper
> > fix. Quanmin, could you share more details about your test setups, both for
> > the compiling success case and failing case?
>
> Apologies for disturbing you during your travels. Please allow me to explain:
No worry, I'm the one who would like to apologize, for delayed response :)
I'm back from the travel, btw.
>
> When CONFIG_PHYS_ADDR_T_64BIT is enabled on "i386" [1], the phys_addr_t type
> becomes 64-bit, requiring the use of the do_div function. We are in agreement
> on this point.
>
> On arm32, if LPAE (which we intend to support in this series) is enabled,
> CONFIG_PHYS_ADDR_T_64BIT will also be enabled. In this case, pa / addr_unit
> will involve 64-bit division and similarly require the do_div function.
> Obviously, such link errors should normally occur under these circumstances.
> Unfortunately, the expected anomalies did not manifest in my previous tests.
> This may be related to some incorrect adjustments I had made to my local build
> environment quite some time ago — though I cannot be entirely certain. That
> said, I have since cleaned up the old configurations and ensured the current
> environment is clean and normal. For now, we have confirmed the actual problem
> and its root cause, shall we focus on fixing it?
Thank you for sharing the details. I wanted to better understand where the
issue happens and not, to clearly understand the root cause and make a proper
fix based on that. I think we can now focusing on fixing it.
>
> In summary, after introducing addr_unit, we expect that any 32-bit architecture
> should support monitoring of 64-bit phys_addr_t. Therefore, we can consider the
> following adjustment:
>
> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
>
> Or at least adjust it to:
>
> #if defined(__i386__) || (defined(__arm__) && defined(CONFIG_PHYS_ADDR_T_64BIT))
>
> I have thoroughly re-validated the feature functionality today and confirmed the
> correctness of the aforementioned modifications. Therefore, could I kindly ask
> you to consider the aforementioned modifications when you have some free time?
Thank you for the suggestion and testing, Quanmin!
I was thinking making the change for only i386 is right since I was mistakenly
thinking the issue happens only on i386. Now it is clear I was wrong and we
have at least two cases. And I agree your suggestion will fix both cases.
But I'm bit concerned i386 and arm might not all the case, so wannt make the
fix more generalized. My understanding of the problem, which is enlightened
thanks to you, is that not every config supports dividing 64 bit with 32 bit.
And div_u64() is suggested in general for dividing 64 bit with 32 bit. So,
what about making the if condition more general but specific to the root cause,
like below?
static unsigned long damon_pa_core_addr(
phys_addr_t pa, unsigned long addr_unit)
{
/*
* Use div_u64() for avoiding linking errors related with __udivdi3,
* __aeabi_uldivmod, or similar problems. This should also improve the
* performance optimization (read div_u64() comment for the detail).
*/
if (sizeof(pa) == 8 && sizeof(addr_unit) == 4)
return div_u64(pa, addr_unit);
return pa / addr_unit;
}
Because the sizeof() result can be known at compile time, I think it shouldn't
cause the linking time error, and I confirmed that by passing the i386 test
case that kernel test robot shared.
Could I ask your opinion, Quanmin? If you think that works, I could post v3 of
this patch series with the above fix.
Thanks,
SJ
[...]
在 2025/8/28 2:07, SeongJae Park 写道:
> On Wed, 27 Aug 2025 19:37:38 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>
>> Hi SJ,
>>
>> 在 2025/8/27 10:42, SeongJae Park 写道:
>>> On Wed, 27 Aug 2025 10:21:41 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>>>
>>>> 在 2025/8/26 22:21, SeongJae Park 写道:
>>>>> On Tue, 26 Aug 2025 12:51:17 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
>>>>>
>>>>>> Hi SJ,
>>>>>>
>>>>>> 在 2025/8/26 11:21, SeongJae Park 写道:
> [...]
>>>> Please consider approving this fix.
>>> I'm yet in travel, and I'd like to take more time on thinking about the proper
>>> fix. Quanmin, could you share more details about your test setups, both for
>>> the compiling success case and failing case?
>> Apologies for disturbing you during your travels. Please allow me to explain:
> No worry, I'm the one who would like to apologize, for delayed response :)
> I'm back from the travel, btw.
>
>> When CONFIG_PHYS_ADDR_T_64BIT is enabled on "i386" [1], the phys_addr_t type
>> becomes 64-bit, requiring the use of the do_div function. We are in agreement
>> on this point.
>>
>> On arm32, if LPAE (which we intend to support in this series) is enabled,
>> CONFIG_PHYS_ADDR_T_64BIT will also be enabled. In this case, pa / addr_unit
>> will involve 64-bit division and similarly require the do_div function.
>> Obviously, such link errors should normally occur under these circumstances.
>> Unfortunately, the expected anomalies did not manifest in my previous tests.
>> This may be related to some incorrect adjustments I had made to my local build
>> environment quite some time ago — though I cannot be entirely certain. That
>> said, I have since cleaned up the old configurations and ensured the current
>> environment is clean and normal. For now, we have confirmed the actual problem
>> and its root cause, shall we focus on fixing it?
> Thank you for sharing the details. I wanted to better understand where the
> issue happens and not, to clearly understand the root cause and make a proper
> fix based on that. I think we can now focusing on fixing it.
>
>> In summary, after introducing addr_unit, we expect that any 32-bit architecture
>> should support monitoring of 64-bit phys_addr_t. Therefore, we can consider the
>> following adjustment:
>>
>> #if !defined(CONFIG_64BIT) && defined(CONFIG_PHYS_ADDR_T_64BIT)
>>
>> Or at least adjust it to:
>>
>> #if defined(__i386__) || (defined(__arm__) && defined(CONFIG_PHYS_ADDR_T_64BIT))
>>
>> I have thoroughly re-validated the feature functionality today and confirmed the
>> correctness of the aforementioned modifications. Therefore, could I kindly ask
>> you to consider the aforementioned modifications when you have some free time?
> Thank you for the suggestion and testing, Quanmin!
>
> I was thinking making the change for only i386 is right since I was mistakenly
> thinking the issue happens only on i386. Now it is clear I was wrong and we
> have at least two cases. And I agree your suggestion will fix both cases.
>
> But I'm bit concerned i386 and arm might not all the case, so wannt make the
> fix more generalized. My understanding of the problem, which is enlightened
> thanks to you, is that not every config supports dividing 64 bit with 32 bit.
> And div_u64() is suggested in general for dividing 64 bit with 32 bit. So,
> what about making the if condition more general but specific to the root cause,
> like below?
>
> static unsigned long damon_pa_core_addr(
> phys_addr_t pa, unsigned long addr_unit)
> {
> /*
> * Use div_u64() for avoiding linking errors related with __udivdi3,
> * __aeabi_uldivmod, or similar problems. This should also improve the
> * performance optimization (read div_u64() comment for the detail).
> */
> if (sizeof(pa) == 8 && sizeof(addr_unit) == 4)
> return div_u64(pa, addr_unit);
> return pa / addr_unit;
> }
>
> Because the sizeof() result can be known at compile time, I think it shouldn't
> cause the linking time error, and I confirmed that by passing the i386 test
> case that kernel test robot shared.
>
> Could I ask your opinion, Quanmin? If you think that works, I could post v3 of
> this patch series with the above fix.
Great! I believe this approach is better. This modification is more generic
and eliminates the need to deal with those messy configs.
I have also re-validated based on this change, and it is working correctly.
Thanks,
Quanmin Yan
On Thu, 28 Aug 2025 09:38:34 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote: [...] > Great! I believe this approach is better. This modification is more generic > and eliminates the need to deal with those messy configs. > > I have also re-validated based on this change, and it is working correctly. Thank you for reviewing the code and further testing it! I will post v3 with this fix soon, probably tomorrow morning in Pacific Time, unless someone make a different opinion or I find something I missed by then. Thanks, SJ [...]
© 2016 - 2025 Red Hat, Inc.