All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Dependency locks fixes for QXL driver
@ 2015-09-24 13:25 Frediano Ziglio
  2015-09-24 13:25 ` [PATCH v2 1/2] drm/qxl: avoid buffer reservation in qxl_crtc_page_flip Frediano Ziglio
  2015-09-24 13:25 ` [PATCH v2 2/2] drm/qxl: avoid dependency lock Frediano Ziglio
  0 siblings, 2 replies; 4+ messages in thread
From: Frediano Ziglio @ 2015-09-24 13:25 UTC (permalink / raw
  To: David Airlie; +Cc: spice-devel, dri-devel, Frediano Ziglio

These two patches fixes two lock dependencies issues.

Changes from v1: Sent a wrong patch.

Frediano Ziglio (2):
  drm/qxl: avoid buffer reservation in qxl_crtc_page_flip
  drm/qxl: avoid dependency lock

 drivers/gpu/drm/qxl/qxl_display.c | 10 +++++++++-
 drivers/gpu/drm/qxl/qxl_release.c |  4 +---
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] drm/qxl: avoid buffer reservation in qxl_crtc_page_flip
  2015-09-24 13:25 [PATCH v2 0/2] Dependency locks fixes for QXL driver Frediano Ziglio
@ 2015-09-24 13:25 ` Frediano Ziglio
  2015-09-24 13:25 ` [PATCH v2 2/2] drm/qxl: avoid dependency lock Frediano Ziglio
  1 sibling, 0 replies; 4+ messages in thread
From: Frediano Ziglio @ 2015-09-24 13:25 UTC (permalink / raw
  To: David Airlie; +Cc: spice-devel, dri-devel

This avoid a dependency lock error.
According to https://lwn.net/Articles/548909/ users of WW mutex API
should avoid using different context.
When a buffer is reserved with qxl_bo_reserve a ww_mutex_lock without
context is used. However during qxl_draw_dirty_fb different locks
with specific context are used.
This is detected during a machine booting with a debug kernel with lock
dependency checking enabled.
Like many other function in this file to avoid this problem object
pinning is used. Once the object is pinned is not necessary to keep
the lock so it can be released avoiding the locking problem.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index a8dbb3e..656752d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -241,6 +241,10 @@ static int qxl_crtc_page_flip(struct drm_crtc *crtc,
 	ret = qxl_bo_reserve(bo, false);
 	if (ret)
 		return ret;
+	ret = qxl_bo_pin(bo, bo->type, NULL);
+	qxl_bo_unreserve(bo);
+	if (ret)
+		return ret;
 
 	qxl_draw_dirty_fb(qdev, qfb_src, bo, 0, 0,
 			  &norect, one_clip_rect, inc);
@@ -254,7 +258,11 @@ static int qxl_crtc_page_flip(struct drm_crtc *crtc,
 	}
 	drm_vblank_put(dev, qcrtc->index);
 
-	qxl_bo_unreserve(bo);
+	ret = qxl_bo_reserve(bo, false);
+	if (!ret) {
+		qxl_bo_unpin(bo);
+		qxl_bo_unreserve(bo);
+	}
 
 	return 0;
 }
-- 
2.4.3

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] drm/qxl: avoid dependency lock
  2015-09-24 13:25 [PATCH v2 0/2] Dependency locks fixes for QXL driver Frediano Ziglio
  2015-09-24 13:25 ` [PATCH v2 1/2] drm/qxl: avoid buffer reservation in qxl_crtc_page_flip Frediano Ziglio
@ 2015-09-24 13:25 ` Frediano Ziglio
  2015-10-01 14:01   ` poma
  1 sibling, 1 reply; 4+ messages in thread
From: Frediano Ziglio @ 2015-09-24 13:25 UTC (permalink / raw
  To: David Airlie; +Cc: spice-devel, dri-devel

qxl_bo_unref calls drm_gem_object_unreference_unlocked which
locks dev->struct_mutex. However this lock could be already
locked if the call came from qxl_gem_object_free.
As we don't need to call qxl_bo_ref/qxl_bo_unref cause
qxl_release_list_add will hold a reference by itself avoid
to call them and the possible deadlock.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_release.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index b66ec33..4efa8e2 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -307,7 +307,7 @@ int qxl_alloc_surface_release_reserved(struct qxl_device *qdev,
 		idr_ret = qxl_release_alloc(qdev, QXL_RELEASE_SURFACE_CMD, release);
 		if (idr_ret < 0)
 			return idr_ret;
-		bo = qxl_bo_ref(to_qxl_bo(entry->tv.bo));
+		bo = to_qxl_bo(entry->tv.bo);
 
 		(*release)->release_offset = create_rel->release_offset + 64;
 
@@ -316,8 +316,6 @@ int qxl_alloc_surface_release_reserved(struct qxl_device *qdev,
 		info = qxl_release_map(qdev, *release);
 		info->id = idr_ret;
 		qxl_release_unmap(qdev, *release, info);
-
-		qxl_bo_unref(&bo);
 		return 0;
 	}
 
-- 
2.4.3

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] drm/qxl: avoid dependency lock
  2015-09-24 13:25 ` [PATCH v2 2/2] drm/qxl: avoid dependency lock Frediano Ziglio
