* [PATCH v5 0/2] Enhance SVM stats @ 2025-08-27 18:43 Matthew Brost 2025-08-27 18:43 ` [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs Matthew Brost 2025-08-27 18:43 ` [PATCH v5 2/2] drm/xe: Add more SVM GT stats Matthew Brost 0 siblings, 2 replies; 7+ messages in thread From: Matthew Brost @ 2025-08-27 18:43 UTC (permalink / raw) To: intel-xe Add ability to clear GT stats and add more SVM stats for profiling. Scripts should be able to be built on top of stats to breakdown time spent in parts of the SVM page fault handler, bandwidth of copy engine, etc... Will help showing concrete performance improvements when implementing SVM optimizations. Open suggestions of more stats. Prefetch not considered - can be done in a follow up. Scritpt to parse stats for reference: https://pastebin.com/rZZN5sgh v2: - Include local changes missed in first rev v3: - More stats after putting together script to parse v4: - Rebase v5: - Rebase - Drop clear_stats entry Matthew Brost (2): drm/xe: Add clearing stats to GT debugfs drm/xe: Add more SVM GT stats drivers/gpu/drm/xe/xe_gt_debugfs.c | 25 +++- drivers/gpu/drm/xe/xe_gt_stats.c | 47 ++++++ drivers/gpu/drm/xe/xe_gt_stats.h | 1 + drivers/gpu/drm/xe/xe_gt_stats_types.h | 33 +++++ drivers/gpu/drm/xe/xe_svm.c | 196 +++++++++++++++++++++++-- 5 files changed, 292 insertions(+), 10 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs 2025-08-27 18:43 [PATCH v5 0/2] Enhance SVM stats Matthew Brost @ 2025-08-27 18:43 ` Matthew Brost 2025-08-27 19:02 ` Michal Wajdeczko 2025-08-27 18:43 ` [PATCH v5 2/2] drm/xe: Add more SVM GT stats Matthew Brost 1 sibling, 1 reply; 7+ messages in thread From: Matthew Brost @ 2025-08-27 18:43 UTC (permalink / raw) To: intel-xe It helpful to clear GT stats, run a test cases which is being profiled, and look at the results of the stats from the individual test case. Make stats entry writable and upon write clear the stats. v5: - Drop clear_stats debugfs entry (Lucas) Signed-off-by: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/xe/xe_gt_debugfs.c | 25 ++++++++++++++++++++++++- drivers/gpu/drm/xe/xe_gt_stats.c | 14 ++++++++++++++ drivers/gpu/drm/xe/xe_gt_stats.h | 1 + 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c index bf3a67b5951c..0004f279dee1 100644 --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c @@ -327,7 +327,6 @@ static const struct drm_info_list vf_safe_debugfs_list[] = { {"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data = bcs_default_lrc}, {"default_lrc_vcs", .show = xe_gt_debugfs_simple_show, .data = vcs_default_lrc}, {"default_lrc_vecs", .show = xe_gt_debugfs_simple_show, .data = vecs_default_lrc}, - {"stats", .show = xe_gt_debugfs_simple_show, .data = xe_gt_stats_print_info}, {"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig}, }; @@ -363,6 +362,29 @@ static ssize_t write_to_gt_call(const char __user *userbuf, size_t count, loff_t return count; } +static void clear_stats(struct xe_gt *gt) +{ + xe_gt_stats_clear(gt); +} + +static ssize_t stats_write(struct file *file, const char __user *userbuf, + size_t count, loff_t *ppos) +{ + struct seq_file *s = file->private_data; + struct xe_gt *gt = s->private; + + return write_to_gt_call(userbuf, count, ppos, clear_stats, gt); +} + +static int stats_show(struct seq_file *s, void *unused) +{ + struct drm_printer p = drm_seq_file_printer(s); + struct xe_gt *gt = s->private; + + return xe_gt_stats_print_info(gt, &p); +} +DEFINE_SHOW_STORE_ATTRIBUTE(stats); + static void force_reset(struct xe_gt *gt) { struct xe_device *xe = gt_to_xe(gt); @@ -448,6 +470,7 @@ void xe_gt_debugfs_register(struct xe_gt *gt) root->d_inode->i_private = gt; /* VF safe */ + debugfs_create_file("stats", 0600, root, gt, &stats_fops); debugfs_create_file("force_reset", 0600, root, gt, &force_reset_fops); debugfs_create_file("force_reset_sync", 0600, root, gt, &force_reset_sync_fops); diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c index 30f942671c2b..7e12fc3759e2 100644 --- a/drivers/gpu/drm/xe/xe_gt_stats.c +++ b/drivers/gpu/drm/xe/xe_gt_stats.c @@ -50,3 +50,17 @@ int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p) return 0; } + +/** + * xe_gt_stats_clear - Clear the GT stats + * @gt: GT structure + * + * This clear (zeros) all the available GT stats. + */ +void xe_gt_stats_clear(struct xe_gt *gt) +{ + enum xe_gt_stats_id id; + + for (id = 0; id < __XE_GT_STATS_NUM_IDS; ++id) + atomic64_set(>->stats.counters[id], 0); +} diff --git a/drivers/gpu/drm/xe/xe_gt_stats.h b/drivers/gpu/drm/xe/xe_gt_stats.h index 38325ef53617..e8aea32bc971 100644 --- a/drivers/gpu/drm/xe/xe_gt_stats.h +++ b/drivers/gpu/drm/xe/xe_gt_stats.h @@ -13,6 +13,7 @@ struct drm_printer; #ifdef CONFIG_DEBUG_FS int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p); +void xe_gt_stats_clear(struct xe_gt *gt); void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr); #else static inline void -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs 2025-08-27 18:43 ` [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs Matthew Brost @ 2025-08-27 19:02 ` Michal Wajdeczko 2025-08-27 19:14 ` Matthew Brost 0 siblings, 1 reply; 7+ messages in thread From: Michal Wajdeczko @ 2025-08-27 19:02 UTC (permalink / raw) To: Matthew Brost, intel-xe On 8/27/2025 8:43 PM, Matthew Brost wrote: > It helpful to clear GT stats, run a test cases which is being profiled, > and look at the results of the stats from the individual test case. Make > stats entry writable and upon write clear the stats. > > v5: > - Drop clear_stats debugfs entry (Lucas) > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/xe/xe_gt_debugfs.c | 25 ++++++++++++++++++++++++- > drivers/gpu/drm/xe/xe_gt_stats.c | 14 ++++++++++++++ > drivers/gpu/drm/xe/xe_gt_stats.h | 1 + > 3 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c > index bf3a67b5951c..0004f279dee1 100644 > --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c > +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c > @@ -327,7 +327,6 @@ static const struct drm_info_list vf_safe_debugfs_list[] = { > {"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data = bcs_default_lrc}, > {"default_lrc_vcs", .show = xe_gt_debugfs_simple_show, .data = vcs_default_lrc}, > {"default_lrc_vecs", .show = xe_gt_debugfs_simple_show, .data = vecs_default_lrc}, > - {"stats", .show = xe_gt_debugfs_simple_show, .data = xe_gt_stats_print_info}, > {"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig}, > }; > > @@ -363,6 +362,29 @@ static ssize_t write_to_gt_call(const char __user *userbuf, size_t count, loff_t > return count; > } > > +static void clear_stats(struct xe_gt *gt) > +{ > + xe_gt_stats_clear(gt); > +} not needed - see below > + > +static ssize_t stats_write(struct file *file, const char __user *userbuf, > + size_t count, loff_t *ppos) > +{ > + struct seq_file *s = file->private_data; > + struct xe_gt *gt = s->private; > + > + return write_to_gt_call(userbuf, count, ppos, clear_stats, gt); why not pass the xe_gt_stats_clear() directly here? it's the same signature > +} > + > +static int stats_show(struct seq_file *s, void *unused) > +{ > + struct drm_printer p = drm_seq_file_printer(s); > + struct xe_gt *gt = s->private; > + > + return xe_gt_stats_print_info(gt, &p); > +} > +DEFINE_SHOW_STORE_ATTRIBUTE(stats); > + > static void force_reset(struct xe_gt *gt) > { > struct xe_device *xe = gt_to_xe(gt); > @@ -448,6 +470,7 @@ void xe_gt_debugfs_register(struct xe_gt *gt) > root->d_inode->i_private = gt; > > /* VF safe */ > + debugfs_create_file("stats", 0600, root, gt, &stats_fops); > debugfs_create_file("force_reset", 0600, root, gt, &force_reset_fops); > debugfs_create_file("force_reset_sync", 0600, root, gt, &force_reset_sync_fops); > > diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c > index 30f942671c2b..7e12fc3759e2 100644 > --- a/drivers/gpu/drm/xe/xe_gt_stats.c > +++ b/drivers/gpu/drm/xe/xe_gt_stats.c > @@ -50,3 +50,17 @@ int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p) > > return 0; > } > + > +/** > + * xe_gt_stats_clear - Clear the GT stats > + * @gt: GT structure > + * > + * This clear (zeros) all the available GT stats. > + */ > +void xe_gt_stats_clear(struct xe_gt *gt) > +{ > + enum xe_gt_stats_id id; I would just use "int" as we don't really use enum here > + > + for (id = 0; id < __XE_GT_STATS_NUM_IDS; ++id) or even maybe we can just go from 0 to ARRAY_SIZE(gt->stats.counters) ? > + atomic64_set(>->stats.counters[id], 0); > +} > diff --git a/drivers/gpu/drm/xe/xe_gt_stats.h b/drivers/gpu/drm/xe/xe_gt_stats.h > index 38325ef53617..e8aea32bc971 100644 > --- a/drivers/gpu/drm/xe/xe_gt_stats.h > +++ b/drivers/gpu/drm/xe/xe_gt_stats.h > @@ -13,6 +13,7 @@ struct drm_printer; > > #ifdef CONFIG_DEBUG_FS > int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p); > +void xe_gt_stats_clear(struct xe_gt *gt); > void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr); > #else > static inline void ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs 2025-08-27 19:02 ` Michal Wajdeczko @ 2025-08-27 19:14 ` Matthew Brost 0 siblings, 0 replies; 7+ messages in thread From: Matthew Brost @ 2025-08-27 19:14 UTC (permalink / raw) To: Michal Wajdeczko; +Cc: intel-xe On Wed, Aug 27, 2025 at 09:02:26PM +0200, Michal Wajdeczko wrote: > > > On 8/27/2025 8:43 PM, Matthew Brost wrote: > > It helpful to clear GT stats, run a test cases which is being profiled, > > and look at the results of the stats from the individual test case. Make > > stats entry writable and upon write clear the stats. > > > > v5: > > - Drop clear_stats debugfs entry (Lucas) > > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > --- > > drivers/gpu/drm/xe/xe_gt_debugfs.c | 25 ++++++++++++++++++++++++- > > drivers/gpu/drm/xe/xe_gt_stats.c | 14 ++++++++++++++ > > drivers/gpu/drm/xe/xe_gt_stats.h | 1 + > > 3 files changed, 39 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c > > index bf3a67b5951c..0004f279dee1 100644 > > --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c > > +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c > > @@ -327,7 +327,6 @@ static const struct drm_info_list vf_safe_debugfs_list[] = { > > {"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data = bcs_default_lrc}, > > {"default_lrc_vcs", .show = xe_gt_debugfs_simple_show, .data = vcs_default_lrc}, > > {"default_lrc_vecs", .show = xe_gt_debugfs_simple_show, .data = vecs_default_lrc}, > > - {"stats", .show = xe_gt_debugfs_simple_show, .data = xe_gt_stats_print_info}, > > {"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig}, > > }; > > > > @@ -363,6 +362,29 @@ static ssize_t write_to_gt_call(const char __user *userbuf, size_t count, loff_t > > return count; > > } > > > > +static void clear_stats(struct xe_gt *gt) > > +{ > > + xe_gt_stats_clear(gt); > > +} > > not needed - see below > > > + > > +static ssize_t stats_write(struct file *file, const char __user *userbuf, > > + size_t count, loff_t *ppos) > > +{ > > + struct seq_file *s = file->private_data; > > + struct xe_gt *gt = s->private; > > + > > + return write_to_gt_call(userbuf, count, ppos, clear_stats, gt); > > why not pass the xe_gt_stats_clear() directly here? it's the same signature Yes, let me drop this. > > > +} > > + > > +static int stats_show(struct seq_file *s, void *unused) > > +{ > > + struct drm_printer p = drm_seq_file_printer(s); > > + struct xe_gt *gt = s->private; > > + > > + return xe_gt_stats_print_info(gt, &p); > > +} > > +DEFINE_SHOW_STORE_ATTRIBUTE(stats); > > + > > static void force_reset(struct xe_gt *gt) > > { > > struct xe_device *xe = gt_to_xe(gt); > > @@ -448,6 +470,7 @@ void xe_gt_debugfs_register(struct xe_gt *gt) > > root->d_inode->i_private = gt; > > > > /* VF safe */ > > + debugfs_create_file("stats", 0600, root, gt, &stats_fops); > > debugfs_create_file("force_reset", 0600, root, gt, &force_reset_fops); > > debugfs_create_file("force_reset_sync", 0600, root, gt, &force_reset_sync_fops); > > > > diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c > > index 30f942671c2b..7e12fc3759e2 100644 > > --- a/drivers/gpu/drm/xe/xe_gt_stats.c > > +++ b/drivers/gpu/drm/xe/xe_gt_stats.c > > @@ -50,3 +50,17 @@ int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p) > > > > return 0; > > } > > + > > +/** > > + * xe_gt_stats_clear - Clear the GT stats > > + * @gt: GT structure > > + * > > + * This clear (zeros) all the available GT stats. > > + */ > > +void xe_gt_stats_clear(struct xe_gt *gt) > > +{ > > + enum xe_gt_stats_id id; > > I would just use "int" as we don't really use enum here > > > + > > + for (id = 0; id < __XE_GT_STATS_NUM_IDS; ++id) > > or even maybe we can just go from 0 to ARRAY_SIZE(gt->stats.counters) ? > Can do both of these suggestions. Matt > > + atomic64_set(>->stats.counters[id], 0); > > +} > > diff --git a/drivers/gpu/drm/xe/xe_gt_stats.h b/drivers/gpu/drm/xe/xe_gt_stats.h > > index 38325ef53617..e8aea32bc971 100644 > > --- a/drivers/gpu/drm/xe/xe_gt_stats.h > > +++ b/drivers/gpu/drm/xe/xe_gt_stats.h > > @@ -13,6 +13,7 @@ struct drm_printer; > > > > #ifdef CONFIG_DEBUG_FS > > int xe_gt_stats_print_info(struct xe_gt *gt, struct drm_printer *p); > > +void xe_gt_stats_clear(struct xe_gt *gt); > > void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr); > > #else > > static inline void > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] drm/xe: Add more SVM GT stats 2025-08-27 18:43 [PATCH v5 0/2] Enhance SVM stats Matthew Brost 2025-08-27 18:43 ` [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs Matthew Brost @ 2025-08-27 18:43 ` Matthew Brost 2025-08-27 19:15 ` Michal Wajdeczko 1 sibling, 1 reply; 7+ messages in thread From: Matthew Brost @ 2025-08-27 18:43 UTC (permalink / raw) To: intel-xe Add more SVM GT stats which give visibility to where time is spent in the SVM page fault handler. Stats include number of faults at a given size, total SVM page fault time, migration time in us, copy time in us, copy kb, get pages time in us, and bind time in us. Will help in tuning SVM for performance. v2: - Include local changes v3: - Add tlb invalidation + valid page fault + per size copy size stats v4: - Ensure gt not NULL when incrementing SVM copy stats - Normalize stats names - Use magic macros to generate increment functions for ranges Signed-off-by: Matthew Brost <matthew.brost@intel.com> Reviewed-by: Francois Dugast <francois.dugast@intel.com> --- drivers/gpu/drm/xe/xe_gt_stats.c | 33 +++++ drivers/gpu/drm/xe/xe_gt_stats_types.h | 33 +++++ drivers/gpu/drm/xe/xe_svm.c | 196 +++++++++++++++++++++++-- 3 files changed, 253 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c index 7e12fc3759e2..2422577e4dee 100644 --- a/drivers/gpu/drm/xe/xe_gt_stats.c +++ b/drivers/gpu/drm/xe/xe_gt_stats.c @@ -29,8 +29,41 @@ void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr) static const char *const stat_description[__XE_GT_STATS_NUM_IDS] = { "svm_pagefault_count", "tlb_inval_count", + "svm_tlb_inval_count", + "svm_tlb_inval_us", "vma_pagefault_count", "vma_pagefault_kb", + "svm_4K_pagefault_count", + "svm_64K_pagefault_count", + "svm_2M_pagefault_count", + "svm_4K_valid_pagefault_count", + "svm_64K_valid_pagefault_count", + "svm_2M_valid_pagefault_count", + "svm_4K_pagefault_us", + "svm_64K_pagefault_us", + "svm_2M_pagefault_us", + "svm_4K_migrate_count", + "svm_64K_migrate_count", + "svm_2M_migrate_count", + "svm_4K_migrate_us", + "svm_64K_migrate_us", + "svm_2M_migrate_us", + "svm_device_copy_us", + "svm_4K_device_copy_us", + "svm_64K_device_copy_us", + "svm_2M_device_copy_us", + "svm_cpu_copy_us", + "svm_4K_cpu_copy_us", + "svm_64K_cpu_copy_us", + "svm_2M_cpu_copy_us", + "svm_device_copy_kb", + "svm_cpu_copy_kb", + "svm_4K_get_pages_us", + "svm_64K_get_pages_us", + "svm_2M_get_pages_us", + "svm_4K_bind_us", + "svm_64K_bind_us", + "svm_2M_bind_us", }; /** diff --git a/drivers/gpu/drm/xe/xe_gt_stats_types.h b/drivers/gpu/drm/xe/xe_gt_stats_types.h index be3244d7133c..d8348a8de2e1 100644 --- a/drivers/gpu/drm/xe/xe_gt_stats_types.h +++ b/drivers/gpu/drm/xe/xe_gt_stats_types.h @@ -9,8 +9,41 @@ enum xe_gt_stats_id { XE_GT_STATS_ID_SVM_PAGEFAULT_COUNT, XE_GT_STATS_ID_TLB_INVAL, + XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT, + XE_GT_STATS_ID_SVM_TLB_INVAL_US, XE_GT_STATS_ID_VMA_PAGEFAULT_COUNT, XE_GT_STATS_ID_VMA_PAGEFAULT_KB, + XE_GT_STATS_ID_SVM_4K_PAGEFAULT_COUNT, + XE_GT_STATS_ID_SVM_64K_PAGEFAULT_COUNT, + XE_GT_STATS_ID_SVM_2M_PAGEFAULT_COUNT, + XE_GT_STATS_ID_SVM_4K_VALID_PAGEFAULT_COUNT, + XE_GT_STATS_ID_SVM_64K_VALID_PAGEFAULT_COUNT, + XE_GT_STATS_ID_SVM_2M_VALID_PAGEFAULT_COUNT, + XE_GT_STATS_ID_SVM_4K_PAGEFAULT_US, + XE_GT_STATS_ID_SVM_64K_PAGEFAULT_US, + XE_GT_STATS_ID_SVM_2M_PAGEFAULT_US, + XE_GT_STATS_ID_SVM_4K_MIGRATE_COUNT, + XE_GT_STATS_ID_SVM_64K_MIGRATE_COUNT, + XE_GT_STATS_ID_SVM_2M_MIGRATE_COUNT, + XE_GT_STATS_ID_SVM_4K_MIGRATE_US, + XE_GT_STATS_ID_SVM_64K_MIGRATE_US, + XE_GT_STATS_ID_SVM_2M_MIGRATE_US, + XE_GT_STATS_ID_SVM_DEVICE_COPY_US, + XE_GT_STATS_ID_SVM_4K_DEVICE_COPY_US, + XE_GT_STATS_ID_SVM_64K_DEVICE_COPY_US, + XE_GT_STATS_ID_SVM_2M_DEVICE_COPY_US, + XE_GT_STATS_ID_SVM_CPU_COPY_US, + XE_GT_STATS_ID_SVM_4K_CPU_COPY_US, + XE_GT_STATS_ID_SVM_64K_CPU_COPY_US, + XE_GT_STATS_ID_SVM_2M_CPU_COPY_US, + XE_GT_STATS_ID_SVM_DEVICE_COPY_KB, + XE_GT_STATS_ID_SVM_CPU_COPY_KB, + XE_GT_STATS_ID_SVM_4K_GET_PAGES_US, + XE_GT_STATS_ID_SVM_64K_GET_PAGES_US, + XE_GT_STATS_ID_SVM_2M_GET_PAGES_US, + XE_GT_STATS_ID_SVM_4K_BIND_US, + XE_GT_STATS_ID_SVM_64K_BIND_US, + XE_GT_STATS_ID_SVM_2M_BIND_US, /* must be the last entry */ __XE_GT_STATS_NUM_IDS, }; diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c index 0596039ef0a1..7ab63e6b77b0 100644 --- a/drivers/gpu/drm/xe/xe_svm.c +++ b/drivers/gpu/drm/xe/xe_svm.c @@ -6,6 +6,7 @@ #include <drm/drm_drv.h> #include "xe_bo.h" +#include "xe_exec_queue_types.h" #include "xe_gt_stats.h" #include "xe_gt_tlb_invalidation.h" #include "xe_migrate.h" @@ -113,6 +114,11 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, struct xe_svm_range *range, &vm->svm.garbage_collector.work); } +static void xe_svm_tlb_inval_count_stats_incr(struct xe_gt *gt) +{ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT, 1); +} + static u8 xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r, const struct mmu_notifier_range *mmu_range, @@ -145,13 +151,19 @@ xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r, */ for_each_tile(tile, xe, id) if (xe_pt_zap_ptes_range(tile, vm, range)) { - tile_mask |= BIT(id); /* * WRITE_ONCE pairs with READ_ONCE in * xe_vm_has_valid_gpu_mapping() */ WRITE_ONCE(range->tile_invalidated, range->tile_invalidated | BIT(id)); + + if (!(tile_mask & BIT(id))) { + xe_svm_tlb_inval_count_stats_incr(tile->primary_gt); + if (tile->media_gt) + xe_svm_tlb_inval_count_stats_incr(tile->media_gt); + tile_mask |= BIT(id); + } } return tile_mask; @@ -171,6 +183,24 @@ xe_svm_range_notifier_event_end(struct xe_vm *vm, struct drm_gpusvm_range *r, mmu_range); } +static s64 xe_svm_stats_ktime_us_delta(ktime_t start) +{ + return IS_ENABLED(CONFIG_DEBUG_FS) ? + ktime_us_delta(ktime_get(), start) : 0; +} + +static void xe_svm_tlb_inval_us_stats_incr(struct xe_gt *gt, ktime_t start) +{ + s64 us_delta = xe_svm_stats_ktime_us_delta(start); + + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_TLB_INVAL_US, us_delta); +} + +static ktime_t xe_svm_stats_ktime_get(void) +{ + return IS_ENABLED(CONFIG_DEBUG_FS) ? ktime_get() : 0; +} + static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, struct drm_gpusvm_notifier *notifier, const struct mmu_notifier_range *mmu_range) @@ -178,8 +208,10 @@ static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, struct xe_vm *vm = gpusvm_to_vm(gpusvm); struct xe_device *xe = vm->xe; struct drm_gpusvm_range *r, *first; + struct xe_tile *tile; + ktime_t start = xe_svm_stats_ktime_get(); u64 adj_start = mmu_range->start, adj_end = mmu_range->end; - u8 tile_mask = 0; + u8 tile_mask = 0, id; long err; xe_svm_assert_in_notifier(vm); @@ -232,6 +264,13 @@ static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, r = first; drm_gpusvm_for_each_range(r, notifier, adj_start, adj_end) xe_svm_range_notifier_event_end(vm, r, mmu_range); + for_each_tile(tile, xe, id) { + if (tile_mask & BIT(id)) { + xe_svm_tlb_inval_us_stats_incr(tile->primary_gt, start); + if (tile->media_gt) + xe_svm_tlb_inval_us_stats_incr(tile->media_gt, start); + } + } } static int __xe_svm_garbage_collector(struct xe_vm *vm, @@ -385,11 +424,66 @@ enum xe_svm_copy_dir { XE_SVM_COPY_TO_SRAM, }; +static void xe_svm_copy_kb_stats_incr(struct xe_gt *gt, + const enum xe_svm_copy_dir dir, + int kb) +{ + if (dir == XE_SVM_COPY_TO_VRAM) + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_DEVICE_COPY_KB, kb); + else + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_CPU_COPY_KB, kb); +} + +static void xe_svm_copy_us_stats_incr(struct xe_gt *gt, + const enum xe_svm_copy_dir dir, + unsigned long npages, + ktime_t start) +{ + s64 us_delta = xe_svm_stats_ktime_us_delta(start); + + if (dir == XE_SVM_COPY_TO_VRAM) { + switch (npages) { + case 1: + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_DEVICE_COPY_US, + us_delta); + break; + case 16: + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_DEVICE_COPY_US, + us_delta); + break; + case 512: + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_DEVICE_COPY_US, + us_delta); + break; + } + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_DEVICE_COPY_US, + us_delta); + } else { + switch (npages) { + case 1: + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_CPU_COPY_US, + us_delta); + break; + case 16: + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_CPU_COPY_US, + us_delta); + break; + case 512: + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_CPU_COPY_US, + us_delta); + break; + } + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_CPU_COPY_US, + us_delta); + } +} + static int xe_svm_copy(struct page **pages, struct drm_pagemap_addr *pagemap_addr, unsigned long npages, const enum xe_svm_copy_dir dir) { struct xe_vram_region *vr = NULL; + struct xe_gt *gt = NULL; struct xe_device *xe; struct dma_fence *fence = NULL; unsigned long i; @@ -397,6 +491,7 @@ static int xe_svm_copy(struct page **pages, u64 vram_addr = XE_VRAM_ADDR_INVALID; int err = 0, pos = 0; bool sram = dir == XE_SVM_COPY_TO_SRAM; + ktime_t start = xe_svm_stats_ktime_get(); /* * This flow is complex: it locates physically contiguous device pages, @@ -423,6 +518,7 @@ static int xe_svm_copy(struct page **pages, if (!vr && spage) { vr = page_to_vr(spage); + gt = xe_migrate_exec_queue(vr->migrate)->gt; xe = vr->xe; } XE_WARN_ON(spage && page_to_vr(spage) != vr); @@ -462,6 +558,9 @@ static int xe_svm_copy(struct page **pages, int incr = (match && last) ? 1 : 0; if (vram_addr != XE_VRAM_ADDR_INVALID) { + xe_svm_copy_kb_stats_incr(gt, dir, + (i - pos + incr) * + (PAGE_SIZE / SZ_1K)); if (sram) { vm_dbg(&xe->drm, "COPY TO SRAM - 0x%016llx -> 0x%016llx, NPAGES=%ld", @@ -500,6 +599,8 @@ static int xe_svm_copy(struct page **pages, /* Extra mismatched device page, copy it */ if (!match && last && vram_addr != XE_VRAM_ADDR_INVALID) { + xe_svm_copy_kb_stats_incr(gt, dir, + (PAGE_SIZE / SZ_1K)); if (sram) { vm_dbg(&xe->drm, "COPY TO SRAM - 0x%016llx -> 0x%016llx, NPAGES=%d", @@ -533,6 +634,14 @@ static int xe_svm_copy(struct page **pages, dma_fence_put(fence); } + /* + * XXX: We can't derive the GT here (or anywhere in this functions, but + * compute always uses the primary GT so accumlate stats on the likely + * GT of the fault. + */ + if (gt) + xe_svm_copy_us_stats_incr(gt, dir, npages, start); + return err; #undef XE_MIGRATE_CHUNK_SIZE #undef XE_VRAM_ADDR_INVALID @@ -846,6 +955,57 @@ bool xe_svm_range_needs_migrate_to_vram(struct xe_svm_range *range, struct xe_vm return true; } +#define DECL_SVM_RANGE_COUNT_STATS(elem, stat) \ +static void xe_svm_range_##elem##_count_stats_incr(struct xe_gt *gt, \ + struct xe_svm_range *range) \ +{ \ + switch (xe_svm_range_size(range)) { \ + case SZ_4K: \ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_##stat##_COUNT, 1); \ + break; \ + case SZ_64K: \ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_##stat##_COUNT, 1); \ + break; \ + case SZ_2M: \ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_##stat##_COUNT, 1); \ + break; \ + } \ +} \ + +DECL_SVM_RANGE_COUNT_STATS(fault, PAGEFAULT) +DECL_SVM_RANGE_COUNT_STATS(valid_fault, VALID_PAGEFAULT) +DECL_SVM_RANGE_COUNT_STATS(migrate, MIGRATE) + +#define DECL_SVM_RANGE_US_STATS(elem, stat) \ +static void xe_svm_range_##elem##_us_stats_incr(struct xe_gt *gt, \ + struct xe_svm_range *range, \ + ktime_t start) \ +{ \ + s64 us_delta = xe_svm_stats_ktime_us_delta(start); \ +\ + switch (xe_svm_range_size(range)) { \ + case SZ_4K: \ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_##stat##_US, \ + us_delta); \ + break; \ + case SZ_64K: \ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_##stat##_US, \ + us_delta); \ + break; \ + case SZ_2M: \ + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_##stat##_US, \ + us_delta); \ + break; \ + } \ +} \ + +DECL_SVM_RANGE_US_STATS(migrate, MIGRATE) +DECL_SVM_RANGE_US_STATS(get_pages, GET_PAGES) +DECL_SVM_RANGE_US_STATS(bind, BIND) +DECL_SVM_RANGE_US_STATS(fault, PAGEFAULT) + + + static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, struct xe_gt *gt, u64 fault_addr, bool need_vram) @@ -867,6 +1027,7 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, struct xe_tile *tile = gt_to_tile(gt); int migrate_try_count = ctx.devmem_only ? 3 : 1; ktime_t end = 0; + ktime_t start = xe_svm_stats_ktime_get(), bind_start, get_pages_start; int err; lockdep_assert_held_write(&vm->lock); @@ -885,23 +1046,34 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, if (IS_ERR(range)) return PTR_ERR(range); - if (ctx.devmem_only && !range->base.flags.migrate_devmem) - return -EACCES; + xe_svm_range_fault_count_stats_incr(gt, range); - if (xe_svm_range_is_valid(range, tile, ctx.devmem_only)) - return 0; + if (ctx.devmem_only && !range->base.flags.migrate_devmem) { + err = -EACCES; + goto out; + } + + if (xe_svm_range_is_valid(range, tile, ctx.devmem_only)) { + xe_svm_range_valid_fault_count_stats_incr(gt, range); + range_debug(range, "PAGE FAULT - VALID"); + goto out; + } range_debug(range, "PAGE FAULT"); dpagemap = xe_vma_resolve_pagemap(vma, tile); if (--migrate_try_count >= 0 && xe_svm_range_needs_migrate_to_vram(range, vma, !!dpagemap || ctx.devmem_only)) { + ktime_t migrate_start = xe_svm_stats_ktime_get(); + /* TODO : For multi-device dpagemap will be used to find the * remote tile and remote device. Will need to modify * xe_svm_alloc_vram to use dpagemap for future multi-device * support. */ + xe_svm_range_migrate_count_stats_incr(gt, range); err = xe_svm_alloc_vram(tile, range, &ctx); + xe_svm_range_migrate_us_stats_incr(gt, range, migrate_start); ctx.timeslice_ms <<= 1; /* Double timeslice if we have to retry */ if (err) { if (migrate_try_count || !ctx.devmem_only) { @@ -918,6 +1090,8 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, } } + get_pages_start = xe_svm_stats_ktime_get(); + range_debug(range, "GET PAGES"); err = xe_svm_range_get_pages(vm, range, &ctx); /* Corner where CPU mappings have changed */ @@ -937,11 +1111,13 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, } if (err) { range_debug(range, "PAGE FAULT - FAIL PAGE COLLECT"); - goto err_out; + goto out; } + xe_svm_range_get_pages_us_stats_incr(gt, range, get_pages_start); range_debug(range, "PAGE FAULT - BIND"); + bind_start = xe_svm_stats_ktime_get(); retry_bind: xe_vm_lock(vm, false); fence = xe_vm_range_rebind(vm, vma, range, BIT(tile->id)); @@ -955,14 +1131,16 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, } if (xe_vm_validate_should_retry(NULL, err, &end)) goto retry_bind; - goto err_out; + goto out; } xe_vm_unlock(vm); dma_fence_wait(fence, false); dma_fence_put(fence); + xe_svm_range_bind_us_stats_incr(gt, range, bind_start); -err_out: +out: + xe_svm_range_fault_us_stats_incr(gt, range, start); return err; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] drm/xe: Add more SVM GT stats 2025-08-27 18:43 ` [PATCH v5 2/2] drm/xe: Add more SVM GT stats Matthew Brost @ 2025-08-27 19:15 ` Michal Wajdeczko 2025-08-27 20:17 ` Matthew Brost 0 siblings, 1 reply; 7+ messages in thread From: Michal Wajdeczko @ 2025-08-27 19:15 UTC (permalink / raw) To: Matthew Brost, intel-xe On 8/27/2025 8:43 PM, Matthew Brost wrote: > Add more SVM GT stats which give visibility to where time is spent in > the SVM page fault handler. Stats include number of faults at a given > size, total SVM page fault time, migration time in us, copy time in us, > copy kb, get pages time in us, and bind time in us. Will help in tuning > SVM for performance. > > v2: > - Include local changes > v3: > - Add tlb invalidation + valid page fault + per size copy size stats > v4: > - Ensure gt not NULL when incrementing SVM copy stats > - Normalize stats names > - Use magic macros to generate increment functions for ranges > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > Reviewed-by: Francois Dugast <francois.dugast@intel.com> > --- > drivers/gpu/drm/xe/xe_gt_stats.c | 33 +++++ > drivers/gpu/drm/xe/xe_gt_stats_types.h | 33 +++++ > drivers/gpu/drm/xe/xe_svm.c | 196 +++++++++++++++++++++++-- > 3 files changed, 253 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c > index 7e12fc3759e2..2422577e4dee 100644 > --- a/drivers/gpu/drm/xe/xe_gt_stats.c > +++ b/drivers/gpu/drm/xe/xe_gt_stats.c > @@ -29,8 +29,41 @@ void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr) > static const char *const stat_description[__XE_GT_STATS_NUM_IDS] = { > "svm_pagefault_count", > "tlb_inval_count", > + "svm_tlb_inval_count", > + "svm_tlb_inval_us", maybe use [XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT] = "svm_tlb_inval_count", [XE_GT_STATS_ID_SVM_TLB_INVAL_US] = "svm_tlb_inval_us", ... to avoid mistakes? and I guess it would be ok to use helper macro like #define DEF_STAT_STR(ID,name) [XE_GT_STATS_ID_ ## ID] = name DEF_STAT_STR(SVM_TLB_INVAL_COUNT, "svm_tlb_inval_count"), or if you're brave enough go with a super-macro, like [1] ;) [1] https://elixir.bootlin.com/linux/v6.17-rc3/C/ident/MAKE_XE_GUC_KLV_THRESHOLDS_SET > "vma_pagefault_count", > "vma_pagefault_kb", > + "svm_4K_pagefault_count", > + "svm_64K_pagefault_count", > + "svm_2M_pagefault_count", > + "svm_4K_valid_pagefault_count", > + "svm_64K_valid_pagefault_count", > + "svm_2M_valid_pagefault_count", > + "svm_4K_pagefault_us", > + "svm_64K_pagefault_us", > + "svm_2M_pagefault_us", > + "svm_4K_migrate_count", > + "svm_64K_migrate_count", > + "svm_2M_migrate_count", > + "svm_4K_migrate_us", > + "svm_64K_migrate_us", > + "svm_2M_migrate_us", > + "svm_device_copy_us", > + "svm_4K_device_copy_us", > + "svm_64K_device_copy_us", > + "svm_2M_device_copy_us", > + "svm_cpu_copy_us", > + "svm_4K_cpu_copy_us", > + "svm_64K_cpu_copy_us", > + "svm_2M_cpu_copy_us", > + "svm_device_copy_kb", > + "svm_cpu_copy_kb", > + "svm_4K_get_pages_us", > + "svm_64K_get_pages_us", > + "svm_2M_get_pages_us", > + "svm_4K_bind_us", > + "svm_64K_bind_us", > + "svm_2M_bind_us", > }; > > /** > diff --git a/drivers/gpu/drm/xe/xe_gt_stats_types.h b/drivers/gpu/drm/xe/xe_gt_stats_types.h > index be3244d7133c..d8348a8de2e1 100644 > --- a/drivers/gpu/drm/xe/xe_gt_stats_types.h > +++ b/drivers/gpu/drm/xe/xe_gt_stats_types.h > @@ -9,8 +9,41 @@ > enum xe_gt_stats_id { > XE_GT_STATS_ID_SVM_PAGEFAULT_COUNT, > XE_GT_STATS_ID_TLB_INVAL, > + XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT, > + XE_GT_STATS_ID_SVM_TLB_INVAL_US, > XE_GT_STATS_ID_VMA_PAGEFAULT_COUNT, > XE_GT_STATS_ID_VMA_PAGEFAULT_KB, > + XE_GT_STATS_ID_SVM_4K_PAGEFAULT_COUNT, > + XE_GT_STATS_ID_SVM_64K_PAGEFAULT_COUNT, > + XE_GT_STATS_ID_SVM_2M_PAGEFAULT_COUNT, > + XE_GT_STATS_ID_SVM_4K_VALID_PAGEFAULT_COUNT, > + XE_GT_STATS_ID_SVM_64K_VALID_PAGEFAULT_COUNT, > + XE_GT_STATS_ID_SVM_2M_VALID_PAGEFAULT_COUNT, > + XE_GT_STATS_ID_SVM_4K_PAGEFAULT_US, > + XE_GT_STATS_ID_SVM_64K_PAGEFAULT_US, > + XE_GT_STATS_ID_SVM_2M_PAGEFAULT_US, > + XE_GT_STATS_ID_SVM_4K_MIGRATE_COUNT, > + XE_GT_STATS_ID_SVM_64K_MIGRATE_COUNT, > + XE_GT_STATS_ID_SVM_2M_MIGRATE_COUNT, > + XE_GT_STATS_ID_SVM_4K_MIGRATE_US, > + XE_GT_STATS_ID_SVM_64K_MIGRATE_US, > + XE_GT_STATS_ID_SVM_2M_MIGRATE_US, > + XE_GT_STATS_ID_SVM_DEVICE_COPY_US, > + XE_GT_STATS_ID_SVM_4K_DEVICE_COPY_US, > + XE_GT_STATS_ID_SVM_64K_DEVICE_COPY_US, > + XE_GT_STATS_ID_SVM_2M_DEVICE_COPY_US, > + XE_GT_STATS_ID_SVM_CPU_COPY_US, > + XE_GT_STATS_ID_SVM_4K_CPU_COPY_US, > + XE_GT_STATS_ID_SVM_64K_CPU_COPY_US, > + XE_GT_STATS_ID_SVM_2M_CPU_COPY_US, > + XE_GT_STATS_ID_SVM_DEVICE_COPY_KB, > + XE_GT_STATS_ID_SVM_CPU_COPY_KB, > + XE_GT_STATS_ID_SVM_4K_GET_PAGES_US, > + XE_GT_STATS_ID_SVM_64K_GET_PAGES_US, > + XE_GT_STATS_ID_SVM_2M_GET_PAGES_US, > + XE_GT_STATS_ID_SVM_4K_BIND_US, > + XE_GT_STATS_ID_SVM_64K_BIND_US, > + XE_GT_STATS_ID_SVM_2M_BIND_US, > /* must be the last entry */ > __XE_GT_STATS_NUM_IDS, > }; > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c > index 0596039ef0a1..7ab63e6b77b0 100644 > --- a/drivers/gpu/drm/xe/xe_svm.c > +++ b/drivers/gpu/drm/xe/xe_svm.c > @@ -6,6 +6,7 @@ > #include <drm/drm_drv.h> > > #include "xe_bo.h" > +#include "xe_exec_queue_types.h" > #include "xe_gt_stats.h" > #include "xe_gt_tlb_invalidation.h" > #include "xe_migrate.h" > @@ -113,6 +114,11 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, struct xe_svm_range *range, > &vm->svm.garbage_collector.work); > } > > +static void xe_svm_tlb_inval_count_stats_incr(struct xe_gt *gt) > +{ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT, 1); > +} > + > static u8 > xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r, > const struct mmu_notifier_range *mmu_range, > @@ -145,13 +151,19 @@ xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r, > */ > for_each_tile(tile, xe, id) > if (xe_pt_zap_ptes_range(tile, vm, range)) { > - tile_mask |= BIT(id); > /* > * WRITE_ONCE pairs with READ_ONCE in > * xe_vm_has_valid_gpu_mapping() > */ > WRITE_ONCE(range->tile_invalidated, > range->tile_invalidated | BIT(id)); > + > + if (!(tile_mask & BIT(id))) { > + xe_svm_tlb_inval_count_stats_incr(tile->primary_gt); > + if (tile->media_gt) > + xe_svm_tlb_inval_count_stats_incr(tile->media_gt); > + tile_mask |= BIT(id); > + } > } > > return tile_mask; > @@ -171,6 +183,24 @@ xe_svm_range_notifier_event_end(struct xe_vm *vm, struct drm_gpusvm_range *r, > mmu_range); > } > > +static s64 xe_svm_stats_ktime_us_delta(ktime_t start) > +{ > + return IS_ENABLED(CONFIG_DEBUG_FS) ? > + ktime_us_delta(ktime_get(), start) : 0; > +} > + > +static void xe_svm_tlb_inval_us_stats_incr(struct xe_gt *gt, ktime_t start) > +{ > + s64 us_delta = xe_svm_stats_ktime_us_delta(start); > + > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_TLB_INVAL_US, us_delta); > +} > + > +static ktime_t xe_svm_stats_ktime_get(void) > +{ > + return IS_ENABLED(CONFIG_DEBUG_FS) ? ktime_get() : 0; > +} > + > static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_notifier *notifier, > const struct mmu_notifier_range *mmu_range) > @@ -178,8 +208,10 @@ static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, > struct xe_vm *vm = gpusvm_to_vm(gpusvm); > struct xe_device *xe = vm->xe; > struct drm_gpusvm_range *r, *first; > + struct xe_tile *tile; > + ktime_t start = xe_svm_stats_ktime_get(); > u64 adj_start = mmu_range->start, adj_end = mmu_range->end; > - u8 tile_mask = 0; > + u8 tile_mask = 0, id; > long err; > > xe_svm_assert_in_notifier(vm); > @@ -232,6 +264,13 @@ static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, > r = first; > drm_gpusvm_for_each_range(r, notifier, adj_start, adj_end) > xe_svm_range_notifier_event_end(vm, r, mmu_range); > + for_each_tile(tile, xe, id) { > + if (tile_mask & BIT(id)) { > + xe_svm_tlb_inval_us_stats_incr(tile->primary_gt, start); > + if (tile->media_gt) > + xe_svm_tlb_inval_us_stats_incr(tile->media_gt, start); > + } > + } > } > > static int __xe_svm_garbage_collector(struct xe_vm *vm, > @@ -385,11 +424,66 @@ enum xe_svm_copy_dir { > XE_SVM_COPY_TO_SRAM, > }; > > +static void xe_svm_copy_kb_stats_incr(struct xe_gt *gt, > + const enum xe_svm_copy_dir dir, > + int kb) > +{ > + if (dir == XE_SVM_COPY_TO_VRAM) > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_DEVICE_COPY_KB, kb); > + else > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_CPU_COPY_KB, kb); shouldn't we code this like: if (dir == XE_SVM_COPY_TO_VRAM) id = XE_GT_STATS_ID_SVM_DEVICE_COPY_KB; else id = XE_GT_STATS_ID_SVM_CPU_COPY_KB; xe_gt_stats_incr(gt, id, kb); to have just one function call? or we rely on the compiler to optimize that? > +} > + > +static void xe_svm_copy_us_stats_incr(struct xe_gt *gt, > + const enum xe_svm_copy_dir dir, > + unsigned long npages, > + ktime_t start) > +{ > + s64 us_delta = xe_svm_stats_ktime_us_delta(start); > + > + if (dir == XE_SVM_COPY_TO_VRAM) { > + switch (npages) { > + case 1: > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_DEVICE_COPY_US, > + us_delta); > + break; > + case 16: > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_DEVICE_COPY_US, > + us_delta); > + break; > + case 512: > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_DEVICE_COPY_US, > + us_delta); > + break; > + } > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_DEVICE_COPY_US, > + us_delta); > + } else { > + switch (npages) { > + case 1: > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_CPU_COPY_US, > + us_delta); > + break; > + case 16: > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_CPU_COPY_US, > + us_delta); > + break; > + case 512: > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_CPU_COPY_US, > + us_delta); > + break; > + } > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_CPU_COPY_US, > + us_delta); > + } > +} > + > static int xe_svm_copy(struct page **pages, > struct drm_pagemap_addr *pagemap_addr, > unsigned long npages, const enum xe_svm_copy_dir dir) > { > struct xe_vram_region *vr = NULL; > + struct xe_gt *gt = NULL; > struct xe_device *xe; > struct dma_fence *fence = NULL; > unsigned long i; > @@ -397,6 +491,7 @@ static int xe_svm_copy(struct page **pages, > u64 vram_addr = XE_VRAM_ADDR_INVALID; > int err = 0, pos = 0; > bool sram = dir == XE_SVM_COPY_TO_SRAM; > + ktime_t start = xe_svm_stats_ktime_get(); > > /* > * This flow is complex: it locates physically contiguous device pages, > @@ -423,6 +518,7 @@ static int xe_svm_copy(struct page **pages, > > if (!vr && spage) { > vr = page_to_vr(spage); > + gt = xe_migrate_exec_queue(vr->migrate)->gt; > xe = vr->xe; > } > XE_WARN_ON(spage && page_to_vr(spage) != vr); > @@ -462,6 +558,9 @@ static int xe_svm_copy(struct page **pages, > int incr = (match && last) ? 1 : 0; > > if (vram_addr != XE_VRAM_ADDR_INVALID) { > + xe_svm_copy_kb_stats_incr(gt, dir, > + (i - pos + incr) * > + (PAGE_SIZE / SZ_1K)); > if (sram) { > vm_dbg(&xe->drm, > "COPY TO SRAM - 0x%016llx -> 0x%016llx, NPAGES=%ld", > @@ -500,6 +599,8 @@ static int xe_svm_copy(struct page **pages, > > /* Extra mismatched device page, copy it */ > if (!match && last && vram_addr != XE_VRAM_ADDR_INVALID) { > + xe_svm_copy_kb_stats_incr(gt, dir, > + (PAGE_SIZE / SZ_1K)); > if (sram) { > vm_dbg(&xe->drm, > "COPY TO SRAM - 0x%016llx -> 0x%016llx, NPAGES=%d", > @@ -533,6 +634,14 @@ static int xe_svm_copy(struct page **pages, > dma_fence_put(fence); > } > > + /* > + * XXX: We can't derive the GT here (or anywhere in this functions, but > + * compute always uses the primary GT so accumlate stats on the likely > + * GT of the fault. > + */ > + if (gt) > + xe_svm_copy_us_stats_incr(gt, dir, npages, start); > + > return err; > #undef XE_MIGRATE_CHUNK_SIZE > #undef XE_VRAM_ADDR_INVALID > @@ -846,6 +955,57 @@ bool xe_svm_range_needs_migrate_to_vram(struct xe_svm_range *range, struct xe_vm > return true; > } > > +#define DECL_SVM_RANGE_COUNT_STATS(elem, stat) \ > +static void xe_svm_range_##elem##_count_stats_incr(struct xe_gt *gt, \ > + struct xe_svm_range *range) \ > +{ \ > + switch (xe_svm_range_size(range)) { \ > + case SZ_4K: \ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_##stat##_COUNT, 1); \ > + break; \ > + case SZ_64K: \ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_##stat##_COUNT, 1); \ > + break; \ > + case SZ_2M: \ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_##stat##_COUNT, 1); \ > + break; \ > + } \ > +} \ > + > +DECL_SVM_RANGE_COUNT_STATS(fault, PAGEFAULT) > +DECL_SVM_RANGE_COUNT_STATS(valid_fault, VALID_PAGEFAULT) > +DECL_SVM_RANGE_COUNT_STATS(migrate, MIGRATE) > + > +#define DECL_SVM_RANGE_US_STATS(elem, stat) \ > +static void xe_svm_range_##elem##_us_stats_incr(struct xe_gt *gt, \ > + struct xe_svm_range *range, \ > + ktime_t start) \ > +{ \ > + s64 us_delta = xe_svm_stats_ktime_us_delta(start); \ > +\ > + switch (xe_svm_range_size(range)) { \ > + case SZ_4K: \ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_##stat##_US, \ > + us_delta); \ > + break; \ > + case SZ_64K: \ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_##stat##_US, \ > + us_delta); \ > + break; \ > + case SZ_2M: \ > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_##stat##_US, \ > + us_delta); \ > + break; \ > + } \ > +} \ > + > +DECL_SVM_RANGE_US_STATS(migrate, MIGRATE) > +DECL_SVM_RANGE_US_STATS(get_pages, GET_PAGES) > +DECL_SVM_RANGE_US_STATS(bind, BIND) > +DECL_SVM_RANGE_US_STATS(fault, PAGEFAULT) > + > + > + > static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > struct xe_gt *gt, u64 fault_addr, > bool need_vram) > @@ -867,6 +1027,7 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > struct xe_tile *tile = gt_to_tile(gt); > int migrate_try_count = ctx.devmem_only ? 3 : 1; > ktime_t end = 0; > + ktime_t start = xe_svm_stats_ktime_get(), bind_start, get_pages_start; > int err; > > lockdep_assert_held_write(&vm->lock); > @@ -885,23 +1046,34 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > if (IS_ERR(range)) > return PTR_ERR(range); > > - if (ctx.devmem_only && !range->base.flags.migrate_devmem) > - return -EACCES; > + xe_svm_range_fault_count_stats_incr(gt, range); > > - if (xe_svm_range_is_valid(range, tile, ctx.devmem_only)) > - return 0; > + if (ctx.devmem_only && !range->base.flags.migrate_devmem) { > + err = -EACCES; > + goto out; > + } > + > + if (xe_svm_range_is_valid(range, tile, ctx.devmem_only)) { > + xe_svm_range_valid_fault_count_stats_incr(gt, range); > + range_debug(range, "PAGE FAULT - VALID"); > + goto out; > + } > > range_debug(range, "PAGE FAULT"); > > dpagemap = xe_vma_resolve_pagemap(vma, tile); > if (--migrate_try_count >= 0 && > xe_svm_range_needs_migrate_to_vram(range, vma, !!dpagemap || ctx.devmem_only)) { > + ktime_t migrate_start = xe_svm_stats_ktime_get(); > + > /* TODO : For multi-device dpagemap will be used to find the > * remote tile and remote device. Will need to modify > * xe_svm_alloc_vram to use dpagemap for future multi-device > * support. > */ > + xe_svm_range_migrate_count_stats_incr(gt, range); > err = xe_svm_alloc_vram(tile, range, &ctx); > + xe_svm_range_migrate_us_stats_incr(gt, range, migrate_start); > ctx.timeslice_ms <<= 1; /* Double timeslice if we have to retry */ > if (err) { > if (migrate_try_count || !ctx.devmem_only) { > @@ -918,6 +1090,8 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > } > } > > + get_pages_start = xe_svm_stats_ktime_get(); > + > range_debug(range, "GET PAGES"); > err = xe_svm_range_get_pages(vm, range, &ctx); > /* Corner where CPU mappings have changed */ > @@ -937,11 +1111,13 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > } > if (err) { > range_debug(range, "PAGE FAULT - FAIL PAGE COLLECT"); > - goto err_out; > + goto out; > } > > + xe_svm_range_get_pages_us_stats_incr(gt, range, get_pages_start); > range_debug(range, "PAGE FAULT - BIND"); > > + bind_start = xe_svm_stats_ktime_get(); > retry_bind: > xe_vm_lock(vm, false); > fence = xe_vm_range_rebind(vm, vma, range, BIT(tile->id)); > @@ -955,14 +1131,16 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > } > if (xe_vm_validate_should_retry(NULL, err, &end)) > goto retry_bind; > - goto err_out; > + goto out; > } > xe_vm_unlock(vm); > > dma_fence_wait(fence, false); > dma_fence_put(fence); > + xe_svm_range_bind_us_stats_incr(gt, range, bind_start); > > -err_out: > +out: > + xe_svm_range_fault_us_stats_incr(gt, range, start); > > return err; > } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] drm/xe: Add more SVM GT stats 2025-08-27 19:15 ` Michal Wajdeczko @ 2025-08-27 20:17 ` Matthew Brost 0 siblings, 0 replies; 7+ messages in thread From: Matthew Brost @ 2025-08-27 20:17 UTC (permalink / raw) To: Michal Wajdeczko; +Cc: intel-xe On Wed, Aug 27, 2025 at 09:15:46PM +0200, Michal Wajdeczko wrote: > > > On 8/27/2025 8:43 PM, Matthew Brost wrote: > > Add more SVM GT stats which give visibility to where time is spent in > > the SVM page fault handler. Stats include number of faults at a given > > size, total SVM page fault time, migration time in us, copy time in us, > > copy kb, get pages time in us, and bind time in us. Will help in tuning > > SVM for performance. > > > > v2: > > - Include local changes > > v3: > > - Add tlb invalidation + valid page fault + per size copy size stats > > v4: > > - Ensure gt not NULL when incrementing SVM copy stats > > - Normalize stats names > > - Use magic macros to generate increment functions for ranges > > > > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > > Reviewed-by: Francois Dugast <francois.dugast@intel.com> > > --- > > drivers/gpu/drm/xe/xe_gt_stats.c | 33 +++++ > > drivers/gpu/drm/xe/xe_gt_stats_types.h | 33 +++++ > > drivers/gpu/drm/xe/xe_svm.c | 196 +++++++++++++++++++++++-- > > 3 files changed, 253 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_gt_stats.c b/drivers/gpu/drm/xe/xe_gt_stats.c > > index 7e12fc3759e2..2422577e4dee 100644 > > --- a/drivers/gpu/drm/xe/xe_gt_stats.c > > +++ b/drivers/gpu/drm/xe/xe_gt_stats.c > > @@ -29,8 +29,41 @@ void xe_gt_stats_incr(struct xe_gt *gt, const enum xe_gt_stats_id id, int incr) > > static const char *const stat_description[__XE_GT_STATS_NUM_IDS] = { > > "svm_pagefault_count", > > "tlb_inval_count", > > + "svm_tlb_inval_count", > > + "svm_tlb_inval_us", > > maybe use > > [XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT] = "svm_tlb_inval_count", > [XE_GT_STATS_ID_SVM_TLB_INVAL_US] = "svm_tlb_inval_us", > ... > > to avoid mistakes? > > and I guess it would be ok to use helper macro like > > #define DEF_STAT_STR(ID,name) [XE_GT_STATS_ID_ ## ID] = name > > DEF_STAT_STR(SVM_TLB_INVAL_COUNT, "svm_tlb_inval_count"), Sure. > > or if you're brave enough go with a super-macro, like [1] ;) > > [1] https://elixir.bootlin.com/linux/v6.17-rc3/C/ident/MAKE_XE_GUC_KLV_THRESHOLDS_SET > Probably over kill. > > > "vma_pagefault_count", > > "vma_pagefault_kb", > > + "svm_4K_pagefault_count", > > + "svm_64K_pagefault_count", > > + "svm_2M_pagefault_count", > > + "svm_4K_valid_pagefault_count", > > + "svm_64K_valid_pagefault_count", > > + "svm_2M_valid_pagefault_count", > > + "svm_4K_pagefault_us", > > + "svm_64K_pagefault_us", > > + "svm_2M_pagefault_us", > > + "svm_4K_migrate_count", > > + "svm_64K_migrate_count", > > + "svm_2M_migrate_count", > > + "svm_4K_migrate_us", > > + "svm_64K_migrate_us", > > + "svm_2M_migrate_us", > > + "svm_device_copy_us", > > + "svm_4K_device_copy_us", > > + "svm_64K_device_copy_us", > > + "svm_2M_device_copy_us", > > + "svm_cpu_copy_us", > > + "svm_4K_cpu_copy_us", > > + "svm_64K_cpu_copy_us", > > + "svm_2M_cpu_copy_us", > > + "svm_device_copy_kb", > > + "svm_cpu_copy_kb", > > + "svm_4K_get_pages_us", > > + "svm_64K_get_pages_us", > > + "svm_2M_get_pages_us", > > + "svm_4K_bind_us", > > + "svm_64K_bind_us", > > + "svm_2M_bind_us", > > }; > > > > /** > > diff --git a/drivers/gpu/drm/xe/xe_gt_stats_types.h b/drivers/gpu/drm/xe/xe_gt_stats_types.h > > index be3244d7133c..d8348a8de2e1 100644 > > --- a/drivers/gpu/drm/xe/xe_gt_stats_types.h > > +++ b/drivers/gpu/drm/xe/xe_gt_stats_types.h > > @@ -9,8 +9,41 @@ > > enum xe_gt_stats_id { > > XE_GT_STATS_ID_SVM_PAGEFAULT_COUNT, > > XE_GT_STATS_ID_TLB_INVAL, > > + XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT, > > + XE_GT_STATS_ID_SVM_TLB_INVAL_US, > > XE_GT_STATS_ID_VMA_PAGEFAULT_COUNT, > > XE_GT_STATS_ID_VMA_PAGEFAULT_KB, > > + XE_GT_STATS_ID_SVM_4K_PAGEFAULT_COUNT, > > + XE_GT_STATS_ID_SVM_64K_PAGEFAULT_COUNT, > > + XE_GT_STATS_ID_SVM_2M_PAGEFAULT_COUNT, > > + XE_GT_STATS_ID_SVM_4K_VALID_PAGEFAULT_COUNT, > > + XE_GT_STATS_ID_SVM_64K_VALID_PAGEFAULT_COUNT, > > + XE_GT_STATS_ID_SVM_2M_VALID_PAGEFAULT_COUNT, > > + XE_GT_STATS_ID_SVM_4K_PAGEFAULT_US, > > + XE_GT_STATS_ID_SVM_64K_PAGEFAULT_US, > > + XE_GT_STATS_ID_SVM_2M_PAGEFAULT_US, > > + XE_GT_STATS_ID_SVM_4K_MIGRATE_COUNT, > > + XE_GT_STATS_ID_SVM_64K_MIGRATE_COUNT, > > + XE_GT_STATS_ID_SVM_2M_MIGRATE_COUNT, > > + XE_GT_STATS_ID_SVM_4K_MIGRATE_US, > > + XE_GT_STATS_ID_SVM_64K_MIGRATE_US, > > + XE_GT_STATS_ID_SVM_2M_MIGRATE_US, > > + XE_GT_STATS_ID_SVM_DEVICE_COPY_US, > > + XE_GT_STATS_ID_SVM_4K_DEVICE_COPY_US, > > + XE_GT_STATS_ID_SVM_64K_DEVICE_COPY_US, > > + XE_GT_STATS_ID_SVM_2M_DEVICE_COPY_US, > > + XE_GT_STATS_ID_SVM_CPU_COPY_US, > > + XE_GT_STATS_ID_SVM_4K_CPU_COPY_US, > > + XE_GT_STATS_ID_SVM_64K_CPU_COPY_US, > > + XE_GT_STATS_ID_SVM_2M_CPU_COPY_US, > > + XE_GT_STATS_ID_SVM_DEVICE_COPY_KB, > > + XE_GT_STATS_ID_SVM_CPU_COPY_KB, > > + XE_GT_STATS_ID_SVM_4K_GET_PAGES_US, > > + XE_GT_STATS_ID_SVM_64K_GET_PAGES_US, > > + XE_GT_STATS_ID_SVM_2M_GET_PAGES_US, > > + XE_GT_STATS_ID_SVM_4K_BIND_US, > > + XE_GT_STATS_ID_SVM_64K_BIND_US, > > + XE_GT_STATS_ID_SVM_2M_BIND_US, > > /* must be the last entry */ > > __XE_GT_STATS_NUM_IDS, > > }; > > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c > > index 0596039ef0a1..7ab63e6b77b0 100644 > > --- a/drivers/gpu/drm/xe/xe_svm.c > > +++ b/drivers/gpu/drm/xe/xe_svm.c > > @@ -6,6 +6,7 @@ > > #include <drm/drm_drv.h> > > > > #include "xe_bo.h" > > +#include "xe_exec_queue_types.h" > > #include "xe_gt_stats.h" > > #include "xe_gt_tlb_invalidation.h" > > #include "xe_migrate.h" > > @@ -113,6 +114,11 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, struct xe_svm_range *range, > > &vm->svm.garbage_collector.work); > > } > > > > +static void xe_svm_tlb_inval_count_stats_incr(struct xe_gt *gt) > > +{ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_TLB_INVAL_COUNT, 1); > > +} > > + > > static u8 > > xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r, > > const struct mmu_notifier_range *mmu_range, > > @@ -145,13 +151,19 @@ xe_svm_range_notifier_event_begin(struct xe_vm *vm, struct drm_gpusvm_range *r, > > */ > > for_each_tile(tile, xe, id) > > if (xe_pt_zap_ptes_range(tile, vm, range)) { > > - tile_mask |= BIT(id); > > /* > > * WRITE_ONCE pairs with READ_ONCE in > > * xe_vm_has_valid_gpu_mapping() > > */ > > WRITE_ONCE(range->tile_invalidated, > > range->tile_invalidated | BIT(id)); > > + > > + if (!(tile_mask & BIT(id))) { > > + xe_svm_tlb_inval_count_stats_incr(tile->primary_gt); > > + if (tile->media_gt) > > + xe_svm_tlb_inval_count_stats_incr(tile->media_gt); > > + tile_mask |= BIT(id); > > + } > > } > > > > return tile_mask; > > @@ -171,6 +183,24 @@ xe_svm_range_notifier_event_end(struct xe_vm *vm, struct drm_gpusvm_range *r, > > mmu_range); > > } > > > > +static s64 xe_svm_stats_ktime_us_delta(ktime_t start) > > +{ > > + return IS_ENABLED(CONFIG_DEBUG_FS) ? > > + ktime_us_delta(ktime_get(), start) : 0; > > +} > > + > > +static void xe_svm_tlb_inval_us_stats_incr(struct xe_gt *gt, ktime_t start) > > +{ > > + s64 us_delta = xe_svm_stats_ktime_us_delta(start); > > + > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_TLB_INVAL_US, us_delta); > > +} > > + > > +static ktime_t xe_svm_stats_ktime_get(void) > > +{ > > + return IS_ENABLED(CONFIG_DEBUG_FS) ? ktime_get() : 0; > > +} > > + > > static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, > > struct drm_gpusvm_notifier *notifier, > > const struct mmu_notifier_range *mmu_range) > > @@ -178,8 +208,10 @@ static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, > > struct xe_vm *vm = gpusvm_to_vm(gpusvm); > > struct xe_device *xe = vm->xe; > > struct drm_gpusvm_range *r, *first; > > + struct xe_tile *tile; > > + ktime_t start = xe_svm_stats_ktime_get(); > > u64 adj_start = mmu_range->start, adj_end = mmu_range->end; > > - u8 tile_mask = 0; > > + u8 tile_mask = 0, id; > > long err; > > > > xe_svm_assert_in_notifier(vm); > > @@ -232,6 +264,13 @@ static void xe_svm_invalidate(struct drm_gpusvm *gpusvm, > > r = first; > > drm_gpusvm_for_each_range(r, notifier, adj_start, adj_end) > > xe_svm_range_notifier_event_end(vm, r, mmu_range); > > + for_each_tile(tile, xe, id) { > > + if (tile_mask & BIT(id)) { > > + xe_svm_tlb_inval_us_stats_incr(tile->primary_gt, start); > > + if (tile->media_gt) > > + xe_svm_tlb_inval_us_stats_incr(tile->media_gt, start); > > + } > > + } > > } > > > > static int __xe_svm_garbage_collector(struct xe_vm *vm, > > @@ -385,11 +424,66 @@ enum xe_svm_copy_dir { > > XE_SVM_COPY_TO_SRAM, > > }; > > > > +static void xe_svm_copy_kb_stats_incr(struct xe_gt *gt, > > + const enum xe_svm_copy_dir dir, > > + int kb) > > +{ > > + if (dir == XE_SVM_COPY_TO_VRAM) > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_DEVICE_COPY_KB, kb); > > + else > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_CPU_COPY_KB, kb); > > shouldn't we code this like: > > if (dir == XE_SVM_COPY_TO_VRAM) > id = XE_GT_STATS_ID_SVM_DEVICE_COPY_KB; > else > id = XE_GT_STATS_ID_SVM_CPU_COPY_KB; > > xe_gt_stats_incr(gt, id, kb); > > to have just one function call? or we rely on the compiler to optimize that? > I don't think this matters. Matt > > +} > > + > > +static void xe_svm_copy_us_stats_incr(struct xe_gt *gt, > > + const enum xe_svm_copy_dir dir, > > + unsigned long npages, > > + ktime_t start) > > +{ > > + s64 us_delta = xe_svm_stats_ktime_us_delta(start); > > + > > + if (dir == XE_SVM_COPY_TO_VRAM) { > > + switch (npages) { > > + case 1: > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_DEVICE_COPY_US, > > + us_delta); > > + break; > > + case 16: > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_DEVICE_COPY_US, > > + us_delta); > > + break; > > + case 512: > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_DEVICE_COPY_US, > > + us_delta); > > + break; > > + } > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_DEVICE_COPY_US, > > + us_delta); > > + } else { > > + switch (npages) { > > + case 1: > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_CPU_COPY_US, > > + us_delta); > > + break; > > + case 16: > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_CPU_COPY_US, > > + us_delta); > > + break; > > + case 512: > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_CPU_COPY_US, > > + us_delta); > > + break; > > + } > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_CPU_COPY_US, > > + us_delta); > > + } > > +} > > + > > static int xe_svm_copy(struct page **pages, > > struct drm_pagemap_addr *pagemap_addr, > > unsigned long npages, const enum xe_svm_copy_dir dir) > > { > > struct xe_vram_region *vr = NULL; > > + struct xe_gt *gt = NULL; > > struct xe_device *xe; > > struct dma_fence *fence = NULL; > > unsigned long i; > > @@ -397,6 +491,7 @@ static int xe_svm_copy(struct page **pages, > > u64 vram_addr = XE_VRAM_ADDR_INVALID; > > int err = 0, pos = 0; > > bool sram = dir == XE_SVM_COPY_TO_SRAM; > > + ktime_t start = xe_svm_stats_ktime_get(); > > > > /* > > * This flow is complex: it locates physically contiguous device pages, > > @@ -423,6 +518,7 @@ static int xe_svm_copy(struct page **pages, > > > > if (!vr && spage) { > > vr = page_to_vr(spage); > > + gt = xe_migrate_exec_queue(vr->migrate)->gt; > > xe = vr->xe; > > } > > XE_WARN_ON(spage && page_to_vr(spage) != vr); > > @@ -462,6 +558,9 @@ static int xe_svm_copy(struct page **pages, > > int incr = (match && last) ? 1 : 0; > > > > if (vram_addr != XE_VRAM_ADDR_INVALID) { > > + xe_svm_copy_kb_stats_incr(gt, dir, > > + (i - pos + incr) * > > + (PAGE_SIZE / SZ_1K)); > > if (sram) { > > vm_dbg(&xe->drm, > > "COPY TO SRAM - 0x%016llx -> 0x%016llx, NPAGES=%ld", > > @@ -500,6 +599,8 @@ static int xe_svm_copy(struct page **pages, > > > > /* Extra mismatched device page, copy it */ > > if (!match && last && vram_addr != XE_VRAM_ADDR_INVALID) { > > + xe_svm_copy_kb_stats_incr(gt, dir, > > + (PAGE_SIZE / SZ_1K)); > > if (sram) { > > vm_dbg(&xe->drm, > > "COPY TO SRAM - 0x%016llx -> 0x%016llx, NPAGES=%d", > > @@ -533,6 +634,14 @@ static int xe_svm_copy(struct page **pages, > > dma_fence_put(fence); > > } > > > > + /* > > + * XXX: We can't derive the GT here (or anywhere in this functions, but > > + * compute always uses the primary GT so accumlate stats on the likely > > + * GT of the fault. > > + */ > > + if (gt) > > + xe_svm_copy_us_stats_incr(gt, dir, npages, start); > > + > > return err; > > #undef XE_MIGRATE_CHUNK_SIZE > > #undef XE_VRAM_ADDR_INVALID > > @@ -846,6 +955,57 @@ bool xe_svm_range_needs_migrate_to_vram(struct xe_svm_range *range, struct xe_vm > > return true; > > } > > > > +#define DECL_SVM_RANGE_COUNT_STATS(elem, stat) \ > > +static void xe_svm_range_##elem##_count_stats_incr(struct xe_gt *gt, \ > > + struct xe_svm_range *range) \ > > +{ \ > > + switch (xe_svm_range_size(range)) { \ > > + case SZ_4K: \ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_##stat##_COUNT, 1); \ > > + break; \ > > + case SZ_64K: \ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_##stat##_COUNT, 1); \ > > + break; \ > > + case SZ_2M: \ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_##stat##_COUNT, 1); \ > > + break; \ > > + } \ > > +} \ > > + > > +DECL_SVM_RANGE_COUNT_STATS(fault, PAGEFAULT) > > +DECL_SVM_RANGE_COUNT_STATS(valid_fault, VALID_PAGEFAULT) > > +DECL_SVM_RANGE_COUNT_STATS(migrate, MIGRATE) > > + > > +#define DECL_SVM_RANGE_US_STATS(elem, stat) \ > > +static void xe_svm_range_##elem##_us_stats_incr(struct xe_gt *gt, \ > > + struct xe_svm_range *range, \ > > + ktime_t start) \ > > +{ \ > > + s64 us_delta = xe_svm_stats_ktime_us_delta(start); \ > > +\ > > + switch (xe_svm_range_size(range)) { \ > > + case SZ_4K: \ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_4K_##stat##_US, \ > > + us_delta); \ > > + break; \ > > + case SZ_64K: \ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_64K_##stat##_US, \ > > + us_delta); \ > > + break; \ > > + case SZ_2M: \ > > + xe_gt_stats_incr(gt, XE_GT_STATS_ID_SVM_2M_##stat##_US, \ > > + us_delta); \ > > + break; \ > > + } \ > > +} \ > > + > > +DECL_SVM_RANGE_US_STATS(migrate, MIGRATE) > > +DECL_SVM_RANGE_US_STATS(get_pages, GET_PAGES) > > +DECL_SVM_RANGE_US_STATS(bind, BIND) > > +DECL_SVM_RANGE_US_STATS(fault, PAGEFAULT) > > + > > + > > + > > static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > > struct xe_gt *gt, u64 fault_addr, > > bool need_vram) > > @@ -867,6 +1027,7 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > > struct xe_tile *tile = gt_to_tile(gt); > > int migrate_try_count = ctx.devmem_only ? 3 : 1; > > ktime_t end = 0; > > + ktime_t start = xe_svm_stats_ktime_get(), bind_start, get_pages_start; > > int err; > > > > lockdep_assert_held_write(&vm->lock); > > @@ -885,23 +1046,34 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > > if (IS_ERR(range)) > > return PTR_ERR(range); > > > > - if (ctx.devmem_only && !range->base.flags.migrate_devmem) > > - return -EACCES; > > + xe_svm_range_fault_count_stats_incr(gt, range); > > > > - if (xe_svm_range_is_valid(range, tile, ctx.devmem_only)) > > - return 0; > > + if (ctx.devmem_only && !range->base.flags.migrate_devmem) { > > + err = -EACCES; > > + goto out; > > + } > > + > > + if (xe_svm_range_is_valid(range, tile, ctx.devmem_only)) { > > + xe_svm_range_valid_fault_count_stats_incr(gt, range); > > + range_debug(range, "PAGE FAULT - VALID"); > > + goto out; > > + } > > > > range_debug(range, "PAGE FAULT"); > > > > dpagemap = xe_vma_resolve_pagemap(vma, tile); > > if (--migrate_try_count >= 0 && > > xe_svm_range_needs_migrate_to_vram(range, vma, !!dpagemap || ctx.devmem_only)) { > > + ktime_t migrate_start = xe_svm_stats_ktime_get(); > > + > > /* TODO : For multi-device dpagemap will be used to find the > > * remote tile and remote device. Will need to modify > > * xe_svm_alloc_vram to use dpagemap for future multi-device > > * support. > > */ > > + xe_svm_range_migrate_count_stats_incr(gt, range); > > err = xe_svm_alloc_vram(tile, range, &ctx); > > + xe_svm_range_migrate_us_stats_incr(gt, range, migrate_start); > > ctx.timeslice_ms <<= 1; /* Double timeslice if we have to retry */ > > if (err) { > > if (migrate_try_count || !ctx.devmem_only) { > > @@ -918,6 +1090,8 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > > } > > } > > > > + get_pages_start = xe_svm_stats_ktime_get(); > > + > > range_debug(range, "GET PAGES"); > > err = xe_svm_range_get_pages(vm, range, &ctx); > > /* Corner where CPU mappings have changed */ > > @@ -937,11 +1111,13 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > > } > > if (err) { > > range_debug(range, "PAGE FAULT - FAIL PAGE COLLECT"); > > - goto err_out; > > + goto out; > > } > > > > + xe_svm_range_get_pages_us_stats_incr(gt, range, get_pages_start); > > range_debug(range, "PAGE FAULT - BIND"); > > > > + bind_start = xe_svm_stats_ktime_get(); > > retry_bind: > > xe_vm_lock(vm, false); > > fence = xe_vm_range_rebind(vm, vma, range, BIT(tile->id)); > > @@ -955,14 +1131,16 @@ static int __xe_svm_handle_pagefault(struct xe_vm *vm, struct xe_vma *vma, > > } > > if (xe_vm_validate_should_retry(NULL, err, &end)) > > goto retry_bind; > > - goto err_out; > > + goto out; > > } > > xe_vm_unlock(vm); > > > > dma_fence_wait(fence, false); > > dma_fence_put(fence); > > + xe_svm_range_bind_us_stats_incr(gt, range, bind_start); > > > > -err_out: > > +out: > > + xe_svm_range_fault_us_stats_incr(gt, range, start); > > > > return err; > > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-27 20:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-27 18:43 [PATCH v5 0/2] Enhance SVM stats Matthew Brost 2025-08-27 18:43 ` [PATCH v5 1/2] drm/xe: Add clearing stats to GT debugfs Matthew Brost 2025-08-27 19:02 ` Michal Wajdeczko 2025-08-27 19:14 ` Matthew Brost 2025-08-27 18:43 ` [PATCH v5 2/2] drm/xe: Add more SVM GT stats Matthew Brost 2025-08-27 19:15 ` Michal Wajdeczko 2025-08-27 20:17 ` Matthew Brost
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.