* [PATCH] drm: Use srcu to protect drm_device.unplugged
@ 2018-03-28 7:38 ` Oleksandr Andrushchenko
0 siblings, 0 replies; 9+ messages in thread
From: Oleksandr Andrushchenko @ 2018-03-28 7:38 UTC (permalink / raw)
To: linux-kernel, dri-devel, airlied, daniel.vetter, seanpaul, gustavo
Cc: andr2000, Noralf Trønnes, Oleksandr Andrushchenko, intel-gfx
From: Noralf Trønnes <noralf@tronnes.org>
Use srcu to protect drm_device.unplugged in a race free manner.
Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
sections preventing access to device resources that are not available
after the device is gone.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Cc: intel-gfx@lists.freedesktop.org
---
drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
include/drm/drm_device.h | 9 +++++++-
include/drm/drm_drv.h | 15 +++++++++----
3 files changed, 68 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a1b9338736e3..32a83b41ab61 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -32,6 +32,7 @@
#include <linux/moduleparam.h>
#include <linux/mount.h>
#include <linux/slab.h>
+#include <linux/srcu.h>
#include <drm/drm_drv.h>
#include <drm/drmP.h>
@@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
static struct dentry *drm_debugfs_root;
+DEFINE_STATIC_SRCU(drm_unplug_srcu);
+
/*
* DRM Minors
* A DRM device can provide several char-dev interfaces on the DRM-Major. Each
@@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_put_dev);
-static void drm_device_set_unplugged(struct drm_device *dev)
+/**
+ * drm_dev_enter - Enter device critical section
+ * @dev: DRM device
+ * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
+ *
+ * This function marks and protects the beginning of a section that should not
+ * be entered after the device has been unplugged. The section end is marked
+ * with drm_dev_exit(). Calls to this function can be nested.
+ *
+ * Returns:
+ * True if it is OK to enter the section, false otherwise.
+ */
+bool drm_dev_enter(struct drm_device *dev, int *idx)
+{
+ *idx = srcu_read_lock(&drm_unplug_srcu);
+
+ if (dev->unplugged) {
+ srcu_read_unlock(&drm_unplug_srcu, *idx);
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL(drm_dev_enter);
+
+/**
+ * drm_dev_exit - Exit device critical section
+ * @idx: index returned from drm_dev_enter()
+ *
+ * This function marks the end of a section that should not be entered after
+ * the device has been unplugged.
+ */
+void drm_dev_exit(int idx)
{
- smp_wmb();
- atomic_set(&dev->unplugged, 1);
+ srcu_read_unlock(&drm_unplug_srcu, idx);
}
+EXPORT_SYMBOL(drm_dev_exit);
/**
* drm_dev_unplug - unplug a DRM device
* @dev: DRM device
*
* This unplugs a hotpluggable DRM device, which makes it inaccessible to
- * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
+ * userspace operations. Entry-points can use drm_dev_enter() and
+ * drm_dev_exit() to protect device resources in a race free manner. This
* essentially unregisters the device like drm_dev_unregister(), but can be
* called while there are still open users of @dev.
*/
@@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
drm_dev_unregister(dev);
mutex_lock(&drm_global_mutex);
- drm_device_set_unplugged(dev);
if (dev->open_count == 0)
drm_dev_put(dev);
mutex_unlock(&drm_global_mutex);
+
+ /*
+ * After synchronizing any critical read section is guaranteed to see
+ * the new value of ->unplugged, and any critical section which might
+ * still have seen the old value of ->unplugged is guaranteed to have
+ * finished.
+ */
+ dev->unplugged = true;
+ synchronize_srcu(&drm_unplug_srcu);
}
EXPORT_SYMBOL(drm_dev_unplug);
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 7c4fa32f3fc6..3a0eac2885b7 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -46,7 +46,14 @@ struct drm_device {
/* currently active master for this device. Protected by master_mutex */
struct drm_master *master;
- atomic_t unplugged; /**< Flag whether dev is dead */
+ /**
+ * @unplugged:
+ *
+ * Flag to tell if the device has been unplugged.
+ * See drm_dev_enter() and drm_dev_is_unplugged().
+ */
+ bool unplugged;
+
struct inode *anon_inode; /**< inode for private address-space */
char *unique; /**< unique name of the device */
/*@} */
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index d23dcdd1bd95..7e545f5f94d3 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
void drm_dev_put(struct drm_device *dev);
void drm_dev_unref(struct drm_device *dev);
void drm_put_dev(struct drm_device *dev);
+bool drm_dev_enter(struct drm_device *dev, int *idx);
+void drm_dev_exit(int idx);
void drm_dev_unplug(struct drm_device *dev);
/**
@@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
* unplugged, these two functions guarantee that any store before calling
* drm_dev_unplug() is visible to callers of this function after it completes
*/
-static inline int drm_dev_is_unplugged(struct drm_device *dev)
+static inline bool drm_dev_is_unplugged(struct drm_device *dev)
{
- int ret = atomic_read(&dev->unplugged);
- smp_rmb();
- return ret;
+ int idx;
+
+ if (drm_dev_enter(dev, &idx)) {
+ drm_dev_exit(idx);
+ return false;
+ }
+
+ return true;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] drm: Use srcu to protect drm_device.unplugged
@ 2018-03-28 7:38 ` Oleksandr Andrushchenko
0 siblings, 0 replies; 9+ messages in thread
From: Oleksandr Andrushchenko @ 2018-03-28 7:38 UTC (permalink / raw)
To: linux-kernel, dri-devel, airlied, daniel.vetter, seanpaul, gustavo
Cc: andr2000, intel-gfx, Noralf Trønnes, Oleksandr Andrushchenko
From: Noralf Trønnes <noralf@tronnes.org>
Use srcu to protect drm_device.unplugged in a race free manner.
Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
sections preventing access to device resources that are not available
after the device is gone.
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Cc: intel-gfx@lists.freedesktop.org
---
drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
include/drm/drm_device.h | 9 +++++++-
include/drm/drm_drv.h | 15 +++++++++----
3 files changed, 68 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a1b9338736e3..32a83b41ab61 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -32,6 +32,7 @@
#include <linux/moduleparam.h>
#include <linux/mount.h>
#include <linux/slab.h>
+#include <linux/srcu.h>
#include <drm/drm_drv.h>
#include <drm/drmP.h>
@@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
static struct dentry *drm_debugfs_root;
+DEFINE_STATIC_SRCU(drm_unplug_srcu);
+
/*
* DRM Minors
* A DRM device can provide several char-dev interfaces on the DRM-Major. Each
@@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_put_dev);
-static void drm_device_set_unplugged(struct drm_device *dev)
+/**
+ * drm_dev_enter - Enter device critical section
+ * @dev: DRM device
+ * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
+ *
+ * This function marks and protects the beginning of a section that should not
+ * be entered after the device has been unplugged. The section end is marked
+ * with drm_dev_exit(). Calls to this function can be nested.
+ *
+ * Returns:
+ * True if it is OK to enter the section, false otherwise.
+ */
+bool drm_dev_enter(struct drm_device *dev, int *idx)
+{
+ *idx = srcu_read_lock(&drm_unplug_srcu);
+
+ if (dev->unplugged) {
+ srcu_read_unlock(&drm_unplug_srcu, *idx);
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL(drm_dev_enter);
+
+/**
+ * drm_dev_exit - Exit device critical section
+ * @idx: index returned from drm_dev_enter()
+ *
+ * This function marks the end of a section that should not be entered after
+ * the device has been unplugged.
+ */
+void drm_dev_exit(int idx)
{
- smp_wmb();
- atomic_set(&dev->unplugged, 1);
+ srcu_read_unlock(&drm_unplug_srcu, idx);
}
+EXPORT_SYMBOL(drm_dev_exit);
/**
* drm_dev_unplug - unplug a DRM device
* @dev: DRM device
*
* This unplugs a hotpluggable DRM device, which makes it inaccessible to
- * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
+ * userspace operations. Entry-points can use drm_dev_enter() and
+ * drm_dev_exit() to protect device resources in a race free manner. This
* essentially unregisters the device like drm_dev_unregister(), but can be
* called while there are still open users of @dev.
*/
@@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
drm_dev_unregister(dev);
mutex_lock(&drm_global_mutex);
- drm_device_set_unplugged(dev);
if (dev->open_count == 0)
drm_dev_put(dev);
mutex_unlock(&drm_global_mutex);
+
+ /*
+ * After synchronizing any critical read section is guaranteed to see
+ * the new value of ->unplugged, and any critical section which might
+ * still have seen the old value of ->unplugged is guaranteed to have
+ * finished.
+ */
+ dev->unplugged = true;
+ synchronize_srcu(&drm_unplug_srcu);
}
EXPORT_SYMBOL(drm_dev_unplug);
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 7c4fa32f3fc6..3a0eac2885b7 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -46,7 +46,14 @@ struct drm_device {
/* currently active master for this device. Protected by master_mutex */
struct drm_master *master;
- atomic_t unplugged; /**< Flag whether dev is dead */
+ /**
+ * @unplugged:
+ *
+ * Flag to tell if the device has been unplugged.
+ * See drm_dev_enter() and drm_dev_is_unplugged().
+ */
+ bool unplugged;
+
struct inode *anon_inode; /**< inode for private address-space */
char *unique; /**< unique name of the device */
/*@} */
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index d23dcdd1bd95..7e545f5f94d3 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
void drm_dev_put(struct drm_device *dev);
void drm_dev_unref(struct drm_device *dev);
void drm_put_dev(struct drm_device *dev);
+bool drm_dev_enter(struct drm_device *dev, int *idx);
+void drm_dev_exit(int idx);
void drm_dev_unplug(struct drm_device *dev);
/**
@@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
* unplugged, these two functions guarantee that any store before calling
* drm_dev_unplug() is visible to callers of this function after it completes
*/
-static inline int drm_dev_is_unplugged(struct drm_device *dev)
+static inline bool drm_dev_is_unplugged(struct drm_device *dev)
{
- int ret = atomic_read(&dev->unplugged);
- smp_rmb();
- return ret;
+ int idx;
+
+ if (drm_dev_enter(dev, &idx)) {
+ drm_dev_exit(idx);
+ return false;
+ }
+
+ return true;
}
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✗ Fi.CI.SPARSE: warning for drm: Use srcu to protect drm_device.unplugged
2018-03-28 7:38 ` Oleksandr Andrushchenko
(?)
@ 2018-03-28 7:54 ` Patchwork
-1 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-28 7:54 UTC (permalink / raw)
To: Oleksandr Andrushchenko; +Cc: intel-gfx
== Series Details ==
Series: drm: Use srcu to protect drm_device.unplugged
URL : https://patchwork.freedesktop.org/series/40793/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm: Use srcu to protect drm_device.unplugged
+drivers/gpu/drm/drm_drv.c:336:6: warning: context imbalance in 'drm_dev_enter' - different lock contexts for basic block
+drivers/gpu/drm/drm_drv.c:356:6: warning: context imbalance in 'drm_dev_exit' - unexpected unlock
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm: Use srcu to protect drm_device.unplugged
2018-03-28 7:38 ` Oleksandr Andrushchenko
(?)
(?)
@ 2018-03-28 8:08 ` Patchwork
-1 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-28 8:08 UTC (permalink / raw)
To: Oleksandr Andrushchenko; +Cc: intel-gfx
== Series Details ==
Series: drm: Use srcu to protect drm_device.unplugged
URL : https://patchwork.freedesktop.org/series/40793/
State : success
== Summary ==
Series 40793v1 drm: Use srcu to protect drm_device.unplugged
https://patchwork.freedesktop.org/api/1.0/series/40793/revisions/1/mbox/
---- Known issues:
Test debugfs_test:
Subgroup read_all_entries:
pass -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b-frame-sequence:
pass -> FAIL (fi-skl-guc) fdo#103191
Subgroup suspend-read-crc-pipe-c:
pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> FAIL (fi-ilk-650) fdo#104008
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:432s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:439s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:379s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:545s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:297s
fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:521s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:523s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:519s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:414s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:571s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:590s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:426s
fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:323s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:541s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:405s
fi-ilk-650 total:285 pass:224 dwarn:0 dfail:0 fail:1 skip:60 time:420s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:476s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:432s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:477s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:469s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:516s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:663s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:439s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:537s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:500s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:503s
fi-skl-guc total:285 pass:256 dwarn:0 dfail:0 fail:1 skip:28 time:427s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:443s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:529s
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:488s
23c67dc0cf31917b696b87dac997aca76c0d248d drm-tip: 2018y-03m-28d-06h-36m-40s UTC integration manifest
2eceb2811170 drm: Use srcu to protect drm_device.unplugged
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8514/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.IGT: success for drm: Use srcu to protect drm_device.unplugged
2018-03-28 7:38 ` Oleksandr Andrushchenko
` (2 preceding siblings ...)
(?)
@ 2018-03-28 13:21 ` Patchwork
-1 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-03-28 13:21 UTC (permalink / raw)
To: Oleksandr Andrushchenko; +Cc: intel-gfx
== Series Details ==
Series: drm: Use srcu to protect drm_device.unplugged
URL : https://patchwork.freedesktop.org/series/40793/
State : success
== Summary ==
---- Known issues:
Test kms_flip:
Subgroup 2x-plain-flip-ts-check-interruptible:
fail -> PASS (shard-hsw) fdo#100368 +2
Subgroup flip-vs-expired-vblank-interruptible:
pass -> FAIL (shard-hsw) fdo#102887 +1
Test kms_plane:
Subgroup plane-panning-bottom-right-suspend-pipe-b-planes:
pass -> FAIL (shard-apl) fdo#103375
Test kms_setmode:
Subgroup basic:
pass -> FAIL (shard-hsw) fdo#99912
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
shard-apl total:3495 pass:1830 dwarn:1 dfail:0 fail:8 skip:1655 time:12974s
shard-hsw total:3495 pass:1780 dwarn:1 dfail:0 fail:4 skip:1709 time:11632s
shard-snb total:3495 pass:1373 dwarn:1 dfail:0 fail:4 skip:2117 time:7102s
Blacklisted hosts:
shard-kbl total:3495 pass:1956 dwarn:1 dfail:1 fail:9 skip:1528 time:9683s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8514/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Use srcu to protect drm_device.unplugged
2018-03-28 7:38 ` Oleksandr Andrushchenko
@ 2018-03-28 15:09 ` Daniel Vetter
-1 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2018-03-28 15:09 UTC (permalink / raw)
To: Oleksandr Andrushchenko
Cc: linux-kernel, dri-devel, airlied, daniel.vetter, seanpaul,
gustavo, intel-gfx, Oleksandr Andrushchenko
On Wed, Mar 28, 2018 at 10:38:35AM +0300, Oleksandr Andrushchenko wrote:
> From: Noralf Trønnes <noralf@tronnes.org>
>
> Use srcu to protect drm_device.unplugged in a race free manner.
> Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
> sections preventing access to device resources that are not available
> after the device is gone.
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Cc: intel-gfx@lists.freedesktop.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Oleksandr, please push to drm-misc-next once you have dim tools setup up
and everything.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
> include/drm/drm_device.h | 9 +++++++-
> include/drm/drm_drv.h | 15 +++++++++----
> 3 files changed, 68 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index a1b9338736e3..32a83b41ab61 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -32,6 +32,7 @@
> #include <linux/moduleparam.h>
> #include <linux/mount.h>
> #include <linux/slab.h>
> +#include <linux/srcu.h>
>
> #include <drm/drm_drv.h>
> #include <drm/drmP.h>
> @@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
>
> static struct dentry *drm_debugfs_root;
>
> +DEFINE_STATIC_SRCU(drm_unplug_srcu);
> +
> /*
> * DRM Minors
> * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
> @@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_put_dev);
>
> -static void drm_device_set_unplugged(struct drm_device *dev)
> +/**
> + * drm_dev_enter - Enter device critical section
> + * @dev: DRM device
> + * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
> + *
> + * This function marks and protects the beginning of a section that should not
> + * be entered after the device has been unplugged. The section end is marked
> + * with drm_dev_exit(). Calls to this function can be nested.
> + *
> + * Returns:
> + * True if it is OK to enter the section, false otherwise.
> + */
> +bool drm_dev_enter(struct drm_device *dev, int *idx)
> +{
> + *idx = srcu_read_lock(&drm_unplug_srcu);
> +
> + if (dev->unplugged) {
> + srcu_read_unlock(&drm_unplug_srcu, *idx);
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_dev_enter);
> +
> +/**
> + * drm_dev_exit - Exit device critical section
> + * @idx: index returned from drm_dev_enter()
> + *
> + * This function marks the end of a section that should not be entered after
> + * the device has been unplugged.
> + */
> +void drm_dev_exit(int idx)
> {
> - smp_wmb();
> - atomic_set(&dev->unplugged, 1);
> + srcu_read_unlock(&drm_unplug_srcu, idx);
> }
> +EXPORT_SYMBOL(drm_dev_exit);
>
> /**
> * drm_dev_unplug - unplug a DRM device
> * @dev: DRM device
> *
> * This unplugs a hotpluggable DRM device, which makes it inaccessible to
> - * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
> + * userspace operations. Entry-points can use drm_dev_enter() and
> + * drm_dev_exit() to protect device resources in a race free manner. This
> * essentially unregisters the device like drm_dev_unregister(), but can be
> * called while there are still open users of @dev.
> */
> @@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
> drm_dev_unregister(dev);
>
> mutex_lock(&drm_global_mutex);
> - drm_device_set_unplugged(dev);
> if (dev->open_count == 0)
> drm_dev_put(dev);
> mutex_unlock(&drm_global_mutex);
> +
> + /*
> + * After synchronizing any critical read section is guaranteed to see
> + * the new value of ->unplugged, and any critical section which might
> + * still have seen the old value of ->unplugged is guaranteed to have
> + * finished.
> + */
> + dev->unplugged = true;
> + synchronize_srcu(&drm_unplug_srcu);
> }
> EXPORT_SYMBOL(drm_dev_unplug);
>
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 7c4fa32f3fc6..3a0eac2885b7 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -46,7 +46,14 @@ struct drm_device {
> /* currently active master for this device. Protected by master_mutex */
> struct drm_master *master;
>
> - atomic_t unplugged; /**< Flag whether dev is dead */
> + /**
> + * @unplugged:
> + *
> + * Flag to tell if the device has been unplugged.
> + * See drm_dev_enter() and drm_dev_is_unplugged().
> + */
> + bool unplugged;
> +
> struct inode *anon_inode; /**< inode for private address-space */
> char *unique; /**< unique name of the device */
> /*@} */
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index d23dcdd1bd95..7e545f5f94d3 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
> void drm_dev_put(struct drm_device *dev);
> void drm_dev_unref(struct drm_device *dev);
> void drm_put_dev(struct drm_device *dev);
> +bool drm_dev_enter(struct drm_device *dev, int *idx);
> +void drm_dev_exit(int idx);
> void drm_dev_unplug(struct drm_device *dev);
>
> /**
> @@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
> * unplugged, these two functions guarantee that any store before calling
> * drm_dev_unplug() is visible to callers of this function after it completes
> */
> -static inline int drm_dev_is_unplugged(struct drm_device *dev)
> +static inline bool drm_dev_is_unplugged(struct drm_device *dev)
> {
> - int ret = atomic_read(&dev->unplugged);
> - smp_rmb();
> - return ret;
> + int idx;
> +
> + if (drm_dev_enter(dev, &idx)) {
> + drm_dev_exit(idx);
> + return false;
> + }
> +
> + return true;
> }
>
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Use srcu to protect drm_device.unplugged
@ 2018-03-28 15:09 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2018-03-28 15:09 UTC (permalink / raw)
To: Oleksandr Andrushchenko
Cc: Oleksandr Andrushchenko, airlied, intel-gfx, linux-kernel,
dri-devel, daniel.vetter
On Wed, Mar 28, 2018 at 10:38:35AM +0300, Oleksandr Andrushchenko wrote:
> From: Noralf Trønnes <noralf@tronnes.org>
>
> Use srcu to protect drm_device.unplugged in a race free manner.
> Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
> sections preventing access to device resources that are not available
> after the device is gone.
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Cc: intel-gfx@lists.freedesktop.org
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Oleksandr, please push to drm-misc-next once you have dim tools setup up
and everything.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
> include/drm/drm_device.h | 9 +++++++-
> include/drm/drm_drv.h | 15 +++++++++----
> 3 files changed, 68 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index a1b9338736e3..32a83b41ab61 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -32,6 +32,7 @@
> #include <linux/moduleparam.h>
> #include <linux/mount.h>
> #include <linux/slab.h>
> +#include <linux/srcu.h>
>
> #include <drm/drm_drv.h>
> #include <drm/drmP.h>
> @@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
>
> static struct dentry *drm_debugfs_root;
>
> +DEFINE_STATIC_SRCU(drm_unplug_srcu);
> +
> /*
> * DRM Minors
> * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
> @@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
> }
> EXPORT_SYMBOL(drm_put_dev);
>
> -static void drm_device_set_unplugged(struct drm_device *dev)
> +/**
> + * drm_dev_enter - Enter device critical section
> + * @dev: DRM device
> + * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
> + *
> + * This function marks and protects the beginning of a section that should not
> + * be entered after the device has been unplugged. The section end is marked
> + * with drm_dev_exit(). Calls to this function can be nested.
> + *
> + * Returns:
> + * True if it is OK to enter the section, false otherwise.
> + */
> +bool drm_dev_enter(struct drm_device *dev, int *idx)
> +{
> + *idx = srcu_read_lock(&drm_unplug_srcu);
> +
> + if (dev->unplugged) {
> + srcu_read_unlock(&drm_unplug_srcu, *idx);
> + return false;
> + }
> +
> + return true;
> +}
> +EXPORT_SYMBOL(drm_dev_enter);
> +
> +/**
> + * drm_dev_exit - Exit device critical section
> + * @idx: index returned from drm_dev_enter()
> + *
> + * This function marks the end of a section that should not be entered after
> + * the device has been unplugged.
> + */
> +void drm_dev_exit(int idx)
> {
> - smp_wmb();
> - atomic_set(&dev->unplugged, 1);
> + srcu_read_unlock(&drm_unplug_srcu, idx);
> }
> +EXPORT_SYMBOL(drm_dev_exit);
>
> /**
> * drm_dev_unplug - unplug a DRM device
> * @dev: DRM device
> *
> * This unplugs a hotpluggable DRM device, which makes it inaccessible to
> - * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
> + * userspace operations. Entry-points can use drm_dev_enter() and
> + * drm_dev_exit() to protect device resources in a race free manner. This
> * essentially unregisters the device like drm_dev_unregister(), but can be
> * called while there are still open users of @dev.
> */
> @@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
> drm_dev_unregister(dev);
>
> mutex_lock(&drm_global_mutex);
> - drm_device_set_unplugged(dev);
> if (dev->open_count == 0)
> drm_dev_put(dev);
> mutex_unlock(&drm_global_mutex);
> +
> + /*
> + * After synchronizing any critical read section is guaranteed to see
> + * the new value of ->unplugged, and any critical section which might
> + * still have seen the old value of ->unplugged is guaranteed to have
> + * finished.
> + */
> + dev->unplugged = true;
> + synchronize_srcu(&drm_unplug_srcu);
> }
> EXPORT_SYMBOL(drm_dev_unplug);
>
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 7c4fa32f3fc6..3a0eac2885b7 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -46,7 +46,14 @@ struct drm_device {
> /* currently active master for this device. Protected by master_mutex */
> struct drm_master *master;
>
> - atomic_t unplugged; /**< Flag whether dev is dead */
> + /**
> + * @unplugged:
> + *
> + * Flag to tell if the device has been unplugged.
> + * See drm_dev_enter() and drm_dev_is_unplugged().
> + */
> + bool unplugged;
> +
> struct inode *anon_inode; /**< inode for private address-space */
> char *unique; /**< unique name of the device */
> /*@} */
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index d23dcdd1bd95..7e545f5f94d3 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
> void drm_dev_put(struct drm_device *dev);
> void drm_dev_unref(struct drm_device *dev);
> void drm_put_dev(struct drm_device *dev);
> +bool drm_dev_enter(struct drm_device *dev, int *idx);
> +void drm_dev_exit(int idx);
> void drm_dev_unplug(struct drm_device *dev);
>
> /**
> @@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
> * unplugged, these two functions guarantee that any store before calling
> * drm_dev_unplug() is visible to callers of this function after it completes
> */
> -static inline int drm_dev_is_unplugged(struct drm_device *dev)
> +static inline bool drm_dev_is_unplugged(struct drm_device *dev)
> {
> - int ret = atomic_read(&dev->unplugged);
> - smp_rmb();
> - return ret;
> + int idx;
> +
> + if (drm_dev_enter(dev, &idx)) {
> + drm_dev_exit(idx);
> + return false;
> + }
> +
> + return true;
> }
>
>
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Use srcu to protect drm_device.unplugged
2018-03-28 15:09 ` Daniel Vetter
@ 2018-03-29 11:18 ` Oleksandr Andrushchenko
-1 siblings, 0 replies; 9+ messages in thread
From: Oleksandr Andrushchenko @ 2018-03-29 11:18 UTC (permalink / raw)
To: linux-kernel, dri-devel, airlied, daniel.vetter, seanpaul,
gustavo, intel-gfx, Oleksandr Andrushchenko
On 03/28/2018 06:09 PM, Daniel Vetter wrote:
> On Wed, Mar 28, 2018 at 10:38:35AM +0300, Oleksandr Andrushchenko wrote:
>> From: Noralf Trønnes <noralf@tronnes.org>
>>
>> Use srcu to protect drm_device.unplugged in a race free manner.
>> Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
>> sections preventing access to device resources that are not available
>> after the device is gone.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Cc: intel-gfx@lists.freedesktop.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Oleksandr, please push to drm-misc-next once you have dim tools setup up
> and everything.
Pushed to drm-misc-next
>
> Thanks, Daniel
Thank you,
Oleksandr
>> ---
>> drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
>> include/drm/drm_device.h | 9 +++++++-
>> include/drm/drm_drv.h | 15 +++++++++----
>> 3 files changed, 68 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index a1b9338736e3..32a83b41ab61 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -32,6 +32,7 @@
>> #include <linux/moduleparam.h>
>> #include <linux/mount.h>
>> #include <linux/slab.h>
>> +#include <linux/srcu.h>
>>
>> #include <drm/drm_drv.h>
>> #include <drm/drmP.h>
>> @@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
>>
>> static struct dentry *drm_debugfs_root;
>>
>> +DEFINE_STATIC_SRCU(drm_unplug_srcu);
>> +
>> /*
>> * DRM Minors
>> * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
>> @@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
>> }
>> EXPORT_SYMBOL(drm_put_dev);
>>
>> -static void drm_device_set_unplugged(struct drm_device *dev)
>> +/**
>> + * drm_dev_enter - Enter device critical section
>> + * @dev: DRM device
>> + * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
>> + *
>> + * This function marks and protects the beginning of a section that should not
>> + * be entered after the device has been unplugged. The section end is marked
>> + * with drm_dev_exit(). Calls to this function can be nested.
>> + *
>> + * Returns:
>> + * True if it is OK to enter the section, false otherwise.
>> + */
>> +bool drm_dev_enter(struct drm_device *dev, int *idx)
>> +{
>> + *idx = srcu_read_lock(&drm_unplug_srcu);
>> +
>> + if (dev->unplugged) {
>> + srcu_read_unlock(&drm_unplug_srcu, *idx);
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +EXPORT_SYMBOL(drm_dev_enter);
>> +
>> +/**
>> + * drm_dev_exit - Exit device critical section
>> + * @idx: index returned from drm_dev_enter()
>> + *
>> + * This function marks the end of a section that should not be entered after
>> + * the device has been unplugged.
>> + */
>> +void drm_dev_exit(int idx)
>> {
>> - smp_wmb();
>> - atomic_set(&dev->unplugged, 1);
>> + srcu_read_unlock(&drm_unplug_srcu, idx);
>> }
>> +EXPORT_SYMBOL(drm_dev_exit);
>>
>> /**
>> * drm_dev_unplug - unplug a DRM device
>> * @dev: DRM device
>> *
>> * This unplugs a hotpluggable DRM device, which makes it inaccessible to
>> - * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
>> + * userspace operations. Entry-points can use drm_dev_enter() and
>> + * drm_dev_exit() to protect device resources in a race free manner. This
>> * essentially unregisters the device like drm_dev_unregister(), but can be
>> * called while there are still open users of @dev.
>> */
>> @@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
>> drm_dev_unregister(dev);
>>
>> mutex_lock(&drm_global_mutex);
>> - drm_device_set_unplugged(dev);
>> if (dev->open_count == 0)
>> drm_dev_put(dev);
>> mutex_unlock(&drm_global_mutex);
>> +
>> + /*
>> + * After synchronizing any critical read section is guaranteed to see
>> + * the new value of ->unplugged, and any critical section which might
>> + * still have seen the old value of ->unplugged is guaranteed to have
>> + * finished.
>> + */
>> + dev->unplugged = true;
>> + synchronize_srcu(&drm_unplug_srcu);
>> }
>> EXPORT_SYMBOL(drm_dev_unplug);
>>
>> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> index 7c4fa32f3fc6..3a0eac2885b7 100644
>> --- a/include/drm/drm_device.h
>> +++ b/include/drm/drm_device.h
>> @@ -46,7 +46,14 @@ struct drm_device {
>> /* currently active master for this device. Protected by master_mutex */
>> struct drm_master *master;
>>
>> - atomic_t unplugged; /**< Flag whether dev is dead */
>> + /**
>> + * @unplugged:
>> + *
>> + * Flag to tell if the device has been unplugged.
>> + * See drm_dev_enter() and drm_dev_is_unplugged().
>> + */
>> + bool unplugged;
>> +
>> struct inode *anon_inode; /**< inode for private address-space */
>> char *unique; /**< unique name of the device */
>> /*@} */
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index d23dcdd1bd95..7e545f5f94d3 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
>> void drm_dev_put(struct drm_device *dev);
>> void drm_dev_unref(struct drm_device *dev);
>> void drm_put_dev(struct drm_device *dev);
>> +bool drm_dev_enter(struct drm_device *dev, int *idx);
>> +void drm_dev_exit(int idx);
>> void drm_dev_unplug(struct drm_device *dev);
>>
>> /**
>> @@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
>> * unplugged, these two functions guarantee that any store before calling
>> * drm_dev_unplug() is visible to callers of this function after it completes
>> */
>> -static inline int drm_dev_is_unplugged(struct drm_device *dev)
>> +static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>> {
>> - int ret = atomic_read(&dev->unplugged);
>> - smp_rmb();
>> - return ret;
>> + int idx;
>> +
>> + if (drm_dev_enter(dev, &idx)) {
>> + drm_dev_exit(idx);
>> + return false;
>> + }
>> +
>> + return true;
>> }
>>
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Use srcu to protect drm_device.unplugged
@ 2018-03-29 11:18 ` Oleksandr Andrushchenko
0 siblings, 0 replies; 9+ messages in thread
From: Oleksandr Andrushchenko @ 2018-03-29 11:18 UTC (permalink / raw)
To: linux-kernel, dri-devel, airlied, daniel.vetter, seanpaul,
gustavo, intel-gfx, Oleksandr Andrushchenko
On 03/28/2018 06:09 PM, Daniel Vetter wrote:
> On Wed, Mar 28, 2018 at 10:38:35AM +0300, Oleksandr Andrushchenko wrote:
>> From: Noralf Trønnes <noralf@tronnes.org>
>>
>> Use srcu to protect drm_device.unplugged in a race free manner.
>> Drivers can use drm_dev_enter()/drm_dev_exit() to protect and mark
>> sections preventing access to device resources that are not available
>> after the device is gone.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Tested-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> Cc: intel-gfx@lists.freedesktop.org
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Oleksandr, please push to drm-misc-next once you have dim tools setup up
> and everything.
Pushed to drm-misc-next
>
> Thanks, Daniel
Thank you,
Oleksandr
>> ---
>> drivers/gpu/drm/drm_drv.c | 54 ++++++++++++++++++++++++++++++++++++++++++-----
>> include/drm/drm_device.h | 9 +++++++-
>> include/drm/drm_drv.h | 15 +++++++++----
>> 3 files changed, 68 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index a1b9338736e3..32a83b41ab61 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -32,6 +32,7 @@
>> #include <linux/moduleparam.h>
>> #include <linux/mount.h>
>> #include <linux/slab.h>
>> +#include <linux/srcu.h>
>>
>> #include <drm/drm_drv.h>
>> #include <drm/drmP.h>
>> @@ -75,6 +76,8 @@ static bool drm_core_init_complete = false;
>>
>> static struct dentry *drm_debugfs_root;
>>
>> +DEFINE_STATIC_SRCU(drm_unplug_srcu);
>> +
>> /*
>> * DRM Minors
>> * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
>> @@ -318,18 +321,51 @@ void drm_put_dev(struct drm_device *dev)
>> }
>> EXPORT_SYMBOL(drm_put_dev);
>>
>> -static void drm_device_set_unplugged(struct drm_device *dev)
>> +/**
>> + * drm_dev_enter - Enter device critical section
>> + * @dev: DRM device
>> + * @idx: Pointer to index that will be passed to the matching drm_dev_exit()
>> + *
>> + * This function marks and protects the beginning of a section that should not
>> + * be entered after the device has been unplugged. The section end is marked
>> + * with drm_dev_exit(). Calls to this function can be nested.
>> + *
>> + * Returns:
>> + * True if it is OK to enter the section, false otherwise.
>> + */
>> +bool drm_dev_enter(struct drm_device *dev, int *idx)
>> +{
>> + *idx = srcu_read_lock(&drm_unplug_srcu);
>> +
>> + if (dev->unplugged) {
>> + srcu_read_unlock(&drm_unplug_srcu, *idx);
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +EXPORT_SYMBOL(drm_dev_enter);
>> +
>> +/**
>> + * drm_dev_exit - Exit device critical section
>> + * @idx: index returned from drm_dev_enter()
>> + *
>> + * This function marks the end of a section that should not be entered after
>> + * the device has been unplugged.
>> + */
>> +void drm_dev_exit(int idx)
>> {
>> - smp_wmb();
>> - atomic_set(&dev->unplugged, 1);
>> + srcu_read_unlock(&drm_unplug_srcu, idx);
>> }
>> +EXPORT_SYMBOL(drm_dev_exit);
>>
>> /**
>> * drm_dev_unplug - unplug a DRM device
>> * @dev: DRM device
>> *
>> * This unplugs a hotpluggable DRM device, which makes it inaccessible to
>> - * userspace operations. Entry-points can use drm_dev_is_unplugged(). This
>> + * userspace operations. Entry-points can use drm_dev_enter() and
>> + * drm_dev_exit() to protect device resources in a race free manner. This
>> * essentially unregisters the device like drm_dev_unregister(), but can be
>> * called while there are still open users of @dev.
>> */
>> @@ -338,10 +374,18 @@ void drm_dev_unplug(struct drm_device *dev)
>> drm_dev_unregister(dev);
>>
>> mutex_lock(&drm_global_mutex);
>> - drm_device_set_unplugged(dev);
>> if (dev->open_count == 0)
>> drm_dev_put(dev);
>> mutex_unlock(&drm_global_mutex);
>> +
>> + /*
>> + * After synchronizing any critical read section is guaranteed to see
>> + * the new value of ->unplugged, and any critical section which might
>> + * still have seen the old value of ->unplugged is guaranteed to have
>> + * finished.
>> + */
>> + dev->unplugged = true;
>> + synchronize_srcu(&drm_unplug_srcu);
>> }
>> EXPORT_SYMBOL(drm_dev_unplug);
>>
>> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> index 7c4fa32f3fc6..3a0eac2885b7 100644
>> --- a/include/drm/drm_device.h
>> +++ b/include/drm/drm_device.h
>> @@ -46,7 +46,14 @@ struct drm_device {
>> /* currently active master for this device. Protected by master_mutex */
>> struct drm_master *master;
>>
>> - atomic_t unplugged; /**< Flag whether dev is dead */
>> + /**
>> + * @unplugged:
>> + *
>> + * Flag to tell if the device has been unplugged.
>> + * See drm_dev_enter() and drm_dev_is_unplugged().
>> + */
>> + bool unplugged;
>> +
>> struct inode *anon_inode; /**< inode for private address-space */
>> char *unique; /**< unique name of the device */
>> /*@} */
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index d23dcdd1bd95..7e545f5f94d3 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -624,6 +624,8 @@ void drm_dev_get(struct drm_device *dev);
>> void drm_dev_put(struct drm_device *dev);
>> void drm_dev_unref(struct drm_device *dev);
>> void drm_put_dev(struct drm_device *dev);
>> +bool drm_dev_enter(struct drm_device *dev, int *idx);
>> +void drm_dev_exit(int idx);
>> void drm_dev_unplug(struct drm_device *dev);
>>
>> /**
>> @@ -635,11 +637,16 @@ void drm_dev_unplug(struct drm_device *dev);
>> * unplugged, these two functions guarantee that any store before calling
>> * drm_dev_unplug() is visible to callers of this function after it completes
>> */
>> -static inline int drm_dev_is_unplugged(struct drm_device *dev)
>> +static inline bool drm_dev_is_unplugged(struct drm_device *dev)
>> {
>> - int ret = atomic_read(&dev->unplugged);
>> - smp_rmb();
>> - return ret;
>> + int idx;
>> +
>> + if (drm_dev_enter(dev, &idx)) {
>> + drm_dev_exit(idx);
>> + return false;
>> + }
>> +
>> + return true;
>> }
>>
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-29 11:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 7:38 [PATCH] drm: Use srcu to protect drm_device.unplugged Oleksandr Andrushchenko
2018-03-28 7:38 ` Oleksandr Andrushchenko
2018-03-28 7:54 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
2018-03-28 8:08 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-28 13:21 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-28 15:09 ` [PATCH] " Daniel Vetter
2018-03-28 15:09 ` Daniel Vetter
2018-03-29 11:18 ` Oleksandr Andrushchenko
2018-03-29 11:18 ` Oleksandr Andrushchenko
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.