Re: [PATCH v2 1/2] mm/slub: Fix cmp_loc_by_count() to return 0 when counts are equal

[Harry Yoo <harry.yoo@oracle.com>]

On Tue, Aug 26, 2025 at 02:23:14PM +0800, Kuan-Wei Chiu wrote:
> The comparison function cmp_loc_by_count() used for sorting stack trace
> locations in debugfs currently returns -1 if a->count > b->count and 1
> otherwise. This breaks the antisymmetry property required by sort(),
> because when two counts are equal, both cmp(a, b) and cmp(b, a) return
> 1.
> 
> This can lead to undefined or incorrect ordering results. Fix it by
> updating the comparison logic to explicitly handle the case when counts
> are equal, and use cmp_int() to ensure the comparison function adheres
> to the required mathematical properties of antisymmetry.
> 
> Fixes: 553c0369b3e1 ("mm/slub: sort debugfs output by frequency of stack traces")
> Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> ---

While the author withdrew the claim that it definitely leads to incorrect
results, it remains true that the API requires both transitivity and
antisymmetry for correctness, so:

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

-- 
Cheers,
Harry / Hyeonggon

>  mm/slub.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 30003763d224..081816ff89ab 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -7716,10 +7716,7 @@ static int cmp_loc_by_count(const void *a, const void *b, const void *data)
>  	struct location *loc1 = (struct location *)a;
>  	struct location *loc2 = (struct location *)b;
>  
> -	if (loc1->count > loc2->count)
> -		return -1;
> -	else
> -		return 1;
> +	return cmp_int(loc2->count, loc1->count);
>  }
>  
>  static void *slab_debugfs_start(struct seq_file *seq, loff_t *ppos)
> -- 
> 2.34.1