Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `defrag_mb_moved` counter loses precision and will significantly undercount the amount of data moved. --
--- Patch [21]: [PATCH v3 21/33] drm/xe: Add defrag GT stats --- commit 84ae24372375a8f7d60dca398232ca340eba4dd8 Author: Matthew Brost <[email protected]> Subject: drm/xe: Add defrag GT stats This commit introduces observability for the page defragmentation worker by adding GT statistics counters. It tracks counts for buffer objects added to the defrag list, successful defragmentations, partial successes, failures, and the amount of data moved in megabytes. > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index 907cfa57481f4..073d926ef9ace 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c [ ... ] > @@ -1274,8 +1277,21 @@ static int xe_bo_defrag_one(struct xe_device *xe, > struct xe_bo *bo, > xe_dbg(xe, "Defrag attempt on BO size=%zu: ret=%pe consumed=%llu\n", > xe_bo_size(bo), ERR_PTR(ret), *consumed); > > - if (!ret && ttm_tt_is_beneficial_order_failed(bo->ttm.ttm)) > + if (!ret && ttm_tt_is_beneficial_order_failed(bo->ttm.ttm)) { > *needs_more = true; > + xe_gt_stats_incr(xe_root_mmio_gt(xe), > + XE_GT_STATS_ID_DEFRAG_PARTIAL_SUCCESS_COUNT, > + 1); > + } else { > + xe_gt_stats_incr(xe_root_mmio_gt(xe), > + ret ? XE_GT_STATS_ID_DEFRAG_FAILED_COUNT : > + XE_GT_STATS_ID_DEFRAG_SUCCESS_COUNT, 1); > + } > + > + if (!ret) > + xe_gt_stats_incr(xe_root_mmio_gt(xe), > + XE_GT_STATS_ID_DEFRAG_MB_MOVED, > + *consumed >> 20); [Severity: Medium] Does this right-shift truncate the accumulation of data moved? If *consumed is less than 1 MiB (for example, a 64 KiB chunk), *consumed >> 20 evaluates to 0, meaning it will not be counted at all. Could this lead to significantly undercounting the actual data moved? Would it be better to accumulate raw bytes in the counter and only convert to megabytes when formatting the output for userspace? > > unlock: > xe_bo_unlock(bo); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=21
