* [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
@ 2023-09-25 13:49 ` Nirmoy Das
0 siblings, 0 replies; 11+ messages in thread
From: Nirmoy Das @ 2023-09-25 13:49 UTC (permalink / raw)
To: intel-gfx; +Cc: Andrzej Hajda, Jonathan Cavitt, dri-devel, Nirmoy Das
From: Jonathan Cavitt <jonathan.cavitt@intel.com>
Where applicable, use for_each_gt instead of to_gt in the
i915_gem_shrinker functions to make them apply to more than just the
primary GT. Specifically, this ensure i915_gem_shrink_all retires all
requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
VMAs from all GTs.
v2: Pass correct GT to intel_gt_retire_requests(Andrzej).
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 44 ++++++++++++--------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 214763942aa2..9cb7bbfb4278 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -14,6 +14,7 @@
#include <linux/vmalloc.h>
#include "gt/intel_gt_requests.h"
+#include "gt/intel_gt.h"
#include "i915_trace.h"
@@ -119,7 +120,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
intel_wakeref_t wakeref = 0;
unsigned long count = 0;
unsigned long scanned = 0;
- int err = 0;
+ int err = 0, i = 0;
+ struct intel_gt *gt;
/* CHV + VTD workaround use stop_machine(); need to trylock vm->mutex */
bool trylock_vm = !ww && intel_vm_no_concurrent_access_wa(i915);
@@ -147,9 +149,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
* what we can do is give them a kick so that we do not keep idle
* contexts around longer than is necessary.
*/
- if (shrink & I915_SHRINK_ACTIVE)
- /* Retire requests to unpin all idle contexts */
- intel_gt_retire_requests(to_gt(i915));
+ if (shrink & I915_SHRINK_ACTIVE) {
+ for_each_gt(gt, i915, i)
+ /* Retire requests to unpin all idle contexts */
+ intel_gt_retire_requests(gt);
+ }
/*
* As we may completely rewrite the (un)bound list whilst unbinding
@@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
struct i915_vma *vma, *next;
unsigned long freed_pages = 0;
intel_wakeref_t wakeref;
+ struct intel_gt *gt;
+ int i;
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
@@ -397,24 +403,26 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
I915_SHRINK_VMAPS);
/* We also want to clear any cached iomaps as they wrap vmap */
- mutex_lock(&to_gt(i915)->ggtt->vm.mutex);
- list_for_each_entry_safe(vma, next,
- &to_gt(i915)->ggtt->vm.bound_list, vm_link) {
- unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
- struct drm_i915_gem_object *obj = vma->obj;
-
- if (!vma->iomap || i915_vma_is_active(vma))
- continue;
+ for_each_gt(gt, i915, i) {
+ mutex_lock(>->ggtt->vm.mutex);
+ list_for_each_entry_safe(vma, next,
+ >->ggtt->vm.bound_list, vm_link) {
+ unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
+ struct drm_i915_gem_object *obj = vma->obj;
+
+ if (!vma->iomap || i915_vma_is_active(vma))
+ continue;
- if (!i915_gem_object_trylock(obj, NULL))
- continue;
+ if (!i915_gem_object_trylock(obj, NULL))
+ continue;
- if (__i915_vma_unbind(vma) == 0)
- freed_pages += count;
+ if (__i915_vma_unbind(vma) == 0)
+ freed_pages += count;
- i915_gem_object_unlock(obj);
+ i915_gem_object_unlock(obj);
+ }
+ mutex_unlock(>->ggtt->vm.mutex);
}
- mutex_unlock(&to_gt(i915)->ggtt->vm.mutex);
*(unsigned long *)ptr += freed_pages;
return NOTIFY_DONE;
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
@ 2023-09-25 13:49 ` Nirmoy Das
0 siblings, 0 replies; 11+ messages in thread
From: Nirmoy Das @ 2023-09-25 13:49 UTC (permalink / raw)
To: intel-gfx
Cc: Andrzej Hajda, Jonathan Cavitt, andi.shyti, dri-devel, Nirmoy Das
From: Jonathan Cavitt <jonathan.cavitt@intel.com>
Where applicable, use for_each_gt instead of to_gt in the
i915_gem_shrinker functions to make them apply to more than just the
primary GT. Specifically, this ensure i915_gem_shrink_all retires all
requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
VMAs from all GTs.
v2: Pass correct GT to intel_gt_retire_requests(Andrzej).
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 44 ++++++++++++--------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 214763942aa2..9cb7bbfb4278 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -14,6 +14,7 @@
#include <linux/vmalloc.h>
#include "gt/intel_gt_requests.h"
+#include "gt/intel_gt.h"
#include "i915_trace.h"
@@ -119,7 +120,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
intel_wakeref_t wakeref = 0;
unsigned long count = 0;
unsigned long scanned = 0;
- int err = 0;
+ int err = 0, i = 0;
+ struct intel_gt *gt;
/* CHV + VTD workaround use stop_machine(); need to trylock vm->mutex */
bool trylock_vm = !ww && intel_vm_no_concurrent_access_wa(i915);
@@ -147,9 +149,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
* what we can do is give them a kick so that we do not keep idle
* contexts around longer than is necessary.
*/
- if (shrink & I915_SHRINK_ACTIVE)
- /* Retire requests to unpin all idle contexts */
- intel_gt_retire_requests(to_gt(i915));
+ if (shrink & I915_SHRINK_ACTIVE) {
+ for_each_gt(gt, i915, i)
+ /* Retire requests to unpin all idle contexts */
+ intel_gt_retire_requests(gt);
+ }
/*
* As we may completely rewrite the (un)bound list whilst unbinding
@@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
struct i915_vma *vma, *next;
unsigned long freed_pages = 0;
intel_wakeref_t wakeref;
+ struct intel_gt *gt;
+ int i;
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
@@ -397,24 +403,26 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
I915_SHRINK_VMAPS);
/* We also want to clear any cached iomaps as they wrap vmap */
- mutex_lock(&to_gt(i915)->ggtt->vm.mutex);
- list_for_each_entry_safe(vma, next,
- &to_gt(i915)->ggtt->vm.bound_list, vm_link) {
- unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
- struct drm_i915_gem_object *obj = vma->obj;
-
- if (!vma->iomap || i915_vma_is_active(vma))
- continue;
+ for_each_gt(gt, i915, i) {
+ mutex_lock(>->ggtt->vm.mutex);
+ list_for_each_entry_safe(vma, next,
+ >->ggtt->vm.bound_list, vm_link) {
+ unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
+ struct drm_i915_gem_object *obj = vma->obj;
+
+ if (!vma->iomap || i915_vma_is_active(vma))
+ continue;
- if (!i915_gem_object_trylock(obj, NULL))
- continue;
+ if (!i915_gem_object_trylock(obj, NULL))
+ continue;
- if (__i915_vma_unbind(vma) == 0)
- freed_pages += count;
+ if (__i915_vma_unbind(vma) == 0)
+ freed_pages += count;
- i915_gem_object_unlock(obj);
+ i915_gem_object_unlock(obj);
+ }
+ mutex_unlock(>->ggtt->vm.mutex);
}
- mutex_unlock(&to_gt(i915)->ggtt->vm.mutex);
*(unsigned long *)ptr += freed_pages;
return NOTIFY_DONE;
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
2023-09-25 13:49 ` Nirmoy Das
@ 2023-09-25 14:46 ` Andi Shyti
-1 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2023-09-25 14:46 UTC (permalink / raw)
To: Nirmoy Das; +Cc: intel-gfx, Jonathan Cavitt, dri-devel, Andrzej Hajda
Hi Nirmoy,
you forgot the v2 here.
On Mon, Sep 25, 2023 at 03:49:38PM +0200, Nirmoy Das wrote:
> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>
> Where applicable, use for_each_gt instead of to_gt in the
> i915_gem_shrinker functions to make them apply to more than just the
> primary GT. Specifically, this ensure i915_gem_shrink_all retires all
> requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
> VMAs from all GTs.
>
> v2: Pass correct GT to intel_gt_retire_requests(Andrzej).
>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
[...]
> - if (shrink & I915_SHRINK_ACTIVE)
> - /* Retire requests to unpin all idle contexts */
> - intel_gt_retire_requests(to_gt(i915));
> + if (shrink & I915_SHRINK_ACTIVE) {
> + for_each_gt(gt, i915, i)
> + /* Retire requests to unpin all idle contexts */
> + intel_gt_retire_requests(gt);
> + }
These two brackets are not needed.
>
> /*
> * As we may completely rewrite the (un)bound list whilst unbinding
> @@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
> struct i915_vma *vma, *next;
> unsigned long freed_pages = 0;
> intel_wakeref_t wakeref;
> + struct intel_gt *gt;
> + int i;
the trend is to use 'unsigned int' here and I've seen it
reviewed. Personally, if I really have to express a preference, I
prefer 'int' because it's a bit safer, generally I don't really
mind :)
The rest looks good.
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Andi
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
@ 2023-09-25 14:46 ` Andi Shyti
0 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2023-09-25 14:46 UTC (permalink / raw)
To: Nirmoy Das
Cc: intel-gfx, Jonathan Cavitt, andi.shyti, dri-devel, Andrzej Hajda
Hi Nirmoy,
you forgot the v2 here.
On Mon, Sep 25, 2023 at 03:49:38PM +0200, Nirmoy Das wrote:
> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>
> Where applicable, use for_each_gt instead of to_gt in the
> i915_gem_shrinker functions to make them apply to more than just the
> primary GT. Specifically, this ensure i915_gem_shrink_all retires all
> requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
> VMAs from all GTs.
>
> v2: Pass correct GT to intel_gt_retire_requests(Andrzej).
>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
[...]
> - if (shrink & I915_SHRINK_ACTIVE)
> - /* Retire requests to unpin all idle contexts */
> - intel_gt_retire_requests(to_gt(i915));
> + if (shrink & I915_SHRINK_ACTIVE) {
> + for_each_gt(gt, i915, i)
> + /* Retire requests to unpin all idle contexts */
> + intel_gt_retire_requests(gt);
> + }
These two brackets are not needed.
>
> /*
> * As we may completely rewrite the (un)bound list whilst unbinding
> @@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
> struct i915_vma *vma, *next;
> unsigned long freed_pages = 0;
> intel_wakeref_t wakeref;
> + struct intel_gt *gt;
> + int i;
the trend is to use 'unsigned int' here and I've seen it
reviewed. Personally, if I really have to express a preference, I
prefer 'int' because it's a bit safer, generally I don't really
mind :)
The rest looks good.
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Andi
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
2023-09-25 14:46 ` Andi Shyti
@ 2023-09-25 15:20 ` Jani Nikula
-1 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2023-09-25 15:20 UTC (permalink / raw)
To: Andi Shyti, Nirmoy Das
Cc: intel-gfx, Jonathan Cavitt, dri-devel, Andrzej Hajda
On Mon, 25 Sep 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> Hi Nirmoy,
>
> you forgot the v2 here.
>
> On Mon, Sep 25, 2023 at 03:49:38PM +0200, Nirmoy Das wrote:
>> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>
>> Where applicable, use for_each_gt instead of to_gt in the
>> i915_gem_shrinker functions to make them apply to more than just the
>> primary GT. Specifically, this ensure i915_gem_shrink_all retires all
>> requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
>> VMAs from all GTs.
>>
>> v2: Pass correct GT to intel_gt_retire_requests(Andrzej).
>>
>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
> [...]
>
>> - if (shrink & I915_SHRINK_ACTIVE)
>> - /* Retire requests to unpin all idle contexts */
>> - intel_gt_retire_requests(to_gt(i915));
>> + if (shrink & I915_SHRINK_ACTIVE) {
>> + for_each_gt(gt, i915, i)
>> + /* Retire requests to unpin all idle contexts */
>> + intel_gt_retire_requests(gt);
>> + }
>
> These two brackets are not needed.
>
>>
>> /*
>> * As we may completely rewrite the (un)bound list whilst unbinding
>> @@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
>> struct i915_vma *vma, *next;
>> unsigned long freed_pages = 0;
>> intel_wakeref_t wakeref;
>> + struct intel_gt *gt;
>> + int i;
>
> the trend is to use 'unsigned int' here and I've seen it
> reviewed. Personally, if I really have to express a preference, I
> prefer 'int' because it's a bit safer, generally I don't really
> mind :)
Always use int over unsigned int if you don't have a specific reason not
to. ("It can't be negative" is not a good reason.)
BR,
Jani.
>
> The rest looks good.
>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>
> Andi
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
@ 2023-09-25 15:20 ` Jani Nikula
0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2023-09-25 15:20 UTC (permalink / raw)
To: Andi Shyti, Nirmoy Das
Cc: intel-gfx, Jonathan Cavitt, dri-devel, andi.shyti, Andrzej Hajda
On Mon, 25 Sep 2023, Andi Shyti <andi.shyti@linux.intel.com> wrote:
> Hi Nirmoy,
>
> you forgot the v2 here.
>
> On Mon, Sep 25, 2023 at 03:49:38PM +0200, Nirmoy Das wrote:
>> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>
>> Where applicable, use for_each_gt instead of to_gt in the
>> i915_gem_shrinker functions to make them apply to more than just the
>> primary GT. Specifically, this ensure i915_gem_shrink_all retires all
>> requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
>> VMAs from all GTs.
>>
>> v2: Pass correct GT to intel_gt_retire_requests(Andrzej).
>>
>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
> [...]
>
>> - if (shrink & I915_SHRINK_ACTIVE)
>> - /* Retire requests to unpin all idle contexts */
>> - intel_gt_retire_requests(to_gt(i915));
>> + if (shrink & I915_SHRINK_ACTIVE) {
>> + for_each_gt(gt, i915, i)
>> + /* Retire requests to unpin all idle contexts */
>> + intel_gt_retire_requests(gt);
>> + }
>
> These two brackets are not needed.
>
>>
>> /*
>> * As we may completely rewrite the (un)bound list whilst unbinding
>> @@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
>> struct i915_vma *vma, *next;
>> unsigned long freed_pages = 0;
>> intel_wakeref_t wakeref;
>> + struct intel_gt *gt;
>> + int i;
>
> the trend is to use 'unsigned int' here and I've seen it
> reviewed. Personally, if I really have to express a preference, I
> prefer 'int' because it's a bit safer, generally I don't really
> mind :)
Always use int over unsigned int if you don't have a specific reason not
to. ("It can't be negative" is not a good reason.)
BR,
Jani.
>
> The rest looks good.
>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
>
> Andi
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
2023-09-25 15:20 ` Jani Nikula
@ 2023-09-25 15:24 ` Andi Shyti
-1 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2023-09-25 15:24 UTC (permalink / raw)
To: Jani Nikula
Cc: intel-gfx, Jonathan Cavitt, dri-devel, Andrzej Hajda, Nirmoy Das
Hi Jani,
> >> struct i915_vma *vma, *next;
> >> unsigned long freed_pages = 0;
> >> intel_wakeref_t wakeref;
> >> + struct intel_gt *gt;
> >> + int i;
> >
> > the trend is to use 'unsigned int' here and I've seen it
> > reviewed. Personally, if I really have to express a preference, I
> > prefer 'int' because it's a bit safer, generally I don't really
> > mind :)
>
> Always use int over unsigned int if you don't have a specific reason not
> to. ("It can't be negative" is not a good reason.)
Finally someone! I totally agree!
Andi
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
@ 2023-09-25 15:24 ` Andi Shyti
0 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2023-09-25 15:24 UTC (permalink / raw)
To: Jani Nikula
Cc: Andi Shyti, intel-gfx, Jonathan Cavitt, dri-devel, Andrzej Hajda,
Nirmoy Das
Hi Jani,
> >> struct i915_vma *vma, *next;
> >> unsigned long freed_pages = 0;
> >> intel_wakeref_t wakeref;
> >> + struct intel_gt *gt;
> >> + int i;
> >
> > the trend is to use 'unsigned int' here and I've seen it
> > reviewed. Personally, if I really have to express a preference, I
> > prefer 'int' because it's a bit safer, generally I don't really
> > mind :)
>
> Always use int over unsigned int if you don't have a specific reason not
> to. ("It can't be negative" is not a good reason.)
Finally someone! I totally agree!
Andi
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
@ 2023-09-22 12:35 Nirmoy Das
2023-09-25 13:23 ` Andrzej Hajda
0 siblings, 1 reply; 11+ messages in thread
From: Nirmoy Das @ 2023-09-22 12:35 UTC (permalink / raw)
To: intel-gfx; +Cc: andrzej.hajda, Jonathan Cavitt, dri-devel, Nirmoy Das
From: Jonathan Cavitt <jonathan.cavitt@intel.com>
Where applicable, use for_each_gt instead of to_gt in the
i915_gem_shrinker functions to make them apply to more than just the
primary GT. Specifically, this ensure i915_gem_shrink_all retires all
requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
VMAs from all GTs.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 44 ++++++++++++--------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 214763942aa2..3ef1fd32f80a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -14,6 +14,7 @@
#include <linux/vmalloc.h>
#include "gt/intel_gt_requests.h"
+#include "gt/intel_gt.h"
#include "i915_trace.h"
@@ -119,7 +120,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
intel_wakeref_t wakeref = 0;
unsigned long count = 0;
unsigned long scanned = 0;
- int err = 0;
+ int err = 0, i = 0;
+ struct intel_gt *gt;
/* CHV + VTD workaround use stop_machine(); need to trylock vm->mutex */
bool trylock_vm = !ww && intel_vm_no_concurrent_access_wa(i915);
@@ -147,9 +149,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
* what we can do is give them a kick so that we do not keep idle
* contexts around longer than is necessary.
*/
- if (shrink & I915_SHRINK_ACTIVE)
- /* Retire requests to unpin all idle contexts */
- intel_gt_retire_requests(to_gt(i915));
+ if (shrink & I915_SHRINK_ACTIVE) {
+ for_each_gt(gt, i915, i)
+ /* Retire requests to unpin all idle contexts */
+ intel_gt_retire_requests(to_gt(i915));
+ }
/*
* As we may completely rewrite the (un)bound list whilst unbinding
@@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
struct i915_vma *vma, *next;
unsigned long freed_pages = 0;
intel_wakeref_t wakeref;
+ struct intel_gt *gt;
+ int i;
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
@@ -397,24 +403,26 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
I915_SHRINK_VMAPS);
/* We also want to clear any cached iomaps as they wrap vmap */
- mutex_lock(&to_gt(i915)->ggtt->vm.mutex);
- list_for_each_entry_safe(vma, next,
- &to_gt(i915)->ggtt->vm.bound_list, vm_link) {
- unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
- struct drm_i915_gem_object *obj = vma->obj;
-
- if (!vma->iomap || i915_vma_is_active(vma))
- continue;
+ for_each_gt(gt, i915, i) {
+ mutex_lock(>->ggtt->vm.mutex);
+ list_for_each_entry_safe(vma, next,
+ >->ggtt->vm.bound_list, vm_link) {
+ unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
+ struct drm_i915_gem_object *obj = vma->obj;
+
+ if (!vma->iomap || i915_vma_is_active(vma))
+ continue;
- if (!i915_gem_object_trylock(obj, NULL))
- continue;
+ if (!i915_gem_object_trylock(obj, NULL))
+ continue;
- if (__i915_vma_unbind(vma) == 0)
- freed_pages += count;
+ if (__i915_vma_unbind(vma) == 0)
+ freed_pages += count;
- i915_gem_object_unlock(obj);
+ i915_gem_object_unlock(obj);
+ }
+ mutex_unlock(>->ggtt->vm.mutex);
}
- mutex_unlock(&to_gt(i915)->ggtt->vm.mutex);
*(unsigned long *)ptr += freed_pages;
return NOTIFY_DONE;
--
2.41.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
2023-09-22 12:35 [Intel-gfx] " Nirmoy Das
@ 2023-09-25 13:23 ` Andrzej Hajda
2023-09-25 13:32 ` Nirmoy Das
0 siblings, 1 reply; 11+ messages in thread
From: Andrzej Hajda @ 2023-09-25 13:23 UTC (permalink / raw)
To: Nirmoy Das, intel-gfx; +Cc: Jonathan Cavitt, dri-devel
On 22.09.2023 14:35, Nirmoy Das wrote:
> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>
> Where applicable, use for_each_gt instead of to_gt in the
> i915_gem_shrinker functions to make them apply to more than just the
> primary GT. Specifically, this ensure i915_gem_shrink_all retires all
> requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
> VMAs from all GTs.
>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 44 ++++++++++++--------
> 1 file changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index 214763942aa2..3ef1fd32f80a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -14,6 +14,7 @@
> #include <linux/vmalloc.h>
>
> #include "gt/intel_gt_requests.h"
> +#include "gt/intel_gt.h"
>
> #include "i915_trace.h"
>
> @@ -119,7 +120,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
> intel_wakeref_t wakeref = 0;
> unsigned long count = 0;
> unsigned long scanned = 0;
> - int err = 0;
> + int err = 0, i = 0;
> + struct intel_gt *gt;
>
> /* CHV + VTD workaround use stop_machine(); need to trylock vm->mutex */
> bool trylock_vm = !ww && intel_vm_no_concurrent_access_wa(i915);
> @@ -147,9 +149,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
> * what we can do is give them a kick so that we do not keep idle
> * contexts around longer than is necessary.
> */
> - if (shrink & I915_SHRINK_ACTIVE)
> - /* Retire requests to unpin all idle contexts */
> - intel_gt_retire_requests(to_gt(i915));
> + if (shrink & I915_SHRINK_ACTIVE) {
> + for_each_gt(gt, i915, i)
> + /* Retire requests to unpin all idle contexts */
> + intel_gt_retire_requests(to_gt(i915));
to_gt(...) -> gt ?
> + }
>
> /*
> * As we may completely rewrite the (un)bound list whilst unbinding
> @@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
> struct i915_vma *vma, *next;
> unsigned long freed_pages = 0;
> intel_wakeref_t wakeref;
> + struct intel_gt *gt;
> + int i;
>
> with_intel_runtime_pm(&i915->runtime_pm, wakeref)
> freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
> @@ -397,24 +403,26 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
> I915_SHRINK_VMAPS);
>
> /* We also want to clear any cached iomaps as they wrap vmap */
> - mutex_lock(&to_gt(i915)->ggtt->vm.mutex);
> - list_for_each_entry_safe(vma, next,
> - &to_gt(i915)->ggtt->vm.bound_list, vm_link) {
> - unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
> - struct drm_i915_gem_object *obj = vma->obj;
> -
> - if (!vma->iomap || i915_vma_is_active(vma))
> - continue;
> + for_each_gt(gt, i915, i) {
> + mutex_lock(>->ggtt->vm.mutex);
> + list_for_each_entry_safe(vma, next,
> + >->ggtt->vm.bound_list, vm_link) {
> + unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
> + struct drm_i915_gem_object *obj = vma->obj;
> +
> + if (!vma->iomap || i915_vma_is_active(vma))
> + continue;
>
> - if (!i915_gem_object_trylock(obj, NULL))
> - continue;
> + if (!i915_gem_object_trylock(obj, NULL))
> + continue;
>
> - if (__i915_vma_unbind(vma) == 0)
> - freed_pages += count;
> + if (__i915_vma_unbind(vma) == 0)
> + freed_pages += count;
>
> - i915_gem_object_unlock(obj);
> + i915_gem_object_unlock(obj);
> + }
> + mutex_unlock(>->ggtt->vm.mutex);
> }
> - mutex_unlock(&to_gt(i915)->ggtt->vm.mutex);
This seems correct.
With 1st stanza fixed:
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Regards
Andrzej
>
> *(unsigned long *)ptr += freed_pages;
> return NOTIFY_DONE;
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware
2023-09-25 13:23 ` Andrzej Hajda
@ 2023-09-25 13:32 ` Nirmoy Das
0 siblings, 0 replies; 11+ messages in thread
From: Nirmoy Das @ 2023-09-25 13:32 UTC (permalink / raw)
To: Andrzej Hajda, Nirmoy Das, intel-gfx; +Cc: Jonathan Cavitt, dri-devel
On 9/25/2023 3:23 PM, Andrzej Hajda wrote:
> On 22.09.2023 14:35, Nirmoy Das wrote:
>> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>
>> Where applicable, use for_each_gt instead of to_gt in the
>> i915_gem_shrinker functions to make them apply to more than just the
>> primary GT. Specifically, this ensure i915_gem_shrink_all retires all
>> requests across all GTs, and this makes i915_gem_shrinker_vmap unmap
>> VMAs from all GTs.
>>
>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 44 ++++++++++++--------
>> 1 file changed, 26 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> index 214763942aa2..3ef1fd32f80a 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> @@ -14,6 +14,7 @@
>> #include <linux/vmalloc.h>
>> #include "gt/intel_gt_requests.h"
>> +#include "gt/intel_gt.h"
>> #include "i915_trace.h"
>> @@ -119,7 +120,8 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
>> intel_wakeref_t wakeref = 0;
>> unsigned long count = 0;
>> unsigned long scanned = 0;
>> - int err = 0;
>> + int err = 0, i = 0;
>> + struct intel_gt *gt;
>> /* CHV + VTD workaround use stop_machine(); need to trylock
>> vm->mutex */
>> bool trylock_vm = !ww && intel_vm_no_concurrent_access_wa(i915);
>> @@ -147,9 +149,11 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
>> * what we can do is give them a kick so that we do not keep idle
>> * contexts around longer than is necessary.
>> */
>> - if (shrink & I915_SHRINK_ACTIVE)
>> - /* Retire requests to unpin all idle contexts */
>> - intel_gt_retire_requests(to_gt(i915));
>> + if (shrink & I915_SHRINK_ACTIVE) {
>> + for_each_gt(gt, i915, i)
>> + /* Retire requests to unpin all idle contexts */
>> + intel_gt_retire_requests(to_gt(i915));
>
>
> to_gt(...) -> gt ?
Wow, a huge miss. Thanks will resend!
>
>
>> + }
>> /*
>> * As we may completely rewrite the (un)bound list whilst
>> unbinding
>> @@ -389,6 +393,8 @@ i915_gem_shrinker_vmap(struct notifier_block *nb,
>> unsigned long event, void *ptr
>> struct i915_vma *vma, *next;
>> unsigned long freed_pages = 0;
>> intel_wakeref_t wakeref;
>> + struct intel_gt *gt;
>> + int i;
>> with_intel_runtime_pm(&i915->runtime_pm, wakeref)
>> freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
>> @@ -397,24 +403,26 @@ i915_gem_shrinker_vmap(struct notifier_block
>> *nb, unsigned long event, void *ptr
>> I915_SHRINK_VMAPS);
>> /* We also want to clear any cached iomaps as they wrap vmap */
>> - mutex_lock(&to_gt(i915)->ggtt->vm.mutex);
>> - list_for_each_entry_safe(vma, next,
>> - &to_gt(i915)->ggtt->vm.bound_list, vm_link) {
>> - unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
>> - struct drm_i915_gem_object *obj = vma->obj;
>> -
>> - if (!vma->iomap || i915_vma_is_active(vma))
>> - continue;
>> + for_each_gt(gt, i915, i) {
>> + mutex_lock(>->ggtt->vm.mutex);
>> + list_for_each_entry_safe(vma, next,
>> + >->ggtt->vm.bound_list, vm_link) {
>> + unsigned long count = i915_vma_size(vma) >> PAGE_SHIFT;
>> + struct drm_i915_gem_object *obj = vma->obj;
>> +
>> + if (!vma->iomap || i915_vma_is_active(vma))
>> + continue;
>> - if (!i915_gem_object_trylock(obj, NULL))
>> - continue;
>> + if (!i915_gem_object_trylock(obj, NULL))
>> + continue;
>> - if (__i915_vma_unbind(vma) == 0)
>> - freed_pages += count;
>> + if (__i915_vma_unbind(vma) == 0)
>> + freed_pages += count;
>> - i915_gem_object_unlock(obj);
>> + i915_gem_object_unlock(obj);
>> + }
>> + mutex_unlock(>->ggtt->vm.mutex);
>> }
>> - mutex_unlock(&to_gt(i915)->ggtt->vm.mutex);
>
>
> This seems correct.
>
> With 1st stanza fixed:
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Thanks,
Nirmoy
>
> Regards
> Andrzej
>
>
>> *(unsigned long *)ptr += freed_pages;
>> return NOTIFY_DONE;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-25 15:24 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-25 13:49 [Intel-gfx] [PATCH] drm/i915/gem: Make i915_gem_shrinker multi-gt aware Nirmoy Das
2023-09-25 13:49 ` Nirmoy Das
2023-09-25 14:46 ` [Intel-gfx] " Andi Shyti
2023-09-25 14:46 ` Andi Shyti
2023-09-25 15:20 ` [Intel-gfx] " Jani Nikula
2023-09-25 15:20 ` Jani Nikula
2023-09-25 15:24 ` [Intel-gfx] " Andi Shyti
2023-09-25 15:24 ` Andi Shyti
-- strict thread matches above, loose matches on Subject: below --
2023-09-22 12:35 [Intel-gfx] " Nirmoy Das
2023-09-25 13:23 ` Andrzej Hajda
2023-09-25 13:32 ` Nirmoy Das
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.