Re: [PATCH] drm/amd/display: Fix error handling on waiting for completion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-10-26 7:07 a.m., Stylon Wang wrote:
[Why]
In GNOME Settings->Display the switching from mirror mode to single display
occasionally causes wait_for_completion_interruptible_timeout() to return
-ERESTARTSYS and fails atomic check.

[How]
Replace the call with wait_for_completion_timeout() since the waiting for
hw_done and flip_done completion doesn't need to worry about interruption
from signal.

Signed-off-by: Stylon Wang <stylon.wang@xxxxxxx>

I think this is okay, but I'll write out how I think these work here in case anyone has corrections.

Both variants allow the thread to sleep, but the interruptible variant can waken due to signals. These signals are a secondary wakeup event and would require use to restart the wait and (probably) keep track of how long we were waiting before.

We want wakeup only on completion, so we should be using the `wait_for_completion_timeout()` variants instead in most (if not all?) cases in our display driver.

This probably has some nuances that matter more for different variants of UAPI, but with this understanding I think this is:

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>

Now, if we could revive that patch series I had from the other year and outright drop `do_aquire_global_lock()`...

Regards,
Nicholas Kazlauskas

---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4cd64529b180..b8f4ff323de1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9844,10 +9844,10 @@ static int do_aquire_global_lock(struct drm_device *dev,
  		 * Make sure all pending HW programming completed and
  		 * page flips done
  		 */
-		ret = wait_for_completion_interruptible_timeout(&commit->hw_done, 10*HZ);
+		ret = wait_for_completion_timeout(&commit->hw_done, 10*HZ);
if (ret > 0)
-			ret = wait_for_completion_interruptible_timeout(
+			ret = wait_for_completion_timeout(
  					&commit->flip_done, 10*HZ);
if (ret == 0)





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux