ACK: [RFC/SRU Bionic 0/2] Reverts we probably want to do quickly

Kleber Souza kleber.souza at canonical.com
Fri Jul 30 10:23:37 UTC 2021


On 28.07.21 17:02, Stefan Bader wrote:
> While we were looking for a regression I stumbled over two changes we
> imported from 4.19.y in to Bionic. Both differ from the versions in
> 4.19.y and somehow I am suspicious that they might not be needed or even
> might cause some harm.
> 
> The amdgpu change follows a change that is not in 4.15 where GEM objects
> are stored in a different place and somehow I believe that this might
> have added the need to release multiple references. Though I cannot find
> the place where those multiple references are taken (which is the claim
> of the fix). The test done before the release loop is using the same
> reference as the modified fix code. So maybe it is ok. On the other hand
> it is sometimes better not to change an apparently working system.
> 
> With the mutex change it was a missing removal of a __sched directive.
> It was dropped from a function which appears to be the counterpart of a
> new function that gets added with the fix. This function (which does
> exist in 4.19 but not in 4.15 was added to 4.19 when adding Wound-Wait
> mutexes:
> 
> commit 08295b3b5beec9aac0f7a9db86f0fc3792039da3
> Author: Thomas Hellstrom <thellstrom at vmware.com>
> Date:   Fri Jun 15 10:17:38 2018 +0200
> 
>      locking: Implement an algorithm choice for Wound-Wait mutexes
> ...
> /*
> + * Add @waiter to a given location in the lock wait_list and set the
> + * FLAG_WAITERS flag if it's the first waiter.
> + */
> +static void __sched
> +__mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
> +                  struct list_head *list)
> 
> The question is whether without this, the clearing is actually required
> or potentially even be dangerous.
> 
> -Stefan
> 
> Stefan Bader (2):
>    Revert "locking/mutex: clear MUTEX_FLAGS if wait_list is empty due to
>      signal"
>    Revert "drm/amd/amdgpu: fix refcount leak"
> 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 ---
>   kernel/locking/mutex-debug.c           |  4 ++--
>   kernel/locking/mutex-debug.h           |  2 +-
>   kernel/locking/mutex.c                 | 16 ++++------------
>   kernel/locking/mutex.h                 |  4 +++-
>   5 files changed, 10 insertions(+), 19 deletions(-)
> 

Reverting those patches looks sensible.


Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

Thanks




More information about the kernel-team mailing list