[SRU] [Groovy] [PATCH 0/1] Fix noise lines on i915 + Xorg

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [Groovy] [PATCH 0/1] Fix noise lines on i915 + Xorg

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1896091

[Impact]
Noise-like lines of graphics corruption when moving windows in Xorg
sessions.

[Fix]
Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
callbacks".

[Test]
Daniel van Vugt confirmed reverting the commit fixes the issue.
I can also reproduce the issue on one of my system, I can confirm it
fixes the issue too.

[Where problems could occur]
Race condition the commit solves may appear again.
However it was to solve the issue on 5.4, and the race could already be
solved on 5.8.

Kai-Heng Feng (1):
  Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
    callbacks"

 drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
 drivers/gpu/drm/i915/i915_active.h       | 10 +++--
 drivers/gpu/drm/i915/i915_active_types.h |  2 -
 3 files changed, 14 insertions(+), 50 deletions(-)

--
2.28.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1] Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire callbacks"

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1896091

This reverts commit c1484c993957fe6b366ab453fbb7b50c0d2916de.

Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
 drivers/gpu/drm/i915/i915_active.h       | 10 +++--
 drivers/gpu/drm/i915/i915_active_types.h |  2 -
 3 files changed, 14 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 768713657e85..d960d0be5bd2 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -148,22 +148,8 @@ __active_retire(struct i915_active *ref)
  spin_unlock_irqrestore(&ref->tree_lock, flags);
 
  /* After the final retire, the entire struct may be freed */
- if (ref->retire) {
- if (ref->active) {
- bool freed = false;
-
- /* Don't race with the active callback, and avoid UaF */
- down_write(&ref->rwsem);
- ref->freed = &freed;
- ref->retire(ref);
- if (!freed) {
- ref->freed = NULL;
- up_write(&ref->rwsem);
- }
- } else {
- ref->retire(ref);
- }
- }
+ if (ref->retire)
+ ref->retire(ref);
 
  /* ... except if you wait on it, you must manage your own references! */
  wake_up_var(ref);
@@ -293,8 +279,7 @@ void __i915_active_init(struct i915_active *ref,
  int (*active)(struct i915_active *ref),
  void (*retire)(struct i915_active *ref),
  struct lock_class_key *mkey,
- struct lock_class_key *wkey,
- struct lock_class_key *rkey)
+ struct lock_class_key *wkey)
 {
  unsigned long bits;
 
@@ -303,13 +288,8 @@ void __i915_active_init(struct i915_active *ref,
  ref->flags = 0;
  ref->active = active;
  ref->retire = ptr_unpack_bits(retire, &bits, 2);
- ref->freed = NULL;
- if (ref->active && ref->retire) {
- __init_rwsem(&ref->rwsem, "i915_active.rwsem", rkey);
+ if (bits & I915_ACTIVE_MAY_SLEEP)
  ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
- } else if (bits & I915_ACTIVE_MAY_SLEEP) {
- ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
- }
 
  spin_lock_init(&ref->tree_lock);
  ref->tree = RB_ROOT;
@@ -448,20 +428,8 @@ int i915_active_acquire(struct i915_active *ref)
  return err;
 
  if (likely(!i915_active_acquire_if_busy(ref))) {
- if (ref->active) {
- if (ref->retire) {
- /*
- * This can be a recursive call, and the mutex
- * above already protects from concurrent active
- * callbacks, so a read lock fits best.
- */
- down_read(&ref->rwsem);
- err = ref->active(ref);
- up_read(&ref->rwsem);
- } else {
- err = ref->active(ref);
- }
- }
+ if (ref->active)
+ err = ref->active(ref);
  if (!err) {
  spin_lock_irq(&ref->tree_lock); /* __active_retire() */
  debug_active_activate(ref);
@@ -683,20 +651,16 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence,
  return await_active(ref, flags, sw_await_fence, fence, fence);
 }
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
 void i915_active_fini(struct i915_active *ref)
 {
- if (ref->freed) {
- *ref->freed = true;
- up_write(&ref->rwsem);
- }
-#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
  debug_active_fini(ref);
  GEM_BUG_ON(atomic_read(&ref->count));
  GEM_BUG_ON(work_pending(&ref->work));
  GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree));
  mutex_destroy(&ref->mutex);
-#endif
 }
+#endif
 
 static inline bool is_idle_barrier(struct active_node *node, u64 idx)
 {
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
index a2e3742b1090..cf4058150966 100644
--- a/drivers/gpu/drm/i915/i915_active.h
+++ b/drivers/gpu/drm/i915/i915_active.h
@@ -153,16 +153,14 @@ void __i915_active_init(struct i915_active *ref,
  int (*active)(struct i915_active *ref),
  void (*retire)(struct i915_active *ref),
  struct lock_class_key *mkey,
- struct lock_class_key *wkey,
- struct lock_class_key *rkey);
+ struct lock_class_key *wkey);
 
 /* Specialise each class of i915_active to avoid impossible lockdep cycles. */
 #define i915_active_init(ref, active, retire) do { \
  static struct lock_class_key __mkey; \
  static struct lock_class_key __wkey; \
- static struct lock_class_key __rkey; \
  \
- __i915_active_init(ref, active, retire, &__mkey, &__wkey, &__rkey); \
+ __i915_active_init(ref, active, retire, &__mkey, &__wkey); \
 } while (0)
 
 int i915_active_ref(struct i915_active *ref,
@@ -215,7 +213,11 @@ i915_active_is_idle(const struct i915_active *ref)
  return !atomic_read(&ref->count);
 }
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
 void i915_active_fini(struct i915_active *ref);
+#else
+static inline void i915_active_fini(struct i915_active *ref) { }
+#endif
 
 int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
     struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
index aaee2548cb19..6360c3e4b765 100644
--- a/drivers/gpu/drm/i915/i915_active_types.h
+++ b/drivers/gpu/drm/i915/i915_active_types.h
@@ -32,8 +32,6 @@ struct active_node;
 struct i915_active {
  atomic_t count;
  struct mutex mutex;
- struct rw_semaphore rwsem;
- bool *freed;
 
  spinlock_t tree_lock;
  struct active_node *cache;
--
2.28.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU] [Groovy] [PATCH 0/1] Fix noise lines on i915 + Xorg

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 13.11.20 05:47, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1896091
>
> [Impact]
> Noise-like lines of graphics corruption when moving windows in Xorg
> sessions.
>
> [Fix]
> Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
> callbacks".
>
> [Test]
> Daniel van Vugt confirmed reverting the commit fixes the issue.
> I can also reproduce the issue on one of my system, I can confirm it
> fixes the issue too.
>
> [Where problems could occur]
> Race condition the commit solves may appear again.
> However it was to solve the issue on 5.4, and the race could already be
> solved on 5.8.
Is there no way to double check that before we re-open a problem here?

-Stefan

>
> Kai-Heng Feng (1):
>   Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
>     callbacks"
>
>  drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
>  drivers/gpu/drm/i915/i915_active.h       | 10 +++--
>  drivers/gpu/drm/i915/i915_active_types.h |  2 -
>  3 files changed, 14 insertions(+), 50 deletions(-)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU] [Groovy] [PATCH 0/1] Fix noise lines on i915 + Xorg

Kai-Heng Feng


> On Nov 13, 2020, at 16:05, Stefan Bader <[hidden email]> wrote:
>
> On 13.11.20 05:47, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1896091
>>
>> [Impact]
>> Noise-like lines of graphics corruption when moving windows in Xorg
>> sessions.
>>
>> [Fix]
>> Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
>> callbacks".
>>
>> [Test]
>> Daniel van Vugt confirmed reverting the commit fixes the issue.
>> I can also reproduce the issue on one of my system, I can confirm it
>> fixes the issue too.
>>
>> [Where problems could occur]
>> Race condition the commit solves may appear again.
>> However it was to solve the issue on 5.4, and the race could already be
>> solved on 5.8.
>
> Is there no way to double check that before we re-open a problem here?

Actually this commit was also reverted in Focal, replaced by upstream fix "drm/i915/gt: Make intel_ring_unpin() safe for concurrent pint", which is already part of Groovy.

Kai-Heng

>
> -Stefan
>
>>
>> Kai-Heng Feng (1):
>>  Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
>>    callbacks"
>>
>> drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
>> drivers/gpu/drm/i915/i915_active.h       | 10 +++--
>> drivers/gpu/drm/i915/i915_active_types.h |  2 -
>> 3 files changed, 14 insertions(+), 50 deletions(-)


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 1/1] Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire callbacks"

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 13.11.20 05:47, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1896091
>
> This reverts commit c1484c993957fe6b366ab453fbb7b50c0d2916de.
>
> Signed-off-by: Kai-Heng Feng <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

