Re: [PATCH 2/3] drm/xe/gt_stats: Use atomic64_t for counters

[Matthew Auld <matthew.auld@intel.com>]

On 25/02/2025 13:17, Francois Dugast wrote:
> The stats counters are now used for things like counting the VMA
> bytes during page faults. During workload execution, the counter
> value can grow fast and easily reach the atomic int limit, in
> which case it overflows. To make this less likely to happen, push
> the limit by switching to 64b atomic to store the counter value.
> Overhead is very small as there are only 3 stat entries per GT as
> of now, and stats are only enabled with CONFIG_DEBUG_FS.
> 
> Suggested-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
 > --->   drivers/gpu/drm/xe/xe_gt_stats.c | 10 +++++-----
>   drivers/gpu/drm/xe/xe_gt_types.h |  2 +-
>   2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c
> index 753c081b68a6..83de2ec92fdc 100644
> --- a/drivers/gpu/drm/xe/xe_gt_stats.c
> +++ b/drivers/gpu/drm/xe/xe_gt_stats.c
> @@ -30,11 +30,11 @@ void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr)

Do we also bump the incr? Maybe s64?

Either way,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

>   	if (id >= __XE_GT_STATS_NUM_IDS)
>   		return;
>   
> -	atomic_add(incr, &gt->stats.counters[id]);
> +	atomic64_add(incr, &gt->stats.counters[id]);
>   
> -	if (atomic_read(&gt->stats.counters[id]) < 0) {
> +	if (atomic64_read(&gt->stats.counters[id]) < 0) {
>   		xe_gt_dbg(gt, "stats %s overflow, resetting\n", stat_description[id]);
> -		atomic_set(&gt->stats.counters[id], incr);
> +		atomic64_set(&gt->stats.counters[id], incr);
>   	}
>   }
>   
> @@ -50,8 +50,8 @@ int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p)
>   	enum xe_gt_stats_id id;
>   
>   	for (id = 0; id < __XE_GT_STATS_NUM_IDS; ++id)
> -		drm_printf(p, "%s: %d\n", stat_description[id],
> -			   atomic_read(&gt->stats.counters[id]));
> +		drm_printf(p, "%s: %lld\n", stat_description[id],
> +			   atomic64_read(&gt->stats.counters[id]));
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
> index 6e66bf0e8b3f..f72b965cc9e6 100644
> --- a/drivers/gpu/drm/xe/xe_gt_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
> @@ -139,7 +139,7 @@ struct xe_gt {
>   	/** @stats: GT stats */
>   	struct {
>   		/** @stats.counters: counters for various GT stats */
> -		atomic_t counters[__XE_GT_STATS_NUM_IDS];
> +		atomic64_t counters[__XE_GT_STATS_NUM_IDS];
>   	} stats;
>   #endif
>