RE: [PATCH 2/2] x86/numa: Fix the sort compare func used in numa_fill_memblks()

[Dan Williams <dan.j.williams@intel.com>]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The compare function used to sort memblks into starting address
> order fails when the result of its u64 address subtraction gets
> truncated to an int upon return.
> 
> The impact of the bad sort is that memblks will be filled out
> incorrectly. Depending on the set of memblks, a user may see no
> errors at all but still have a bad fill, or see messages reporting
> a node overlap that leads to numa init failure:
> 
> [] node 0 [mem: ] overlaps with node 1 [mem: ]
> [] No NUMA configuration found
> 
> Replace with a comparison that can only result in: 1, 0, -1.

Good eye!

> Fixes: 8f012db27c95 ("x86/numa: Introduce numa_fill_memblks()")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/mm/numa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 8ada9bbfad58..65e9a6e391c0 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -934,7 +934,7 @@ static int __init cmp_memblk(const void *a, const void *b)
>  	const struct numa_memblk *ma = *(const struct numa_memblk **)a;
>  	const struct numa_memblk *mb = *(const struct numa_memblk **)b;
>  
> -	return ma->start - mb->start;
> +	return (ma->start > mb->start) - (ma->start < mb->start);

Maybe just do the less clever but obviously correct thing like 
other unsigned compares like ktime_compare():

    if (ma->start > mb->start)
        return 1;
    if (ma->start < mb->start)
        return -1;
    return 0;

...but otherwise this looks good to me.