So I just wanted to add a new field to struct drm_device and
accidentally stumbled over something. According to comments
dev->open_count is protected by dev->count_lock, but that's totally
not the case. It's protected by drm_global_mutex.
Unfortunately the vga switcheroo callbacks took this comment at face
value. The problem is that we can't just take the drm_global_mutex
because:
- It would lead to a locking inversion with the driver load/unload
paths.
- It wouldn't actually protect anything, for that we'd need to wrap
the entire vga switcheroo code in the drm_global_mutex. And I'm not
sure whether that would actually solve anything.
What we probably want is a try_to_grab_switcheroo reference kind of
thing which is used in the driver's ->open callback. Then we could
move all that ->can_switch madness into the vga switcheroo core where
it really belongs.
But since that would amount to real work take the easy way out and
just add a comment. It's definitely not going to make anything worse
since doing switcheroo state changes while restarting X just isn't
recommended. Even though the delayed switching code does exactly that.
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
drivers/gpu/drm/i915/i915_dma.c | 7 +++++--
drivers/gpu/drm/nouveau/nouveau_vga.c | 7 +++++--
drivers/gpu/drm/radeon/radeon_device.c | 7 +++++--
include/drm/drmP.h | 2 +-
4 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4dff829c2d89..dcb67c6564dd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1286,9 +1286,12 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
bool can_switch;
- spin_lock(&dev->count_lock);
+ /*
+ * FIXME: open_count is protected by drm_global_mutex but that would lead to
+ * locking inversion with the driver load path. And the access here is
+ * completely racy anyway. So don't bother with locking for now.
+ */
can_switch = (dev->open_count == 0);
- spin_unlock(&dev->count_lock);
return can_switch;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
index fb84da3cb50d..7b10c5a9196b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -66,9 +66,12 @@ nouveau_switcheroo_can_switch(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
bool can_switch;
- spin_lock(&dev->count_lock);
+ /*
+ * FIXME: open_count is protected by drm_global_mutex but that would lead to
+ * locking inversion with the driver load path. And the access here is
+ * completely racy anyway. So don't bother with locking for now.
+ */
can_switch = (dev->open_count == 0);
- spin_unlock(&dev->count_lock);
return can_switch;
}
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 835516d2d257..c9811d01a3c7 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1124,9 +1124,12 @@ static bool radeon_switcheroo_can_switch(struct pci_dev *pdev)
struct drm_device *dev = pci_get_drvdata(pdev);
bool can_switch;
- spin_lock(&dev->count_lock);
+ /*
+ * FIXME: open_count is protected by drm_global_mutex but that would lead to
+ * locking inversion with the driver load path. And the access here is
+ * completely racy anyway. So don't bother with locking for now.
+ */
can_switch = (dev->open_count == 0);
- spin_unlock(&dev->count_lock);
return can_switch;
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9f1fb8d36b67..bd16a4c8ece4 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1076,7 +1076,7 @@ struct drm_device {
/** \name Usage Counters */
/*@{ */
- int open_count; /**< Outstanding files open */
+ int open_count; /**< Outstanding files open, protected by drm_global_lock. */
int buf_use; /**< Buffers in use -- cannot alloc */
atomic_t buf_alloc; /**< Buffer allocation in progress */
/*@} */
--
1.8.5.2
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
