Re: [PATCH v9] mm/damon: add node_eligible_mem_bp goal metric

[Ravi Jonnalagadda]

On Sun, Apr 26, 2026 at 4:03 PM SeongJae Park <sj@xxxxxxxxxx> wrote:
>
> [...]
> > Changes since v8:
> > =================
> > https://lore.kernel.org/linux-mm/20260426003245.2687-1-ravis.opensrc@xxxxxxxxx/
> >
> > - Removed unnecessary casts in mult_frac() call to fix potential
> >   divide-by-zero on 32-bit PAE systems (Sashiko/SJ review)
> > - Kept original parameter name 'ctx' in damos_set_effective_quota()
> >   to avoid unnecessary diff (SJ review)
> [...]
> > +static unsigned long damos_get_node_eligible_mem_bp(struct damon_ctx *c,
> > +             struct damos *s, int nid)
> > +{
> > +     phys_addr_t total_eligible = 0;
> > +     phys_addr_t node_eligible;
> > +
> > +     if (c->ops.id != DAMON_OPS_PADDR)
> > +             return 0;
> > +
> > +     if (nid < 0 || nid >= MAX_NUMNODES || !node_online(nid))
> > +             return 0;
> > +
> > +     node_eligible = damos_calc_eligible_bytes(c, s, nid, &total_eligible);
> > +
> > +     if (!total_eligible)
> > +             return 0;
> > +
> > +     return mult_frac(node_eligible, 10000, total_eligible);
> > +}
>
> Sashiko found [1] this can cause __udivdi3 linking issue on 32bit machine with
> CONFIG_PHYS_ADDR_T_64BIT.  Seems that is correct to me.  I believe below
> attaching fixup can fix it?
>
> Ravi, if my fixup looks good to you, could you please post another version
> (say, v9.1) for another Sashiko review round?
>

Makes sense SJ. will send v9.1 shortly with the fixup.

> [1] https://lore.kernel.org/20260426220409.95B26C2BCAF@xxxxxxxxxxxxxxx
>
>
> Thanks,
> SJ
>
> [...]
>
> === >8 ===
> From 828ddfdfa3a996efc4620919a3cc77d55088c466 Mon Sep 17 00:00:00 2001
> From: SeongJae Park <sj@xxxxxxxxxx>
> Date: Sun, 26 Apr 2026 15:54:48 -0700
> Subject: [PATCH] mm/damon/core: avoid __udivdi3 linking issue
>
> On 32bit machines having PHYS_ADDR_T_64BIT, mult_frac() with phys_addr_t
> will cause __udivdi3 linking issue.  Cast values to 'unsigned long'
> before calling mult_frac().  This could cause divide-by-zero if the
> denominator becomes zero by casting.  Avoid it by checking the zero case
> with the casted value.
>
> Signed-off-by: SeongJae Park <sj@xxxxxxxxxx>
> ---
>  mm/damon/core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0ea747d487465..aec43314fc4a9 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2921,10 +2921,11 @@ static unsigned long damos_get_node_eligible_mem_bp(struct damon_ctx *c,
>
>         node_eligible = damos_calc_eligible_bytes(c, s, nid, &total_eligible);
>
> -       if (!total_eligible)
> +       if (!(unsigned long)total_eligible)
>                 return 0;
>
> -       return mult_frac(node_eligible, 10000, total_eligible);
> +       return mult_frac((unsigned long)node_eligible, 10000,
> +                       (unsigned long)total_eligible);
>  }
>  #else /* CONFIG_DAMON_PADDR */
>  static unsigned long damos_get_node_eligible_mem_bp(struct damon_ctx *c,
> --
> 2.47.3
>