@ 2015-10-01 14:01   ` poma
  0 siblings, 0 replies; 4+ messages in thread
From: poma @ 2015-10-01 14:01 UTC (permalink / raw
  To: Frediano Ziglio, David Airlie; +Cc: spice-devel, dri-devel

On 24.09.2015 15:25, Frediano Ziglio wrote:
> qxl_bo_unref calls drm_gem_object_unreference_unlocked which
> locks dev->struct_mutex. However this lock could be already
> locked if the call came from qxl_gem_object_free.
> As we don't need to call qxl_bo_ref/qxl_bo_unref cause
> qxl_release_list_add will hold a reference by itself avoid
> to call them and the possible deadlock.
> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  drivers/gpu/drm/qxl/qxl_release.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
> index b66ec33..4efa8e2 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -307,7 +307,7 @@ int qxl_alloc_surface_release_reserved(struct qxl_device *qdev,
>  		idr_ret = qxl_release_alloc(qdev, QXL_RELEASE_SURFACE_CMD, release);
>  		if (idr_ret < 0)
>  			return idr_ret;
> -		bo = qxl_bo_ref(to_qxl_bo(entry->tv.bo));
> +		bo = to_qxl_bo(entry->tv.bo);
>  
>  		(*release)->release_offset = create_rel->release_offset + 64;
>  
> @@ -316,8 +316,6 @@ int qxl_alloc_surface_release_reserved(struct qxl_device *qdev,
>  		info = qxl_release_map(qdev, *release);
>  		info->id = idr_ret;
>  		qxl_release_unmap(qdev, *release, info);
> -
> -		qxl_bo_unref(&bo);
>  		return 0;
>  	}
>  
> 


Tested with:
 - Rawhide-Xfce-Live-1001.iso
  \ 4.3.0-0.rc3.git2.4.fc24.x86_64
   + v2-1-2-drm-qxl-avoid-buffer-reservation-in-qxl_crtc_page_flip.patch
   + v2-2-2-drm-qxl-avoid-dependency-lock.patch

Tested-by: poma <pomidorabelisima@gmail.com>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-10-01 14:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-24 13:25 [PATCH v2 0/2] Dependency locks fixes for QXL driver Frediano Ziglio
2015-09-24 13:25 ` [PATCH v2 1/2] drm/qxl: avoid buffer reservation in qxl_crtc_page_flip Frediano Ziglio
2015-09-24 13:25 ` [PATCH v2 2/2] drm/qxl: avoid dependency lock Frediano Ziglio
2015-10-01 14:01   ` poma

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.