Ok, with the additional info that there was a revert of this and an update for
it in Focal (just the revert was not done forward, and we probably should think
about how we could avoid this to happen again) it sounds less dangerous. You
want to make sure that this info goes into the bug reports regression potential,
too.

-Stefan

>  drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
>  drivers/gpu/drm/i915/i915_active.h       | 10 +++--
>  drivers/gpu/drm/i915/i915_active_types.h |  2 -
>  3 files changed, 14 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 768713657e85..d960d0be5bd2 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -148,22 +148,8 @@ __active_retire(struct i915_active *ref)
>   spin_unlock_irqrestore(&ref->tree_lock, flags);
>  
>   /* After the final retire, the entire struct may be freed */
> - if (ref->retire) {
> - if (ref->active) {
> - bool freed = false;
> -
> - /* Don't race with the active callback, and avoid UaF */
> - down_write(&ref->rwsem);
> - ref->freed = &freed;
> - ref->retire(ref);
> - if (!freed) {
> - ref->freed = NULL;
> - up_write(&ref->rwsem);
> - }
> - } else {
> - ref->retire(ref);
> - }
> - }
> + if (ref->retire)
> + ref->retire(ref);
>  
>   /* ... except if you wait on it, you must manage your own references! */
>   wake_up_var(ref);
> @@ -293,8 +279,7 @@ void __i915_active_init(struct i915_active *ref,
>   int (*active)(struct i915_active *ref),
>   void (*retire)(struct i915_active *ref),
>   struct lock_class_key *mkey,
> - struct lock_class_key *wkey,
> - struct lock_class_key *rkey)
> + struct lock_class_key *wkey)
>  {
>   unsigned long bits;
>  
> @@ -303,13 +288,8 @@ void __i915_active_init(struct i915_active *ref,
>   ref->flags = 0;
>   ref->active = active;
>   ref->retire = ptr_unpack_bits(retire, &bits, 2);
> - ref->freed = NULL;
> - if (ref->active && ref->retire) {
> - __init_rwsem(&ref->rwsem, "i915_active.rwsem", rkey);
> + if (bits & I915_ACTIVE_MAY_SLEEP)
>   ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
> - } else if (bits & I915_ACTIVE_MAY_SLEEP) {
> - ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
> - }
>  
>   spin_lock_init(&ref->tree_lock);
>   ref->tree = RB_ROOT;
> @@ -448,20 +428,8 @@ int i915_active_acquire(struct i915_active *ref)
>   return err;
>  
>   if (likely(!i915_active_acquire_if_busy(ref))) {
> - if (ref->active) {
> - if (ref->retire) {
> - /*
> - * This can be a recursive call, and the mutex
> - * above already protects from concurrent active
> - * callbacks, so a read lock fits best.
> - */
> - down_read(&ref->rwsem);
> - err = ref->active(ref);
> - up_read(&ref->rwsem);
> - } else {
> - err = ref->active(ref);
> - }
> - }
> + if (ref->active)
> + err = ref->active(ref);
>   if (!err) {
>   spin_lock_irq(&ref->tree_lock); /* __active_retire() */
>   debug_active_activate(ref);
> @@ -683,20 +651,16 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence,
>   return await_active(ref, flags, sw_await_fence, fence, fence);
>  }
>  
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>  void i915_active_fini(struct i915_active *ref)
>  {
> - if (ref->freed) {
> - *ref->freed = true;
> - up_write(&ref->rwsem);
> - }
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>   debug_active_fini(ref);
>   GEM_BUG_ON(atomic_read(&ref->count));
>   GEM_BUG_ON(work_pending(&ref->work));
>   GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree));
>   mutex_destroy(&ref->mutex);
> -#endif
>  }
> +#endif
>  
>  static inline bool is_idle_barrier(struct active_node *node, u64 idx)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
> index a2e3742b1090..cf4058150966 100644
> --- a/drivers/gpu/drm/i915/i915_active.h
> +++ b/drivers/gpu/drm/i915/i915_active.h
> @@ -153,16 +153,14 @@ void __i915_active_init(struct i915_active *ref,
>   int (*active)(struct i915_active *ref),
>   void (*retire)(struct i915_active *ref),
>   struct lock_class_key *mkey,
> - struct lock_class_key *wkey,
> - struct lock_class_key *rkey);
> + struct lock_class_key *wkey);
>  
>  /* Specialise each class of i915_active to avoid impossible lockdep cycles. */
>  #define i915_active_init(ref, active, retire) do { \
>   static struct lock_class_key __mkey; \
>   static struct lock_class_key __wkey; \
> - static struct lock_class_key __rkey; \
>   \
> - __i915_active_init(ref, active, retire, &__mkey, &__wkey, &__rkey); \
> + __i915_active_init(ref, active, retire, &__mkey, &__wkey); \
>  } while (0)
>  
>  int i915_active_ref(struct i915_active *ref,
> @@ -215,7 +213,11 @@ i915_active_is_idle(const struct i915_active *ref)
>   return !atomic_read(&ref->count);
>  }
>  
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>  void i915_active_fini(struct i915_active *ref);
> +#else
> +static inline void i915_active_fini(struct i915_active *ref) { }
> +#endif
>  
>  int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>      struct intel_engine_cs *engine);
> diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
> index aaee2548cb19..6360c3e4b765 100644
> --- a/drivers/gpu/drm/i915/i915_active_types.h
> +++ b/drivers/gpu/drm/i915/i915_active_types.h
> @@ -32,8 +32,6 @@ struct active_node;
>  struct i915_active {
>   atomic_t count;
>   struct mutex mutex;
> - struct rw_semaphore rwsem;
> - bool *freed;
>  
>   spinlock_t tree_lock;
>   struct active_node *cache;
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 1/1] Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire callbacks"

Kleber Souza
In reply to this post by Kai-Heng Feng
On 13.11.20 05:47, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/1896091
>
> This reverts commit c1484c993957fe6b366ab453fbb7b50c0d2916de.
>
> Signed-off-by: Kai-Heng Feng <[hidden email]>

I'm also affected by this and it's pretty annoying. Thanks for finding
a fix!

Acked-by: Kleber Sacilotto de Souza <[hidden email]>


> ---
>   drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
>   drivers/gpu/drm/i915/i915_active.h       | 10 +++--
>   drivers/gpu/drm/i915/i915_active_types.h |  2 -
>   3 files changed, 14 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 768713657e85..d960d0be5bd2 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -148,22 +148,8 @@ __active_retire(struct i915_active *ref)
>   spin_unlock_irqrestore(&ref->tree_lock, flags);
>  
>   /* After the final retire, the entire struct may be freed */
> - if (ref->retire) {
> - if (ref->active) {
> - bool freed = false;
> -
> - /* Don't race with the active callback, and avoid UaF */
> - down_write(&ref->rwsem);
> - ref->freed = &freed;
> - ref->retire(ref);
> - if (!freed) {
> - ref->freed = NULL;
> - up_write(&ref->rwsem);
> - }
> - } else {
> - ref->retire(ref);
> - }
> - }
> + if (ref->retire)
> + ref->retire(ref);
>  
>   /* ... except if you wait on it, you must manage your own references! */
>   wake_up_var(ref);
> @@ -293,8 +279,7 @@ void __i915_active_init(struct i915_active *ref,
>   int (*active)(struct i915_active *ref),
>   void (*retire)(struct i915_active *ref),
>   struct lock_class_key *mkey,
> - struct lock_class_key *wkey,
> - struct lock_class_key *rkey)
> + struct lock_class_key *wkey)
>   {
>   unsigned long bits;
>  
> @@ -303,13 +288,8 @@ void __i915_active_init(struct i915_active *ref,
>   ref->flags = 0;
>   ref->active = active;
>   ref->retire = ptr_unpack_bits(retire, &bits, 2);
> - ref->freed = NULL;
> - if (ref->active && ref->retire) {
> - __init_rwsem(&ref->rwsem, "i915_active.rwsem", rkey);
> + if (bits & I915_ACTIVE_MAY_SLEEP)
>   ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
> - } else if (bits & I915_ACTIVE_MAY_SLEEP) {
> - ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;
> - }
>  
>   spin_lock_init(&ref->tree_lock);
>   ref->tree = RB_ROOT;
> @@ -448,20 +428,8 @@ int i915_active_acquire(struct i915_active *ref)
>   return err;
>  
>   if (likely(!i915_active_acquire_if_busy(ref))) {
> - if (ref->active) {
> - if (ref->retire) {
> - /*
> - * This can be a recursive call, and the mutex
> - * above already protects from concurrent active
> - * callbacks, so a read lock fits best.
> - */
> - down_read(&ref->rwsem);
> - err = ref->active(ref);
> - up_read(&ref->rwsem);
> - } else {
> - err = ref->active(ref);
> - }
> - }
> + if (ref->active)
> + err = ref->active(ref);
>   if (!err) {
>   spin_lock_irq(&ref->tree_lock); /* __active_retire() */
>   debug_active_activate(ref);
> @@ -683,20 +651,16 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence,
>   return await_active(ref, flags, sw_await_fence, fence, fence);
>   }
>  
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>   void i915_active_fini(struct i915_active *ref)
>   {
> - if (ref->freed) {
> - *ref->freed = true;
> - up_write(&ref->rwsem);
> - }
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>   debug_active_fini(ref);
>   GEM_BUG_ON(atomic_read(&ref->count));
>   GEM_BUG_ON(work_pending(&ref->work));
>   GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree));
>   mutex_destroy(&ref->mutex);
> -#endif
>   }
> +#endif
>  
>   static inline bool is_idle_barrier(struct active_node *node, u64 idx)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h
> index a2e3742b1090..cf4058150966 100644
> --- a/drivers/gpu/drm/i915/i915_active.h
> +++ b/drivers/gpu/drm/i915/i915_active.h
> @@ -153,16 +153,14 @@ void __i915_active_init(struct i915_active *ref,
>   int (*active)(struct i915_active *ref),
>   void (*retire)(struct i915_active *ref),
>   struct lock_class_key *mkey,
> - struct lock_class_key *wkey,
> - struct lock_class_key *rkey);
> + struct lock_class_key *wkey);
>  
>   /* Specialise each class of i915_active to avoid impossible lockdep cycles. */
>   #define i915_active_init(ref, active, retire) do { \
>   static struct lock_class_key __mkey; \
>   static struct lock_class_key __wkey; \
> - static struct lock_class_key __rkey; \
>   \
> - __i915_active_init(ref, active, retire, &__mkey, &__wkey, &__rkey); \
> + __i915_active_init(ref, active, retire, &__mkey, &__wkey); \
>   } while (0)
>  
>   int i915_active_ref(struct i915_active *ref,
> @@ -215,7 +213,11 @@ i915_active_is_idle(const struct i915_active *ref)
>   return !atomic_read(&ref->count);
>   }
>  
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>   void i915_active_fini(struct i915_active *ref);
> +#else
> +static inline void i915_active_fini(struct i915_active *ref) { }
> +#endif
>  
>   int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>      struct intel_engine_cs *engine);
> diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h
> index aaee2548cb19..6360c3e4b765 100644
> --- a/drivers/gpu/drm/i915/i915_active_types.h
> +++ b/drivers/gpu/drm/i915/i915_active_types.h
> @@ -32,8 +32,6 @@ struct active_node;
>   struct i915_active {
>   atomic_t count;
>   struct mutex mutex;
> - struct rw_semaphore rwsem;
> - bool *freed;
>  
>   spinlock_t tree_lock;
>   struct active_node *cache;
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU] [Groovy] [PATCH 0/1] Fix noise lines on i915 + Xorg

William Breathitt Gray
In reply to this post by Kai-Heng Feng
On Fri, Nov 13, 2020 at 12:47:30PM +0800, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1896091
>
> [Impact]
> Noise-like lines of graphics corruption when moving windows in Xorg
> sessions.
>
> [Fix]
> Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
> callbacks".
>
> [Test]
> Daniel van Vugt confirmed reverting the commit fixes the issue.
> I can also reproduce the issue on one of my system, I can confirm it
> fixes the issue too.
>
> [Where problems could occur]
> Race condition the commit solves may appear again.
> However it was to solve the issue on 5.4, and the race could already be
> solved on 5.8.
>
> Kai-Heng Feng (1):
>   Revert "UBUNTU: SAUCE: drm/i915: Synchronize active and retire
>     callbacks"
>
>  drivers/gpu/drm/i915/i915_active.c       | 52 ++++--------------------
>  drivers/gpu/drm/i915/i915_active.h       | 10 +++--
>  drivers/gpu/drm/i915/i915_active_types.h |  2 -
>  3 files changed, 14 insertions(+), 50 deletions(-)
>
> --
> 2.28.0
Applied to Groovy:linux.

William Breathitt Gray

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment