* [PATCH] drm/i915/dpt: Make DPT object unshrinkable [not found] <20240520152410.1098393-1-vidya.srinivas@intel.com> @ 2024-05-20 16:56 ` Vidya Srinivas 2024-05-22 15:29 ` Vidya Srinivas 0 siblings, 1 reply; 11+ messages in thread From: Vidya Srinivas @ 2024-05-20 16:56 UTC (permalink / raw) To: intel-gfx; +Cc: ville.syrjala, shawn.c.lee, Vidya Srinivas, stable In some scenarios, the DPT object gets shrunk but the actual framebuffer did not and thus its still there on the DPT's vm->bound_list. Then it tries to rewrite the PTEs via a stale CPU mapping. This causes panic. Credits-to: Ville Syrjala <ville.syrjala@linux.intel.com> Shawn Lee <shawn.c.lee@intel.com> Cc: stable@vger.kernel.org Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 3560a062d287..e6b485fc54d4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); static inline bool i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) { - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && + !obj->is_dpt; } static inline bool -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-20 16:56 ` [PATCH] drm/i915/dpt: Make DPT object unshrinkable Vidya Srinivas @ 2024-05-22 15:29 ` Vidya Srinivas 2024-05-23 8:25 ` Tvrtko Ursulin 0 siblings, 1 reply; 11+ messages in thread From: Vidya Srinivas @ 2024-05-22 15:29 UTC (permalink / raw) To: intel-gfx; +Cc: shawn.c.lee, Vidya Srinivas, Ville Syrjala, stable In some scenarios, the DPT object gets shrunk but the actual framebuffer did not and thus its still there on the DPT's vm->bound_list. Then it tries to rewrite the PTEs via a stale CPU mapping. This causes panic. Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: stable@vger.kernel.org Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 3560a062d287..e6b485fc54d4 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); static inline bool i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) { - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && + !obj->is_dpt; } static inline bool -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-22 15:29 ` Vidya Srinivas @ 2024-05-23 8:25 ` Tvrtko Ursulin 2024-05-23 11:19 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Tvrtko Ursulin @ 2024-05-23 8:25 UTC (permalink / raw) To: Vidya Srinivas, intel-gfx; +Cc: shawn.c.lee, Ville Syrjala, stable On 22/05/2024 16:29, Vidya Srinivas wrote: > In some scenarios, the DPT object gets shrunk but > the actual framebuffer did not and thus its still > there on the DPT's vm->bound_list. Then it tries to > rewrite the PTEs via a stale CPU mapping. This causes panic. > > Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: stable@vger.kernel.org > Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > index 3560a062d287..e6b485fc54d4 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); > static inline bool > i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) > { > - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); > + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && > + !obj->is_dpt; Is there a reason i915_gem_object_make_unshrinkable() cannot be used to mark the object at a suitable place? Regards, Tvrtko > } > > static inline bool ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-23 8:25 ` Tvrtko Ursulin @ 2024-05-23 11:19 ` Ville Syrjälä 2024-05-23 12:07 ` Tvrtko Ursulin 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2024-05-23 11:19 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Vidya Srinivas, intel-gfx, shawn.c.lee, stable On Thu, May 23, 2024 at 09:25:45AM +0100, Tvrtko Ursulin wrote: > > On 22/05/2024 16:29, Vidya Srinivas wrote: > > In some scenarios, the DPT object gets shrunk but > > the actual framebuffer did not and thus its still > > there on the DPT's vm->bound_list. Then it tries to > > rewrite the PTEs via a stale CPU mapping. This causes panic. > > > > Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: stable@vger.kernel.org > > Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") > > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > > --- > > drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > index 3560a062d287..e6b485fc54d4 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > > @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); > > static inline bool > > i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) > > { > > - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); > > + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && > > + !obj->is_dpt; > > Is there a reason i915_gem_object_make_unshrinkable() cannot be used to > mark the object at a suitable place? Do you have a suitable place in mind? i915_gem_object_make_unshrinkable() contains some magic ingredients so doesn't look like it can be called willy nilly. Anyways, looks like I forgot to reply that I already pushed this with this extra comment added: /* TODO: make DPT shrinkable when it has no bound vmas */ -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-23 11:19 ` Ville Syrjälä @ 2024-05-23 12:07 ` Tvrtko Ursulin 2024-05-23 12:24 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Tvrtko Ursulin @ 2024-05-23 12:07 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Vidya Srinivas, intel-gfx, shawn.c.lee, stable On 23/05/2024 12:19, Ville Syrjälä wrote: > On Thu, May 23, 2024 at 09:25:45AM +0100, Tvrtko Ursulin wrote: >> >> On 22/05/2024 16:29, Vidya Srinivas wrote: >>> In some scenarios, the DPT object gets shrunk but >>> the actual framebuffer did not and thus its still >>> there on the DPT's vm->bound_list. Then it tries to >>> rewrite the PTEs via a stale CPU mapping. This causes panic. >>> >>> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> >>> Cc: stable@vger.kernel.org >>> Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> >>> --- >>> drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h >>> index 3560a062d287..e6b485fc54d4 100644 >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h >>> @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); >>> static inline bool >>> i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) >>> { >>> - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); >>> + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && >>> + !obj->is_dpt; >> >> Is there a reason i915_gem_object_make_unshrinkable() cannot be used to >> mark the object at a suitable place? > > Do you have a suitable place in mind? > i915_gem_object_make_unshrinkable() contains some magic > ingredients so doesn't look like it can be called willy > nilly. After it is created in intel_dpt_create? I don't see that helper couldn't be called. It is called from madvise and tiling for instance without any apparent special considerations. Also, there is no mention of this angle in the commit message so I assumed it wasn't considered. If it was, then it should have been mentioned why hacky solution was chosen instead... > Anyways, looks like I forgot to reply that I already pushed this > with this extra comment added: > /* TODO: make DPT shrinkable when it has no bound vmas */ ... becuase IMO the special case is quite ugly and out of place. :( I don't remember from the top of my head how DPT magic works but if shrinker protection needs to be tied with VMAs there is also i915_make_make(un)shrinkable to try. Regards, Tvrtko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-23 12:07 ` Tvrtko Ursulin @ 2024-05-23 12:24 ` Ville Syrjälä 2024-05-23 13:14 ` Tvrtko Ursulin 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2024-05-23 12:24 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Vidya Srinivas, intel-gfx, shawn.c.lee, stable On Thu, May 23, 2024 at 01:07:24PM +0100, Tvrtko Ursulin wrote: > > On 23/05/2024 12:19, Ville Syrjälä wrote: > > On Thu, May 23, 2024 at 09:25:45AM +0100, Tvrtko Ursulin wrote: > >> > >> On 22/05/2024 16:29, Vidya Srinivas wrote: > >>> In some scenarios, the DPT object gets shrunk but > >>> the actual framebuffer did not and thus its still > >>> there on the DPT's vm->bound_list. Then it tries to > >>> rewrite the PTEs via a stale CPU mapping. This causes panic. > >>> > >>> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> > >>> Cc: stable@vger.kernel.org > >>> Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") > >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > >>> index 3560a062d287..e6b485fc54d4 100644 > >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > >>> @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); > >>> static inline bool > >>> i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) > >>> { > >>> - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); > >>> + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && > >>> + !obj->is_dpt; > >> > >> Is there a reason i915_gem_object_make_unshrinkable() cannot be used to > >> mark the object at a suitable place? > > > > Do you have a suitable place in mind? > > i915_gem_object_make_unshrinkable() contains some magic > > ingredients so doesn't look like it can be called willy > > nilly. > > After it is created in intel_dpt_create? > > I don't see that helper couldn't be called. It is called from madvise > and tiling for instance without any apparent special considerations. Did you actually read through i915_gem_object_make_unshrinkable()? > > Also, there is no mention of this angle in the commit message so I > assumed it wasn't considered. If it was, then it should have been > mentioned why hacky solution was chosen instead... I suppose. > > > Anyways, looks like I forgot to reply that I already pushed this > > with this extra comment added: > > /* TODO: make DPT shrinkable when it has no bound vmas */ > > ... becuase IMO the special case is quite ugly and out of place. :( Yeah, not the nicest. But there's already a is_dpt check in the i915_gem_object_is_framebuffer() right next door, so it's not *that* out of place. Another option maybe could be to manually clear I915_GEM_OBJECT_IS_SHRINKABLE but I don't think that is supposed to be mutable, so might also have other issues. So a more proper solution with that approach would perhaps need some kind of gem_create_shmem_unshrinkable() function. > > I don't remember from the top of my head how DPT magic works but if > shrinker protection needs to be tied with VMAs there is also > i915_make_make(un)shrinkable to try. I presume you mistyped something there. -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-23 12:24 ` Ville Syrjälä @ 2024-05-23 13:14 ` Tvrtko Ursulin 2024-05-23 13:52 ` Ville Syrjälä 0 siblings, 1 reply; 11+ messages in thread From: Tvrtko Ursulin @ 2024-05-23 13:14 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Vidya Srinivas, intel-gfx, shawn.c.lee, stable On 23/05/2024 13:24, Ville Syrjälä wrote: > On Thu, May 23, 2024 at 01:07:24PM +0100, Tvrtko Ursulin wrote: >> >> On 23/05/2024 12:19, Ville Syrjälä wrote: >>> On Thu, May 23, 2024 at 09:25:45AM +0100, Tvrtko Ursulin wrote: >>>> >>>> On 22/05/2024 16:29, Vidya Srinivas wrote: >>>>> In some scenarios, the DPT object gets shrunk but >>>>> the actual framebuffer did not and thus its still >>>>> there on the DPT's vm->bound_list. Then it tries to >>>>> rewrite the PTEs via a stale CPU mapping. This causes panic. >>>>> >>>>> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> >>>>> Cc: stable@vger.kernel.org >>>>> Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") >>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> >>>>> --- >>>>> drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h >>>>> index 3560a062d287..e6b485fc54d4 100644 >>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h >>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h >>>>> @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); >>>>> static inline bool >>>>> i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) >>>>> { >>>>> - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); >>>>> + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && >>>>> + !obj->is_dpt; >>>> >>>> Is there a reason i915_gem_object_make_unshrinkable() cannot be used to >>>> mark the object at a suitable place? >>> >>> Do you have a suitable place in mind? >>> i915_gem_object_make_unshrinkable() contains some magic >>> ingredients so doesn't look like it can be called willy >>> nilly. >> >> After it is created in intel_dpt_create? >> >> I don't see that helper couldn't be called. It is called from madvise >> and tiling for instance without any apparent special considerations. > > Did you actually read through i915_gem_object_make_unshrinkable()? Briefly, and also looked around how it is used. I don't immediately understand which part concerns you and it is also quite possible I am missing something. But see for example how it is used in intel_context.c+intel_lrc.c to protect the context state object from the shrinker while it is in use by the GPU. It does not appear any black magic is required. Question also is does that kind of lifetime aligns with the DPT use case. >> Also, there is no mention of this angle in the commit message so I >> assumed it wasn't considered. If it was, then it should have been >> mentioned why hacky solution was chosen instead... > > I suppose. > >> >>> Anyways, looks like I forgot to reply that I already pushed this >>> with this extra comment added: >>> /* TODO: make DPT shrinkable when it has no bound vmas */ >> >> ... becuase IMO the special case is quite ugly and out of place. :( > > Yeah, not the nicest. But there's already a is_dpt check in the > i915_gem_object_is_framebuffer() right next door, so it's not > *that* out of place. I also see who added that one! ;) > Another option maybe could be to manually clear > I915_GEM_OBJECT_IS_SHRINKABLE but I don't think that is > supposed to be mutable, so might also have other issues. > So a more proper solution with that approach would perhaps > need some kind of gem_create_shmem_unshrinkable() function. > >> >> I don't remember from the top of my head how DPT magic works but if >> shrinker protection needs to be tied with VMAs there is also >> i915_make_make(un)shrinkable to try. > > I presume you mistyped something there. Oops - i915_vma_make_(un)shrinkable. Anyway, I think it is worth giving it a try if the DPT lifetimes makes it possible. Regards, Tvrtko ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-23 13:14 ` Tvrtko Ursulin @ 2024-05-23 13:52 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2024-05-23 13:52 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Vidya Srinivas, intel-gfx, shawn.c.lee, stable On Thu, May 23, 2024 at 02:14:56PM +0100, Tvrtko Ursulin wrote: > > On 23/05/2024 13:24, Ville Syrjälä wrote: > > On Thu, May 23, 2024 at 01:07:24PM +0100, Tvrtko Ursulin wrote: > >> > >> On 23/05/2024 12:19, Ville Syrjälä wrote: > >>> On Thu, May 23, 2024 at 09:25:45AM +0100, Tvrtko Ursulin wrote: > >>>> > >>>> On 22/05/2024 16:29, Vidya Srinivas wrote: > >>>>> In some scenarios, the DPT object gets shrunk but > >>>>> the actual framebuffer did not and thus its still > >>>>> there on the DPT's vm->bound_list. Then it tries to > >>>>> rewrite the PTEs via a stale CPU mapping. This causes panic. > >>>>> > >>>>> Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com> > >>>>> Cc: stable@vger.kernel.org > >>>>> Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt") > >>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > >>>>> --- > >>>>> drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++- > >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > >>>>> index 3560a062d287..e6b485fc54d4 100644 > >>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > >>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > >>>>> @@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj); > >>>>> static inline bool > >>>>> i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) > >>>>> { > >>>>> - return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE); > >>>>> + return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) && > >>>>> + !obj->is_dpt; > >>>> > >>>> Is there a reason i915_gem_object_make_unshrinkable() cannot be used to > >>>> mark the object at a suitable place? > >>> > >>> Do you have a suitable place in mind? > >>> i915_gem_object_make_unshrinkable() contains some magic > >>> ingredients so doesn't look like it can be called willy > >>> nilly. > >> > >> After it is created in intel_dpt_create? > >> > >> I don't see that helper couldn't be called. It is called from madvise > >> and tiling for instance without any apparent special considerations. > > > > Did you actually read through i915_gem_object_make_unshrinkable()? > > Briefly, and also looked around how it is used. I don't immediately > understand which part concerns you and it is also quite possible I am > missing something. The shrink_pin magic says you can't use this willy nilly. > > But see for example how it is used in intel_context.c+intel_lrc.c to > protect the context state object from the shrinker while it is in use by > the GPU. It does not appear any black magic is required. > > Question also is does that kind of lifetime aligns with the DPT use case. > > >> Also, there is no mention of this angle in the commit message so I > >> assumed it wasn't considered. If it was, then it should have been > >> mentioned why hacky solution was chosen instead... > > > > I suppose. > > > >> > >>> Anyways, looks like I forgot to reply that I already pushed this > >>> with this extra comment added: > >>> /* TODO: make DPT shrinkable when it has no bound vmas */ > >> > >> ... becuase IMO the special case is quite ugly and out of place. :( > > > > Yeah, not the nicest. But there's already a is_dpt check in the > > i915_gem_object_is_framebuffer() right next door, so it's not > > *that* out of place. > > I also see who added that one! ;) > > > Another option maybe could be to manually clear > > I915_GEM_OBJECT_IS_SHRINKABLE but I don't think that is > > supposed to be mutable, so might also have other issues. > > So a more proper solution with that approach would perhaps > > need some kind of gem_create_shmem_unshrinkable() function. > > > >> > >> I don't remember from the top of my head how DPT magic works but if > >> shrinker protection needs to be tied with VMAs there is also > >> i915_make_make(un)shrinkable to try. > > > > I presume you mistyped something there. > > Oops - i915_vma_make_(un)shrinkable. That just calls the obj version of the function. > > Anyway, I think it is worth giving it a try if the DPT lifetimes makes > it possible. > > Regards, > > Tvrtko -- Ville Syrjälä Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/i915/dpt: Make DPT object unshrinkable
@ 2024-05-20 16:50 Vidya Srinivas
2024-05-20 18:19 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Vidya Srinivas @ 2024-05-20 16:50 UTC (permalink / raw)
To: vidya.srinivas; +Cc: stable
In some scenarios, the DPT object gets shrunk but
the actual framebuffer did not and thus its still
there on the DPT's vm->bound_list. Then it tries to
rewrite the PTEs via a stale CPU mapping. This causes panic.
Credits-to: Ville Syrjala <ville.syrjala@linux.intel.com>
Shawn Lee <shawn.c.lee@intel.com>
Cc: stable@vger.kernel.org
Fixes: 0dc987b699ce ("drm/i915/display: Add smem fallback allocation for dpt")
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 3560a062d287..e6b485fc54d4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -284,7 +284,8 @@ bool i915_gem_object_has_iomem(const struct drm_i915_gem_object *obj);
static inline bool
i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj)
{
- return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE);
+ return i915_gem_object_type_has(obj, I915_GEM_OBJECT_IS_SHRINKABLE) &&
+ !obj->is_dpt;
}
static inline bool
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-20 16:50 Vidya Srinivas @ 2024-05-20 18:19 ` Greg KH 2024-05-21 2:34 ` Srinivas, Vidya 0 siblings, 1 reply; 11+ messages in thread From: Greg KH @ 2024-05-20 18:19 UTC (permalink / raw) To: Vidya Srinivas; +Cc: stable On Mon, May 20, 2024 at 10:20:05PM +0530, Vidya Srinivas wrote: > In some scenarios, the DPT object gets shrunk but > the actual framebuffer did not and thus its still > there on the DPT's vm->bound_list. Then it tries to > rewrite the PTEs via a stale CPU mapping. This causes panic. > > Credits-to: Ville Syrjala <ville.syrjala@linux.intel.com> > Shawn Lee <shawn.c.lee@intel.com> Isn't that what "Co-developed-by:" is for, or "Suggested-by:"? I haven't seen "Credits-to:" before, where is that documented? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] drm/i915/dpt: Make DPT object unshrinkable 2024-05-20 18:19 ` Greg KH @ 2024-05-21 2:34 ` Srinivas, Vidya 0 siblings, 0 replies; 11+ messages in thread From: Srinivas, Vidya @ 2024-05-21 2:34 UTC (permalink / raw) To: Greg KH; +Cc: stable@vger.kernel.org > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Monday, May 20, 2024 11:50 PM > To: Srinivas, Vidya <vidya.srinivas@intel.com> > Cc: stable@vger.kernel.org > Subject: Re: [PATCH] drm/i915/dpt: Make DPT object unshrinkable > > On Mon, May 20, 2024 at 10:20:05PM +0530, Vidya Srinivas wrote: > > In some scenarios, the DPT object gets shrunk but the actual > > framebuffer did not and thus its still there on the DPT's > > vm->bound_list. Then it tries to rewrite the PTEs via a stale CPU > > mapping. This causes panic. > > > > Credits-to: Ville Syrjala <ville.syrjala@linux.intel.com> > > Shawn Lee <shawn.c.lee@intel.com> > > Isn't that what "Co-developed-by:" is for, or "Suggested-by:"? > > I haven't seen "Credits-to:" before, where is that documented? Hello Greg, thank you. Sorry, I had seen in some of the patches Example https://patchwork.freedesktop.org/patch/404900/ I will change it to Suggested-by. Regards Vidya > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-05-23 13:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240520152410.1098393-1-vidya.srinivas@intel.com>
2024-05-20 16:56 ` [PATCH] drm/i915/dpt: Make DPT object unshrinkable Vidya Srinivas
2024-05-22 15:29 ` Vidya Srinivas
2024-05-23 8:25 ` Tvrtko Ursulin
2024-05-23 11:19 ` Ville Syrjälä
2024-05-23 12:07 ` Tvrtko Ursulin
2024-05-23 12:24 ` Ville Syrjälä
2024-05-23 13:14 ` Tvrtko Ursulin
2024-05-23 13:52 ` Ville Syrjälä
2024-05-20 16:50 Vidya Srinivas
2024-05-20 18:19 ` Greg KH
2024-05-21 2:34 ` Srinivas, Vidya
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox