[PATCH 0/5] disable drm_global_mutex for most drivers

[PATCH 0/5] disable drm_global_mutex for most drivers

[Daniel Vetter]

Hi all

New version without the deadlock in pan_display that Noralf spotted
(hopefully that appeases CI a bit, v1 was bad), plus now being co-tested
with the right igt series (hopefully).

Review (especially but not only on the igt side) very much appreciated.

Test-With: 20200128112549.172135-1-daniel.vetter@ffwll.ch

igt series link https://patchwork.freedesktop.org/series/72654/

Cheers, Daniel

Daniel Vetter (5):
  drm: Complain if drivers still use the ->load callback
  drm/fbdev-helper: don't force restores
  drm/client: Rename _force to _locked
  drm: Push drm_global_mutex locking in drm_open
  drm: Nerv drm_global_mutex BKL for good drivers

 drivers/gpu/drm/drm_client_modeset.c | 12 ++++----
 drivers/gpu/drm/drm_drv.c            | 26 +++++++++-------
 drivers/gpu/drm/drm_fb_helper.c      | 16 ++--------
 drivers/gpu/drm/drm_file.c           | 44 +++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_internal.h       |  1 +
 include/drm/drm_client.h             |  7 ++++-
 include/drm/drm_drv.h                |  3 ++
 7 files changed, 78 insertions(+), 31 deletions(-)

-- 
2.24.1

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

[Intel-gfx] [PATCH 0/5] disable drm_global_mutex for most drivers

[Daniel Vetter]

Hi all

New version without the deadlock in pan_display that Noralf spotted
(hopefully that appeases CI a bit, v1 was bad), plus now being co-tested
with the right igt series (hopefully).

Review (especially but not only on the igt side) very much appreciated.

Test-With: 20200128112549.172135-1-daniel.vetter@ffwll.ch

igt series link https://patchwork.freedesktop.org/series/72654/

Cheers, Daniel

Daniel Vetter (5):
  drm: Complain if drivers still use the ->load callback
  drm/fbdev-helper: don't force restores
  drm/client: Rename _force to _locked
  drm: Push drm_global_mutex locking in drm_open
  drm: Nerv drm_global_mutex BKL for good drivers

 drivers/gpu/drm/drm_client_modeset.c | 12 ++++----
 drivers/gpu/drm/drm_drv.c            | 26 +++++++++-------
 drivers/gpu/drm/drm_fb_helper.c      | 16 ++--------
 drivers/gpu/drm/drm_file.c           | 44 +++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_internal.h       |  1 +
 include/drm/drm_client.h             |  7 ++++-
 include/drm/drm_drv.h                |  3 ++
 7 files changed, 78 insertions(+), 31 deletions(-)

-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/5] drm: Complain if drivers still use the ->load callback

[Daniel Vetter]

Kinda time to get this sorted. The locking around this really is not
nice.

Thomas mentioned in his review that the only drivers left unconverted
are radeon and amdgpu.

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 6 ++++++
 include/drm/drm_drv.h     | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7c18a980cd4b..8deff75b484c 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 
 	mutex_lock(&drm_global_mutex);
 
+	if (dev->driver->load) {
+		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
+			DRM_INFO("drm driver %s is using deprecated ->load callback\n",
+				 dev->driver->name);
+	}
+
 	ret = drm_minor_register(dev, DRM_MINOR_RENDER);
 	if (ret)
 		goto err_minors;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 77685ed7aa65..77bc63de0a91 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -173,6 +173,9 @@ struct drm_driver {
 	 *
 	 * This is deprecated, do not use!
 	 *
+	 * FIXME: A few non-DRIVER_LEGACY drivers still use this, and should be
+	 * converted.
+	 *
 	 * Returns:
 	 *
 	 * Zero on success, non-zero value on failure.
-- 
2.24.1

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

[Intel-gfx] [PATCH 1/5] drm: Complain if drivers still use the ->load callback

[Daniel Vetter]

Kinda time to get this sorted. The locking around this really is not
nice.

Thomas mentioned in his review that the only drivers left unconverted
are radeon and amdgpu.

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 6 ++++++
 include/drm/drm_drv.h     | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7c18a980cd4b..8deff75b484c 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -948,6 +948,12 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 
 	mutex_lock(&drm_global_mutex);
 
+	if (dev->driver->load) {
+		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
+			DRM_INFO("drm driver %s is using deprecated ->load callback\n",
+				 dev->driver->name);
+	}
+
 	ret = drm_minor_register(dev, DRM_MINOR_RENDER);
 	if (ret)
 		goto err_minors;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 77685ed7aa65..77bc63de0a91 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -173,6 +173,9 @@ struct drm_driver {
 	 *
 	 * This is deprecated, do not use!
 	 *
+	 * FIXME: A few non-DRIVER_LEGACY drivers still use this, and should be
+	 * converted.
+	 *
 	 * Returns:
 	 *
 	 * Zero on success, non-zero value on failure.
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/fbdev-helper: don't force restores

[Daniel Vetter]

Instead check for master status, in case we've raced.

This is the last exception to the general rule that we restore fbcon
only when there's no master active. Compositors are supposed to drop
their master status before they switch to a different console back to
text mode (or just switch to text mode directly, without a vt switch).

This is known to break some subtests of kms_fbcon_fbt in igt, but they're
just wrong - it does a graphics/text mode switch for the vt without
updating the master status.

Also add a comment to the drm_client->restore hook that this is expected
going forward from all clients (there's currently just one).

v2: Also drop the force in pan_display

v3: Restore the _force to pan_display, this actually means _locked in that
path. Spotted by Noralf.

Cc: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 12 +-----------
 include/drm/drm_client.h        |  5 +++++
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4c7cbce7bae7..672934e0eeed 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -250,17 +250,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	/*
-	 * TODO:
-	 * We should bail out here if there is a master by dropping _force.
-	 * Currently these igt tests fail if we do that:
-	 * - kms_fbcon_fbt@psr
-	 * - kms_fbcon_fbt@psr-suspend
-	 *
-	 * So first these tests need to be fixed so they drop master or don't
-	 * have an fd open.
-	 */
-	ret = drm_client_modeset_commit_force(&fb_helper->client);
+	ret = drm_client_modeset_commit(&fb_helper->client);
 
 	do_delayed = fb_helper->delayed_hotplug;
 	if (do_delayed)
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 5cf2c5dd8b1e..d01d311023ac 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -44,6 +44,11 @@ struct drm_client_funcs {
 	 * returns zero gets the privilege to restore and no more clients are
 	 * called. This callback is not called after @unregister has been called.
 	 *
+	 * Note that the core does not guarantee exclusion against concurrent
+	 * drm_open(). Clients need to ensure this themselves, for example by
+	 * using drm_master_internal_acquire() and
+	 * drm_master_internal_release().
+	 *
 	 * This callback is optional.
 	 */
 	int (*restore)(struct drm_client_dev *client);
-- 
2.24.1

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

[Intel-gfx] [PATCH 2/5] drm/fbdev-helper: don't force restores

[Daniel Vetter]

Instead check for master status, in case we've raced.

This is the last exception to the general rule that we restore fbcon
only when there's no master active. Compositors are supposed to drop
their master status before they switch to a different console back to
text mode (or just switch to text mode directly, without a vt switch).

This is known to break some subtests of kms_fbcon_fbt in igt, but they're
just wrong - it does a graphics/text mode switch for the vt without
updating the master status.

Also add a comment to the drm_client->restore hook that this is expected
going forward from all clients (there's currently just one).

v2: Also drop the force in pan_display

v3: Restore the _force to pan_display, this actually means _locked in that
path. Spotted by Noralf.

Cc: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 12 +-----------
 include/drm/drm_client.h        |  5 +++++
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4c7cbce7bae7..672934e0eeed 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -250,17 +250,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 		return 0;
 
 	mutex_lock(&fb_helper->lock);
-	/*
-	 * TODO:
-	 * We should bail out here if there is a master by dropping _force.
-	 * Currently these igt tests fail if we do that:
-	 * - kms_fbcon_fbt@psr
-	 * - kms_fbcon_fbt@psr-suspend
-	 *
-	 * So first these tests need to be fixed so they drop master or don't
-	 * have an fd open.
-	 */
-	ret = drm_client_modeset_commit_force(&fb_helper->client);
+	ret = drm_client_modeset_commit(&fb_helper->client);
 
 	do_delayed = fb_helper->delayed_hotplug;
 	if (do_delayed)
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 5cf2c5dd8b1e..d01d311023ac 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -44,6 +44,11 @@ struct drm_client_funcs {
 	 * returns zero gets the privilege to restore and no more clients are
 	 * called. This callback is not called after @unregister has been called.
 	 *
+	 * Note that the core does not guarantee exclusion against concurrent
+	 * drm_open(). Clients need to ensure this themselves, for example by
+	 * using drm_master_internal_acquire() and
+	 * drm_master_internal_release().
+	 *
 	 * This callback is optional.
 	 */
 	int (*restore)(struct drm_client_dev *client);
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/5] drm/client: Rename _force to _locked

[Daniel Vetter]

Plus extend the kerneldoc a bit to explain how this should be used.
With the previous patch to drop the force restore the main user of
this function is not emphasis on the "I hold the internal master lock
already" aspect, so rename the function to match.

Suggested by Noralf.

Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_client_modeset.c | 12 +++++++-----
 drivers/gpu/drm/drm_fb_helper.c      |  4 ++--
 include/drm/drm_client.h             |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 6d4a29e99ae2..841794e19eb6 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1094,15 +1094,17 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
 }
 
 /**
- * drm_client_modeset_commit_force() - Force commit CRTC configuration
+ * drm_client_modeset_commit_locked() - Force commit CRTC configuration
  * @client: DRM client
  *
- * Commit modeset configuration to crtcs without checking if there is a DRM master.
+ * Commit modeset configuration to crtcs without checking if there is a DRM
+ * master. The assumption is that the caller already holds an internal DRM
+ * master reference acquired with drm_master_internal_acquire().
  *
  * Returns:
  * Zero on success or negative error code on failure.
  */
-int drm_client_modeset_commit_force(struct drm_client_dev *client)
+int drm_client_modeset_commit_locked(struct drm_client_dev *client)
 {
 	struct drm_device *dev = client->dev;
 	int ret;
@@ -1116,7 +1118,7 @@ int drm_client_modeset_commit_force(struct drm_client_dev *client)
 
 	return ret;
 }
-EXPORT_SYMBOL(drm_client_modeset_commit_force);
+EXPORT_SYMBOL(drm_client_modeset_commit_locked);
 
 /**
  * drm_client_modeset_commit() - Commit CRTC configuration
@@ -1135,7 +1137,7 @@ int drm_client_modeset_commit(struct drm_client_dev *client)
 	if (!drm_master_internal_acquire(dev))
 		return -EBUSY;
 
-	ret = drm_client_modeset_commit_force(client);
+	ret = drm_client_modeset_commit_locked(client);
 
 	drm_master_internal_release(dev);
 
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 672934e0eeed..490a99de6ec1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -284,7 +284,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
 			continue;
 
 		mutex_lock(&helper->lock);
-		ret = drm_client_modeset_commit_force(&helper->client);
+		ret = drm_client_modeset_commit_locked(&helper->client);
 		if (ret)
 			error = true;
 		mutex_unlock(&helper->lock);
@@ -1347,7 +1347,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 
 	pan_set(fb_helper, var->xoffset, var->yoffset);
 
-	ret = drm_client_modeset_commit_force(&fb_helper->client);
+	ret = drm_client_modeset_commit_locked(&fb_helper->client);
 	if (!ret) {
 		info->var.xoffset = var->xoffset;
 		info->var.yoffset = var->yoffset;
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index d01d311023ac..3ed5dee899fd 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -161,7 +161,7 @@ int drm_client_modeset_create(struct drm_client_dev *client);
 void drm_client_modeset_free(struct drm_client_dev *client);
 int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
 bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
-int drm_client_modeset_commit_force(struct drm_client_dev *client);
+int drm_client_modeset_commit_locked(struct drm_client_dev *client);
 int drm_client_modeset_commit(struct drm_client_dev *client);
 int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
 
-- 
2.24.1

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

[Intel-gfx] [PATCH 3/5] drm/client: Rename _force to _locked

[Daniel Vetter]

Plus extend the kerneldoc a bit to explain how this should be used.
With the previous patch to drop the force restore the main user of
this function is not emphasis on the "I hold the internal master lock
already" aspect, so rename the function to match.

Suggested by Noralf.

Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_client_modeset.c | 12 +++++++-----
 drivers/gpu/drm/drm_fb_helper.c      |  4 ++--
 include/drm/drm_client.h             |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 6d4a29e99ae2..841794e19eb6 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1094,15 +1094,17 @@ static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
 }
 
 /**
- * drm_client_modeset_commit_force() - Force commit CRTC configuration
+ * drm_client_modeset_commit_locked() - Force commit CRTC configuration
  * @client: DRM client
  *
- * Commit modeset configuration to crtcs without checking if there is a DRM master.
+ * Commit modeset configuration to crtcs without checking if there is a DRM
+ * master. The assumption is that the caller already holds an internal DRM
+ * master reference acquired with drm_master_internal_acquire().
  *
  * Returns:
  * Zero on success or negative error code on failure.
  */
-int drm_client_modeset_commit_force(struct drm_client_dev *client)
+int drm_client_modeset_commit_locked(struct drm_client_dev *client)
 {
 	struct drm_device *dev = client->dev;
 	int ret;
@@ -1116,7 +1118,7 @@ int drm_client_modeset_commit_force(struct drm_client_dev *client)
 
 	return ret;
 }
-EXPORT_SYMBOL(drm_client_modeset_commit_force);
+EXPORT_SYMBOL(drm_client_modeset_commit_locked);
 
 /**
  * drm_client_modeset_commit() - Commit CRTC configuration
@@ -1135,7 +1137,7 @@ int drm_client_modeset_commit(struct drm_client_dev *client)
 	if (!drm_master_internal_acquire(dev))
 		return -EBUSY;
 
-	ret = drm_client_modeset_commit_force(client);
+	ret = drm_client_modeset_commit_locked(client);
 
 	drm_master_internal_release(dev);
 
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 672934e0eeed..490a99de6ec1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -284,7 +284,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
 			continue;
 
 		mutex_lock(&helper->lock);
-		ret = drm_client_modeset_commit_force(&helper->client);
+		ret = drm_client_modeset_commit_locked(&helper->client);
 		if (ret)
 			error = true;
 		mutex_unlock(&helper->lock);
@@ -1347,7 +1347,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
 
 	pan_set(fb_helper, var->xoffset, var->yoffset);
 
-	ret = drm_client_modeset_commit_force(&fb_helper->client);
+	ret = drm_client_modeset_commit_locked(&fb_helper->client);
 	if (!ret) {
 		info->var.xoffset = var->xoffset;
 		info->var.yoffset = var->yoffset;
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index d01d311023ac..3ed5dee899fd 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -161,7 +161,7 @@ int drm_client_modeset_create(struct drm_client_dev *client);
 void drm_client_modeset_free(struct drm_client_dev *client);
 int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width, unsigned int height);
 bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
-int drm_client_modeset_commit_force(struct drm_client_dev *client);
+int drm_client_modeset_commit_locked(struct drm_client_dev *client);
 int drm_client_modeset_commit(struct drm_client_dev *client);
 int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
 
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/client: Rename _force to _locked

[Noralf Trønnes]

Den 29.01.2020 09.24, skrev Daniel Vetter:
> Plus extend the kerneldoc a bit to explain how this should be used.
> With the previous patch to drop the force restore the main user of
> this function is not emphasis on the "I hold the internal master lock

The _not_ confuses me, the emphasis is now that "I hold the lock" right?

> already" aspect, so rename the function to match.
> 
> Suggested by Noralf.
> 
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 3/5] drm/client: Rename _force to _locked

[Noralf Trønnes]

Den 29.01.2020 09.24, skrev Daniel Vetter:
> Plus extend the kerneldoc a bit to explain how this should be used.
> With the previous patch to drop the force restore the main user of
> this function is not emphasis on the "I hold the internal master lock

The _not_ confuses me, the emphasis is now that "I hold the lock" right?

> already" aspect, so rename the function to match.
> 
> Suggested by Noralf.
> 
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---

Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/client: Rename _force to _locked

[Daniel Vetter]

On Wed, Jan 29, 2020 at 2:16 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 29.01.2020 09.24, skrev Daniel Vetter:
> > Plus extend the kerneldoc a bit to explain how this should be used.
> > With the previous patch to drop the force restore the main user of
> > this function is not emphasis on the "I hold the internal master lock
>
> The _not_ confuses me, the emphasis is now that "I hold the lock" right?

Oops, I confused myself and mixed things up between old and new
meaning and what's now important and what not. s/not/now/, I'll fix
when applying.
-Daniel

>
> > already" aspect, so rename the function to match.
> >
> > Suggested by Noralf.
> >
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 3/5] drm/client: Rename _force to _locked

[Daniel Vetter]

On Wed, Jan 29, 2020 at 2:16 PM Noralf Trønnes <noralf@tronnes.org> wrote:
> Den 29.01.2020 09.24, skrev Daniel Vetter:
> > Plus extend the kerneldoc a bit to explain how this should be used.
> > With the previous patch to drop the force restore the main user of
> > this function is not emphasis on the "I hold the internal master lock
>
> The _not_ confuses me, the emphasis is now that "I hold the lock" right?

Oops, I confused myself and mixed things up between old and new
meaning and what's now important and what not. s/not/now/, I'll fix
when applying.
-Daniel

>
> > already" aspect, so rename the function to match.
> >
> > Suggested by Noralf.
> >
> > Cc: Noralf Trønnes <noralf@tronnes.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

We want to only take the BKL on crap drivers, but to know whether
we have a crap driver we first need to look it up. Split this shuffle
out from the main BKL-disabling patch, for more clarity.

Since the minors are refcounted drm_minor_acquire is purely internal
and this does not have a driver visible effect.

v2: Push the locking even further into drm_open(), suggested by Chris.
This gives us more symmetry with drm_release(), and maybe a futuer
avenue where we make drm_globale_mutex locking (partially) opt-in like
with drm_release_noglobal().

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c  | 14 +++++---------
 drivers/gpu/drm/drm_file.c |  6 ++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8deff75b484c..05bdf0b9d2b3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 
 	DRM_DEBUG("\n");
 
-	mutex_lock(&drm_global_mutex);
 	minor = drm_minor_acquire(iminor(inode));
-	if (IS_ERR(minor)) {
-		err = PTR_ERR(minor);
-		goto out_unlock;
-	}
+	if (IS_ERR(minor))
+		return PTR_ERR(minor);
 
 	new_fops = fops_get(minor->dev->driver->fops);
 	if (!new_fops) {
 		err = -ENODEV;
-		goto out_release;
+		goto out;
 	}
 
 	replace_fops(filp, new_fops);
@@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 	else
 		err = 0;
 
-out_release:
+out:
 	drm_minor_release(minor);
-out_unlock:
-	mutex_unlock(&drm_global_mutex);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 1075b3a8b5b1..d36cb74ebe0c 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
+	mutex_unlock(&drm_global_mutex);
+
 	dev = minor->dev;
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
@@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
 			goto err_undo;
 		}
 	}
+
+	mutex_unlock(&drm_global_mutex);
+
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
+	mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
-- 
2.24.1

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

[Intel-gfx] [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

We want to only take the BKL on crap drivers, but to know whether
we have a crap driver we first need to look it up. Split this shuffle
out from the main BKL-disabling patch, for more clarity.

Since the minors are refcounted drm_minor_acquire is purely internal
and this does not have a driver visible effect.

v2: Push the locking even further into drm_open(), suggested by Chris.
This gives us more symmetry with drm_release(), and maybe a futuer
avenue where we make drm_globale_mutex locking (partially) opt-in like
with drm_release_noglobal().

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c  | 14 +++++---------
 drivers/gpu/drm/drm_file.c |  6 ++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8deff75b484c..05bdf0b9d2b3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 
 	DRM_DEBUG("\n");
 
-	mutex_lock(&drm_global_mutex);
 	minor = drm_minor_acquire(iminor(inode));
-	if (IS_ERR(minor)) {
-		err = PTR_ERR(minor);
-		goto out_unlock;
-	}
+	if (IS_ERR(minor))
+		return PTR_ERR(minor);
 
 	new_fops = fops_get(minor->dev->driver->fops);
 	if (!new_fops) {
 		err = -ENODEV;
-		goto out_release;
+		goto out;
 	}
 
 	replace_fops(filp, new_fops);
@@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 	else
 		err = 0;
 
-out_release:
+out:
 	drm_minor_release(minor);
-out_unlock:
-	mutex_unlock(&drm_global_mutex);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 1075b3a8b5b1..d36cb74ebe0c 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
+	mutex_unlock(&drm_global_mutex);
+
 	dev = minor->dev;
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
@@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
 			goto err_undo;
 		}
 	}
+
+	mutex_unlock(&drm_global_mutex);
+
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
+	mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Sam Ravnborg]

Hi Daniel.

On Wed, Jan 29, 2020 at 09:24:09AM +0100, Daniel Vetter wrote:
> We want to only take the BKL on crap drivers, but to know whether
The BKL was killed long time ago..
In other words I was confused until I realized that
- BKL
- drm_global_mutex BKL
- drm_global_mutex

Was all the same. At least my OCD color me confused as is.

> we have a crap driver we first need to look it up. Split this shuffle
> out from the main BKL-disabling patch, for more clarity.
> 
> Since the minors are refcounted drm_minor_acquire is purely internal
> and this does not have a driver visible effect.
> 
> v2: Push the locking even further into drm_open(), suggested by Chris.
> This gives us more symmetry with drm_release(), and maybe a futuer
> avenue where we make drm_globale_mutex locking (partially) opt-in like
s/drm_globale_mutex/drm_global_mutex/

> with drm_release_noglobal().
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Above is IMO fix-while-committing stuff.

	Sam

> ---
>  drivers/gpu/drm/drm_drv.c  | 14 +++++---------
>  drivers/gpu/drm/drm_file.c |  6 ++++++
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8deff75b484c..05bdf0b9d2b3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
>  
>  	DRM_DEBUG("\n");
>  
> -	mutex_lock(&drm_global_mutex);
>  	minor = drm_minor_acquire(iminor(inode));
> -	if (IS_ERR(minor)) {
> -		err = PTR_ERR(minor);
> -		goto out_unlock;
> -	}
> +	if (IS_ERR(minor))
> +		return PTR_ERR(minor);
>  
>  	new_fops = fops_get(minor->dev->driver->fops);
>  	if (!new_fops) {
>  		err = -ENODEV;
> -		goto out_release;
> +		goto out;
>  	}
>  
>  	replace_fops(filp, new_fops);
> @@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
>  	else
>  		err = 0;
>  
> -out_release:
> +out:
>  	drm_minor_release(minor);
> -out_unlock:
> -	mutex_unlock(&drm_global_mutex);
> +
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 1075b3a8b5b1..d36cb74ebe0c 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
>  	if (IS_ERR(minor))
>  		return PTR_ERR(minor);
>  
> +	mutex_unlock(&drm_global_mutex);
> +
>  	dev = minor->dev;
>  	if (!atomic_fetch_inc(&dev->open_count))
>  		need_setup = 1;
> @@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
>  			goto err_undo;
>  		}
>  	}
> +
> +	mutex_unlock(&drm_global_mutex);
> +
>  	return 0;
>  
>  err_undo:
>  	atomic_dec(&dev->open_count);
> +	mutex_unlock(&drm_global_mutex);
>  	drm_minor_release(minor);
>  	return retcode;
>  }
> -- 
> 2.24.1
> 
> _______________________________________________
> 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

Re: [Intel-gfx] [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Sam Ravnborg]

Hi Daniel.

On Wed, Jan 29, 2020 at 09:24:09AM +0100, Daniel Vetter wrote:
> We want to only take the BKL on crap drivers, but to know whether
The BKL was killed long time ago..
In other words I was confused until I realized that
- BKL
- drm_global_mutex BKL
- drm_global_mutex

Was all the same. At least my OCD color me confused as is.

> we have a crap driver we first need to look it up. Split this shuffle
> out from the main BKL-disabling patch, for more clarity.
> 
> Since the minors are refcounted drm_minor_acquire is purely internal
> and this does not have a driver visible effect.
> 
> v2: Push the locking even further into drm_open(), suggested by Chris.
> This gives us more symmetry with drm_release(), and maybe a futuer
> avenue where we make drm_globale_mutex locking (partially) opt-in like
s/drm_globale_mutex/drm_global_mutex/

> with drm_release_noglobal().
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Above is IMO fix-while-committing stuff.

	Sam

> ---
>  drivers/gpu/drm/drm_drv.c  | 14 +++++---------
>  drivers/gpu/drm/drm_file.c |  6 ++++++
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8deff75b484c..05bdf0b9d2b3 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
>  
>  	DRM_DEBUG("\n");
>  
> -	mutex_lock(&drm_global_mutex);
>  	minor = drm_minor_acquire(iminor(inode));
> -	if (IS_ERR(minor)) {
> -		err = PTR_ERR(minor);
> -		goto out_unlock;
> -	}
> +	if (IS_ERR(minor))
> +		return PTR_ERR(minor);
>  
>  	new_fops = fops_get(minor->dev->driver->fops);
>  	if (!new_fops) {
>  		err = -ENODEV;
> -		goto out_release;
> +		goto out;
>  	}
>  
>  	replace_fops(filp, new_fops);
> @@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
>  	else
>  		err = 0;
>  
> -out_release:
> +out:
>  	drm_minor_release(minor);
> -out_unlock:
> -	mutex_unlock(&drm_global_mutex);
> +
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 1075b3a8b5b1..d36cb74ebe0c 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
>  	if (IS_ERR(minor))
>  		return PTR_ERR(minor);
>  
> +	mutex_unlock(&drm_global_mutex);
> +
>  	dev = minor->dev;
>  	if (!atomic_fetch_inc(&dev->open_count))
>  		need_setup = 1;
> @@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
>  			goto err_undo;
>  		}
>  	}
> +
> +	mutex_unlock(&drm_global_mutex);
> +
>  	return 0;
>  
>  err_undo:
>  	atomic_dec(&dev->open_count);
> +	mutex_unlock(&drm_global_mutex);
>  	drm_minor_release(minor);
>  	return retcode;
>  }
> -- 
> 2.24.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

On Wed, Jan 29, 2020 at 05:45:45PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> 
> On Wed, Jan 29, 2020 at 09:24:09AM +0100, Daniel Vetter wrote:
> > We want to only take the BKL on crap drivers, but to know whether
> The BKL was killed long time ago..
> In other words I was confused until I realized that
> - BKL
> - drm_global_mutex BKL
> - drm_global_mutex
> 
> Was all the same. At least my OCD color me confused as is.

The Real BKL was converted into drm_global_mutex for drm when the BKL was
killed. Which is kinda relevant, because the BKL locking horrors (at least
in drm) have been inherited by drm_global_mutex (and the conversion broke
a few things that had to be papered over). Hence the motivation to finally
clean up the locking and figure out what exactly is still protected by
this lock. If you're bored, all this is at least in modern git history
afaics.
-Daniel

> 
> > we have a crap driver we first need to look it up. Split this shuffle
> > out from the main BKL-disabling patch, for more clarity.
> > 
> > Since the minors are refcounted drm_minor_acquire is purely internal
> > and this does not have a driver visible effect.
> > 
> > v2: Push the locking even further into drm_open(), suggested by Chris.
> > This gives us more symmetry with drm_release(), and maybe a futuer
> > avenue where we make drm_globale_mutex locking (partially) opt-in like
> s/drm_globale_mutex/drm_global_mutex/
> 
> > with drm_release_noglobal().
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Above is IMO fix-while-committing stuff.
> 
> 	Sam
> 
> > ---
> >  drivers/gpu/drm/drm_drv.c  | 14 +++++---------
> >  drivers/gpu/drm/drm_file.c |  6 ++++++
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 8deff75b484c..05bdf0b9d2b3 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
> >  
> >  	DRM_DEBUG("\n");
> >  
> > -	mutex_lock(&drm_global_mutex);
> >  	minor = drm_minor_acquire(iminor(inode));
> > -	if (IS_ERR(minor)) {
> > -		err = PTR_ERR(minor);
> > -		goto out_unlock;
> > -	}
> > +	if (IS_ERR(minor))
> > +		return PTR_ERR(minor);
> >  
> >  	new_fops = fops_get(minor->dev->driver->fops);
> >  	if (!new_fops) {
> >  		err = -ENODEV;
> > -		goto out_release;
> > +		goto out;
> >  	}
> >  
> >  	replace_fops(filp, new_fops);
> > @@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
> >  	else
> >  		err = 0;
> >  
> > -out_release:
> > +out:
> >  	drm_minor_release(minor);
> > -out_unlock:
> > -	mutex_unlock(&drm_global_mutex);
> > +
> >  	return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > index 1075b3a8b5b1..d36cb74ebe0c 100644
> > --- a/drivers/gpu/drm/drm_file.c
> > +++ b/drivers/gpu/drm/drm_file.c
> > @@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
> >  	if (IS_ERR(minor))
> >  		return PTR_ERR(minor);
> >  
> > +	mutex_unlock(&drm_global_mutex);
> > +
> >  	dev = minor->dev;
> >  	if (!atomic_fetch_inc(&dev->open_count))
> >  		need_setup = 1;
> > @@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
> >  			goto err_undo;
> >  		}
> >  	}
> > +
> > +	mutex_unlock(&drm_global_mutex);
> > +
> >  	return 0;
> >  
> >  err_undo:
> >  	atomic_dec(&dev->open_count);
> > +	mutex_unlock(&drm_global_mutex);
> >  	drm_minor_release(minor);
> >  	return retcode;
> >  }
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

On Wed, Jan 29, 2020 at 05:45:45PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> 
> On Wed, Jan 29, 2020 at 09:24:09AM +0100, Daniel Vetter wrote:
> > We want to only take the BKL on crap drivers, but to know whether
> The BKL was killed long time ago..
> In other words I was confused until I realized that
> - BKL
> - drm_global_mutex BKL
> - drm_global_mutex
> 
> Was all the same. At least my OCD color me confused as is.

The Real BKL was converted into drm_global_mutex for drm when the BKL was
killed. Which is kinda relevant, because the BKL locking horrors (at least
in drm) have been inherited by drm_global_mutex (and the conversion broke
a few things that had to be papered over). Hence the motivation to finally
clean up the locking and figure out what exactly is still protected by
this lock. If you're bored, all this is at least in modern git history
afaics.
-Daniel

> 
> > we have a crap driver we first need to look it up. Split this shuffle
> > out from the main BKL-disabling patch, for more clarity.
> > 
> > Since the minors are refcounted drm_minor_acquire is purely internal
> > and this does not have a driver visible effect.
> > 
> > v2: Push the locking even further into drm_open(), suggested by Chris.
> > This gives us more symmetry with drm_release(), and maybe a futuer
> > avenue where we make drm_globale_mutex locking (partially) opt-in like
> s/drm_globale_mutex/drm_global_mutex/
> 
> > with drm_release_noglobal().
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> Above is IMO fix-while-committing stuff.
> 
> 	Sam
> 
> > ---
> >  drivers/gpu/drm/drm_drv.c  | 14 +++++---------
> >  drivers/gpu/drm/drm_file.c |  6 ++++++
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 8deff75b484c..05bdf0b9d2b3 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
> >  
> >  	DRM_DEBUG("\n");
> >  
> > -	mutex_lock(&drm_global_mutex);
> >  	minor = drm_minor_acquire(iminor(inode));
> > -	if (IS_ERR(minor)) {
> > -		err = PTR_ERR(minor);
> > -		goto out_unlock;
> > -	}
> > +	if (IS_ERR(minor))
> > +		return PTR_ERR(minor);
> >  
> >  	new_fops = fops_get(minor->dev->driver->fops);
> >  	if (!new_fops) {
> >  		err = -ENODEV;
> > -		goto out_release;
> > +		goto out;
> >  	}
> >  
> >  	replace_fops(filp, new_fops);
> > @@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
> >  	else
> >  		err = 0;
> >  
> > -out_release:
> > +out:
> >  	drm_minor_release(minor);
> > -out_unlock:
> > -	mutex_unlock(&drm_global_mutex);
> > +
> >  	return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > index 1075b3a8b5b1..d36cb74ebe0c 100644
> > --- a/drivers/gpu/drm/drm_file.c
> > +++ b/drivers/gpu/drm/drm_file.c
> > @@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
> >  	if (IS_ERR(minor))
> >  		return PTR_ERR(minor);
> >  
> > +	mutex_unlock(&drm_global_mutex);
> > +
> >  	dev = minor->dev;
> >  	if (!atomic_fetch_inc(&dev->open_count))
> >  		need_setup = 1;
> > @@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
> >  			goto err_undo;
> >  		}
> >  	}
> > +
> > +	mutex_unlock(&drm_global_mutex);
> > +
> >  	return 0;
> >  
> >  err_undo:
> >  	atomic_dec(&dev->open_count);
> > +	mutex_unlock(&drm_global_mutex);
> >  	drm_minor_release(minor);
> >  	return retcode;
> >  }
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > 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

[PATCH] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

We want to only take the BKL on crap drivers, but to know whether
we have a crap driver we first need to look it up. Split this shuffle
out from the main BKL-disabling patch, for more clarity. Historical
aside: When the kernel-wide BKL was removed, it was replaced by
drm_global_mutex within the scope of the drm subsystem hence why these
two things are (almost) interchangeable as concepts here.

Since the minors are refcounted drm_minor_acquire is purely internal
and this does not have a driver visible effect.

v2: Push the locking even further into drm_open(), suggested by Chris.
This gives us more symmetry with drm_release(), and maybe a futuer
avenue where we make drm_global_mutex locking (partially) opt-in like
with drm_release_noglobal().

v3:
- Actually push this stuff correctly, don't unlock twice (Chris)
- Fix typo on commit message, plus explain why BKL = drm_global_mutex
  (Sam)

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c  | 14 +++++---------
 drivers/gpu/drm/drm_file.c |  6 ++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8deff75b484c..05bdf0b9d2b3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 
 	DRM_DEBUG("\n");
 
-	mutex_lock(&drm_global_mutex);
 	minor = drm_minor_acquire(iminor(inode));
-	if (IS_ERR(minor)) {
-		err = PTR_ERR(minor);
-		goto out_unlock;
-	}
+	if (IS_ERR(minor))
+		return PTR_ERR(minor);
 
 	new_fops = fops_get(minor->dev->driver->fops);
 	if (!new_fops) {
 		err = -ENODEV;
-		goto out_release;
+		goto out;
 	}
 
 	replace_fops(filp, new_fops);
@@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 	else
 		err = 0;
 
-out_release:
+out:
 	drm_minor_release(minor);
-out_unlock:
-	mutex_unlock(&drm_global_mutex);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 1075b3a8b5b1..80d556402ab4 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
+	mutex_lock(&drm_global_mutex);
+
 	dev = minor->dev;
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
@@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
 			goto err_undo;
 		}
 	}
+
+	mutex_unlock(&drm_global_mutex);
+
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
+	mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
-- 
2.24.1

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

[Intel-gfx] [PATCH] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

We want to only take the BKL on crap drivers, but to know whether
we have a crap driver we first need to look it up. Split this shuffle
out from the main BKL-disabling patch, for more clarity. Historical
aside: When the kernel-wide BKL was removed, it was replaced by
drm_global_mutex within the scope of the drm subsystem hence why these
two things are (almost) interchangeable as concepts here.

Since the minors are refcounted drm_minor_acquire is purely internal
and this does not have a driver visible effect.

v2: Push the locking even further into drm_open(), suggested by Chris.
This gives us more symmetry with drm_release(), and maybe a futuer
avenue where we make drm_global_mutex locking (partially) opt-in like
with drm_release_noglobal().

v3:
- Actually push this stuff correctly, don't unlock twice (Chris)
- Fix typo on commit message, plus explain why BKL = drm_global_mutex
  (Sam)

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c  | 14 +++++---------
 drivers/gpu/drm/drm_file.c |  6 ++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8deff75b484c..05bdf0b9d2b3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 
 	DRM_DEBUG("\n");
 
-	mutex_lock(&drm_global_mutex);
 	minor = drm_minor_acquire(iminor(inode));
-	if (IS_ERR(minor)) {
-		err = PTR_ERR(minor);
-		goto out_unlock;
-	}
+	if (IS_ERR(minor))
+		return PTR_ERR(minor);
 
 	new_fops = fops_get(minor->dev->driver->fops);
 	if (!new_fops) {
 		err = -ENODEV;
-		goto out_release;
+		goto out;
 	}
 
 	replace_fops(filp, new_fops);
@@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 	else
 		err = 0;
 
-out_release:
+out:
 	drm_minor_release(minor);
-out_unlock:
-	mutex_unlock(&drm_global_mutex);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 1075b3a8b5b1..80d556402ab4 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
+	mutex_lock(&drm_global_mutex);
+
 	dev = minor->dev;
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
@@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
 			goto err_undo;
 		}
 	}
+
+	mutex_unlock(&drm_global_mutex);
+
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
+	mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Daniel Vetter]

This catches the majority of drivers (unfortunately not if we take
users into account, because all the big drivers have at least a
lastclose hook).

With the prep patches out of the way all drm state is fully protected
and either prevents or can deal with the races from dropping the BKL
around open/close. The only thing left to audit are the various driver
hooks - by keeping the BKL around if any of them are set we have a
very simple cop-out!

Note that one of the biggest prep pieces to get here was making
dev->open_count atomic, which was done in

commit 7e13ad896484a0165a68197a2e64091ea28c9602
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jan 24 13:01:07 2020 +0000

    drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c      |  6 +++--
 drivers/gpu/drm/drm_file.c     | 46 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_internal.h |  1 +
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 05bdf0b9d2b3..9fcd6ab3c154 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	struct drm_driver *driver = dev->driver;
 	int ret;
 
-	mutex_lock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
 
 	if (dev->driver->load) {
 		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
@@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 out_unlock:
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	return ret;
 }
 EXPORT_SYMBOL(drm_dev_register);
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index d36cb74ebe0c..efd6fe0b6b4f 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -51,6 +51,37 @@
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
 
+bool drm_dev_needs_global_mutex(struct drm_device *dev)
+{
+	/*
+	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
+	 * bother. They also still need BKL locking for their ioctls, so better
+	 * safe than sorry.
+	 */
+	if (drm_core_check_feature(dev, DRIVER_LEGACY))
+		return true;
+
+	/*
+	 * The deprecated ->load callback must be called after the driver is
+	 * already registered. This means such drivers rely on the BKL to make
+	 * sure an open can't proceed until the driver is actually fully set up.
+	 * Similar hilarity holds for the unload callback.
+	 */
+	if (dev->driver->load || dev->driver->unload)
+		return true;
+
+	/*
+	 * Drivers with the lastclose callback assume that it's synchronized
+	 * against concurrent opens, which again needs the BKL. The proper fix
+	 * is to use the drm_client infrastructure with proper locking for each
+	 * client.
+	 */
+	if (dev->driver->lastclose)
+		return true;
+
+	return false;
+}
+
 /**
  * DOC: file operations
  *
@@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
-	mutex_unlock(&drm_global_mutex);
-
 	dev = minor->dev;
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
+
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
 
@@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
 		}
 	}
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
@@ -444,6 +478,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	struct drm_minor *minor = file_priv->minor;
 	struct drm_device *dev = minor->dev;
 
+	if (drm_dev_needs_global_mutex(dev))
 	mutex_lock(&drm_global_mutex);
 
 	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
@@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	if (atomic_dec_and_test(&dev->open_count))
 		drm_lastclose(dev);
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	drm_minor_release(minor);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 6937bf923f05..aeec2e68d772 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -41,6 +41,7 @@ struct drm_printer;
 
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
+bool drm_dev_needs_global_mutex(struct drm_device *dev);
 struct drm_file *drm_file_alloc(struct drm_minor *minor);
 void drm_file_free(struct drm_file *file);
 void drm_lastclose(struct drm_device *dev);
-- 
2.24.1

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

[Intel-gfx] [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Daniel Vetter]

This catches the majority of drivers (unfortunately not if we take
users into account, because all the big drivers have at least a
lastclose hook).

With the prep patches out of the way all drm state is fully protected
and either prevents or can deal with the races from dropping the BKL
around open/close. The only thing left to audit are the various driver
hooks - by keeping the BKL around if any of them are set we have a
very simple cop-out!

Note that one of the biggest prep pieces to get here was making
dev->open_count atomic, which was done in

commit 7e13ad896484a0165a68197a2e64091ea28c9602
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jan 24 13:01:07 2020 +0000

    drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c      |  6 +++--
 drivers/gpu/drm/drm_file.c     | 46 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_internal.h |  1 +
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 05bdf0b9d2b3..9fcd6ab3c154 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	struct drm_driver *driver = dev->driver;
 	int ret;
 
-	mutex_lock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
 
 	if (dev->driver->load) {
 		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
@@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 out_unlock:
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	return ret;
 }
 EXPORT_SYMBOL(drm_dev_register);
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index d36cb74ebe0c..efd6fe0b6b4f 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -51,6 +51,37 @@
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
 
+bool drm_dev_needs_global_mutex(struct drm_device *dev)
+{
+	/*
+	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
+	 * bother. They also still need BKL locking for their ioctls, so better
+	 * safe than sorry.
+	 */
+	if (drm_core_check_feature(dev, DRIVER_LEGACY))
+		return true;
+
+	/*
+	 * The deprecated ->load callback must be called after the driver is
+	 * already registered. This means such drivers rely on the BKL to make
+	 * sure an open can't proceed until the driver is actually fully set up.
+	 * Similar hilarity holds for the unload callback.
+	 */
+	if (dev->driver->load || dev->driver->unload)
+		return true;
+
+	/*
+	 * Drivers with the lastclose callback assume that it's synchronized
+	 * against concurrent opens, which again needs the BKL. The proper fix
+	 * is to use the drm_client infrastructure with proper locking for each
+	 * client.
+	 */
+	if (dev->driver->lastclose)
+		return true;
+
+	return false;
+}
+
 /**
  * DOC: file operations
  *
@@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
-	mutex_unlock(&drm_global_mutex);
-
 	dev = minor->dev;
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
+
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
 
@@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
 		}
 	}
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
@@ -444,6 +478,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	struct drm_minor *minor = file_priv->minor;
 	struct drm_device *dev = minor->dev;
 
+	if (drm_dev_needs_global_mutex(dev))
 	mutex_lock(&drm_global_mutex);
 
 	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
@@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	if (atomic_dec_and_test(&dev->open_count))
 		drm_lastclose(dev);
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	drm_minor_release(minor);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 6937bf923f05..aeec2e68d772 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -41,6 +41,7 @@ struct drm_printer;
 
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
+bool drm_dev_needs_global_mutex(struct drm_device *dev);
 struct drm_file *drm_file_alloc(struct drm_minor *minor);
 void drm_file_free(struct drm_file *file);
 void drm_lastclose(struct drm_device *dev);
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Sam Ravnborg]

Hi Daniel.

In the nit-pick department today - sorry.

Subject: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers
I did not understand this subject... - what is "Nerv"?

	Sam

On Wed, Jan 29, 2020 at 09:24:10AM +0100, Daniel Vetter wrote:
> This catches the majority of drivers (unfortunately not if we take
> users into account, because all the big drivers have at least a
> lastclose hook).
> 
> With the prep patches out of the way all drm state is fully protected
> and either prevents or can deal with the races from dropping the BKL
> around open/close. The only thing left to audit are the various driver
> hooks - by keeping the BKL around if any of them are set we have a
> very simple cop-out!
> 
> Note that one of the biggest prep pieces to get here was making
> dev->open_count atomic, which was done in
> 
> commit 7e13ad896484a0165a68197a2e64091ea28c9602
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Jan 24 13:01:07 2020 +0000
> 
>     drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c      |  6 +++--
>  drivers/gpu/drm/drm_file.c     | 46 ++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/drm_internal.h |  1 +
>  3 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 05bdf0b9d2b3..9fcd6ab3c154 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  	struct drm_driver *driver = dev->driver;
>  	int ret;
>  
> -	mutex_lock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_lock(&drm_global_mutex);
>  
>  	if (dev->driver->load) {
>  		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
> @@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
>  	drm_minor_unregister(dev, DRM_MINOR_RENDER);
>  out_unlock:
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_dev_register);
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index d36cb74ebe0c..efd6fe0b6b4f 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -51,6 +51,37 @@
>  /* from BKL pushdown */
>  DEFINE_MUTEX(drm_global_mutex);
>  
> +bool drm_dev_needs_global_mutex(struct drm_device *dev)
> +{
> +	/*
> +	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
> +	 * bother. They also still need BKL locking for their ioctls, so better
> +	 * safe than sorry.
> +	 */
> +	if (drm_core_check_feature(dev, DRIVER_LEGACY))
> +		return true;
> +
> +	/*
> +	 * The deprecated ->load callback must be called after the driver is
> +	 * already registered. This means such drivers rely on the BKL to make
> +	 * sure an open can't proceed until the driver is actually fully set up.
> +	 * Similar hilarity holds for the unload callback.
> +	 */
> +	if (dev->driver->load || dev->driver->unload)
> +		return true;
> +
> +	/*
> +	 * Drivers with the lastclose callback assume that it's synchronized
> +	 * against concurrent opens, which again needs the BKL. The proper fix
> +	 * is to use the drm_client infrastructure with proper locking for each
> +	 * client.
> +	 */
> +	if (dev->driver->lastclose)
> +		return true;
> +
> +	return false;
> +}
> +
>  /**
>   * DOC: file operations
>   *
> @@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
>  	if (IS_ERR(minor))
>  		return PTR_ERR(minor);
>  
> -	mutex_unlock(&drm_global_mutex);
> -
>  	dev = minor->dev;
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
> +
>  	if (!atomic_fetch_inc(&dev->open_count))
>  		need_setup = 1;
>  
> @@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
>  		}
>  	}
>  
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  
>  	return 0;
>  
>  err_undo:
>  	atomic_dec(&dev->open_count);
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  	drm_minor_release(minor);
>  	return retcode;
>  }
> @@ -444,6 +478,7 @@ int drm_release(struct inode *inode, struct file *filp)
>  	struct drm_minor *minor = file_priv->minor;
>  	struct drm_device *dev = minor->dev;
>  
> +	if (drm_dev_needs_global_mutex(dev))
>  	mutex_lock(&drm_global_mutex);
>  
>  	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
> @@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
>  	if (atomic_dec_and_test(&dev->open_count))
>  		drm_lastclose(dev);
>  
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  
>  	drm_minor_release(minor);
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 6937bf923f05..aeec2e68d772 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -41,6 +41,7 @@ struct drm_printer;
>  
>  /* drm_file.c */
>  extern struct mutex drm_global_mutex;
> +bool drm_dev_needs_global_mutex(struct drm_device *dev);
>  struct drm_file *drm_file_alloc(struct drm_minor *minor);
>  void drm_file_free(struct drm_file *file);
>  void drm_lastclose(struct drm_device *dev);
> -- 
> 2.24.1
> 
> _______________________________________________
> 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

Re: [Intel-gfx] [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Sam Ravnborg]

Hi Daniel.

In the nit-pick department today - sorry.

Subject: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers
I did not understand this subject... - what is "Nerv"?

	Sam

On Wed, Jan 29, 2020 at 09:24:10AM +0100, Daniel Vetter wrote:
> This catches the majority of drivers (unfortunately not if we take
> users into account, because all the big drivers have at least a
> lastclose hook).
> 
> With the prep patches out of the way all drm state is fully protected
> and either prevents or can deal with the races from dropping the BKL
> around open/close. The only thing left to audit are the various driver
> hooks - by keeping the BKL around if any of them are set we have a
> very simple cop-out!
> 
> Note that one of the biggest prep pieces to get here was making
> dev->open_count atomic, which was done in
> 
> commit 7e13ad896484a0165a68197a2e64091ea28c9602
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Jan 24 13:01:07 2020 +0000
> 
>     drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c      |  6 +++--
>  drivers/gpu/drm/drm_file.c     | 46 ++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/drm_internal.h |  1 +
>  3 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 05bdf0b9d2b3..9fcd6ab3c154 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  	struct drm_driver *driver = dev->driver;
>  	int ret;
>  
> -	mutex_lock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_lock(&drm_global_mutex);
>  
>  	if (dev->driver->load) {
>  		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
> @@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
>  	drm_minor_unregister(dev, DRM_MINOR_RENDER);
>  out_unlock:
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_dev_register);
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index d36cb74ebe0c..efd6fe0b6b4f 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -51,6 +51,37 @@
>  /* from BKL pushdown */
>  DEFINE_MUTEX(drm_global_mutex);
>  
> +bool drm_dev_needs_global_mutex(struct drm_device *dev)
> +{
> +	/*
> +	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
> +	 * bother. They also still need BKL locking for their ioctls, so better
> +	 * safe than sorry.
> +	 */
> +	if (drm_core_check_feature(dev, DRIVER_LEGACY))
> +		return true;
> +
> +	/*
> +	 * The deprecated ->load callback must be called after the driver is
> +	 * already registered. This means such drivers rely on the BKL to make
> +	 * sure an open can't proceed until the driver is actually fully set up.
> +	 * Similar hilarity holds for the unload callback.
> +	 */
> +	if (dev->driver->load || dev->driver->unload)
> +		return true;
> +
> +	/*
> +	 * Drivers with the lastclose callback assume that it's synchronized
> +	 * against concurrent opens, which again needs the BKL. The proper fix
> +	 * is to use the drm_client infrastructure with proper locking for each
> +	 * client.
> +	 */
> +	if (dev->driver->lastclose)
> +		return true;
> +
> +	return false;
> +}
> +
>  /**
>   * DOC: file operations
>   *
> @@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
>  	if (IS_ERR(minor))
>  		return PTR_ERR(minor);
>  
> -	mutex_unlock(&drm_global_mutex);
> -
>  	dev = minor->dev;
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
> +
>  	if (!atomic_fetch_inc(&dev->open_count))
>  		need_setup = 1;
>  
> @@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
>  		}
>  	}
>  
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  
>  	return 0;
>  
>  err_undo:
>  	atomic_dec(&dev->open_count);
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  	drm_minor_release(minor);
>  	return retcode;
>  }
> @@ -444,6 +478,7 @@ int drm_release(struct inode *inode, struct file *filp)
>  	struct drm_minor *minor = file_priv->minor;
>  	struct drm_device *dev = minor->dev;
>  
> +	if (drm_dev_needs_global_mutex(dev))
>  	mutex_lock(&drm_global_mutex);
>  
>  	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
> @@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
>  	if (atomic_dec_and_test(&dev->open_count))
>  		drm_lastclose(dev);
>  
> -	mutex_unlock(&drm_global_mutex);
> +	if (drm_dev_needs_global_mutex(dev))
> +		mutex_unlock(&drm_global_mutex);
>  
>  	drm_minor_release(minor);
>  
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 6937bf923f05..aeec2e68d772 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -41,6 +41,7 @@ struct drm_printer;
>  
>  /* drm_file.c */
>  extern struct mutex drm_global_mutex;
> +bool drm_dev_needs_global_mutex(struct drm_device *dev);
>  struct drm_file *drm_file_alloc(struct drm_minor *minor);
>  void drm_file_free(struct drm_file *file);
>  void drm_lastclose(struct drm_device *dev);
> -- 
> 2.24.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Daniel Vetter]

On Wed, Jan 29, 2020 at 05:47:33PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> 
> In the nit-pick department today - sorry.
> 
> Subject: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers
> I did not understand this subject... - what is "Nerv"?

It's a typo, supposed to be nerf:

https://www.urbandictionary.com/define.php?term=nerf

Cheers, Daniel
> 
> 	Sam
> 
> On Wed, Jan 29, 2020 at 09:24:10AM +0100, Daniel Vetter wrote:
> > This catches the majority of drivers (unfortunately not if we take
> > users into account, because all the big drivers have at least a
> > lastclose hook).
> > 
> > With the prep patches out of the way all drm state is fully protected
> > and either prevents or can deal with the races from dropping the BKL
> > around open/close. The only thing left to audit are the various driver
> > hooks - by keeping the BKL around if any of them are set we have a
> > very simple cop-out!
> > 
> > Note that one of the biggest prep pieces to get here was making
> > dev->open_count atomic, which was done in
> > 
> > commit 7e13ad896484a0165a68197a2e64091ea28c9602
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Fri Jan 24 13:01:07 2020 +0000
> > 
> >     drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_drv.c      |  6 +++--
> >  drivers/gpu/drm/drm_file.c     | 46 ++++++++++++++++++++++++++++++----
> >  drivers/gpu/drm/drm_internal.h |  1 +
> >  3 files changed, 46 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 05bdf0b9d2b3..9fcd6ab3c154 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >  	struct drm_driver *driver = dev->driver;
> >  	int ret;
> >  
> > -	mutex_lock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_lock(&drm_global_mutex);
> >  
> >  	if (dev->driver->load) {
> >  		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
> > @@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >  	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> >  	drm_minor_unregister(dev, DRM_MINOR_RENDER);
> >  out_unlock:
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(drm_dev_register);
> > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > index d36cb74ebe0c..efd6fe0b6b4f 100644
> > --- a/drivers/gpu/drm/drm_file.c
> > +++ b/drivers/gpu/drm/drm_file.c
> > @@ -51,6 +51,37 @@
> >  /* from BKL pushdown */
> >  DEFINE_MUTEX(drm_global_mutex);
> >  
> > +bool drm_dev_needs_global_mutex(struct drm_device *dev)
> > +{
> > +	/*
> > +	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
> > +	 * bother. They also still need BKL locking for their ioctls, so better
> > +	 * safe than sorry.
> > +	 */
> > +	if (drm_core_check_feature(dev, DRIVER_LEGACY))
> > +		return true;
> > +
> > +	/*
> > +	 * The deprecated ->load callback must be called after the driver is
> > +	 * already registered. This means such drivers rely on the BKL to make
> > +	 * sure an open can't proceed until the driver is actually fully set up.
> > +	 * Similar hilarity holds for the unload callback.
> > +	 */
> > +	if (dev->driver->load || dev->driver->unload)
> > +		return true;
> > +
> > +	/*
> > +	 * Drivers with the lastclose callback assume that it's synchronized
> > +	 * against concurrent opens, which again needs the BKL. The proper fix
> > +	 * is to use the drm_client infrastructure with proper locking for each
> > +	 * client.
> > +	 */
> > +	if (dev->driver->lastclose)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  /**
> >   * DOC: file operations
> >   *
> > @@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
> >  	if (IS_ERR(minor))
> >  		return PTR_ERR(minor);
> >  
> > -	mutex_unlock(&drm_global_mutex);
> > -
> >  	dev = minor->dev;
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> > +
> >  	if (!atomic_fetch_inc(&dev->open_count))
> >  		need_setup = 1;
> >  
> > @@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
> >  		}
> >  	}
> >  
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  
> >  	return 0;
> >  
> >  err_undo:
> >  	atomic_dec(&dev->open_count);
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  	drm_minor_release(minor);
> >  	return retcode;
> >  }
> > @@ -444,6 +478,7 @@ int drm_release(struct inode *inode, struct file *filp)
> >  	struct drm_minor *minor = file_priv->minor;
> >  	struct drm_device *dev = minor->dev;
> >  
> > +	if (drm_dev_needs_global_mutex(dev))
> >  	mutex_lock(&drm_global_mutex);
> >  
> >  	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
> > @@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
> >  	if (atomic_dec_and_test(&dev->open_count))
> >  		drm_lastclose(dev);
> >  
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  
> >  	drm_minor_release(minor);
> >  
> > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> > index 6937bf923f05..aeec2e68d772 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -41,6 +41,7 @@ struct drm_printer;
> >  
> >  /* drm_file.c */
> >  extern struct mutex drm_global_mutex;
> > +bool drm_dev_needs_global_mutex(struct drm_device *dev);
> >  struct drm_file *drm_file_alloc(struct drm_minor *minor);
> >  void drm_file_free(struct drm_file *file);
> >  void drm_lastclose(struct drm_device *dev);
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Daniel Vetter]

On Wed, Jan 29, 2020 at 05:47:33PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> 
> In the nit-pick department today - sorry.
> 
> Subject: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers
> I did not understand this subject... - what is "Nerv"?

It's a typo, supposed to be nerf:

https://www.urbandictionary.com/define.php?term=nerf

Cheers, Daniel
> 
> 	Sam
> 
> On Wed, Jan 29, 2020 at 09:24:10AM +0100, Daniel Vetter wrote:
> > This catches the majority of drivers (unfortunately not if we take
> > users into account, because all the big drivers have at least a
> > lastclose hook).
> > 
> > With the prep patches out of the way all drm state is fully protected
> > and either prevents or can deal with the races from dropping the BKL
> > around open/close. The only thing left to audit are the various driver
> > hooks - by keeping the BKL around if any of them are set we have a
> > very simple cop-out!
> > 
> > Note that one of the biggest prep pieces to get here was making
> > dev->open_count atomic, which was done in
> > 
> > commit 7e13ad896484a0165a68197a2e64091ea28c9602
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Fri Jan 24 13:01:07 2020 +0000
> > 
> >     drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_drv.c      |  6 +++--
> >  drivers/gpu/drm/drm_file.c     | 46 ++++++++++++++++++++++++++++++----
> >  drivers/gpu/drm/drm_internal.h |  1 +
> >  3 files changed, 46 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 05bdf0b9d2b3..9fcd6ab3c154 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >  	struct drm_driver *driver = dev->driver;
> >  	int ret;
> >  
> > -	mutex_lock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_lock(&drm_global_mutex);
> >  
> >  	if (dev->driver->load) {
> >  		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
> > @@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >  	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> >  	drm_minor_unregister(dev, DRM_MINOR_RENDER);
> >  out_unlock:
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL(drm_dev_register);
> > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > index d36cb74ebe0c..efd6fe0b6b4f 100644
> > --- a/drivers/gpu/drm/drm_file.c
> > +++ b/drivers/gpu/drm/drm_file.c
> > @@ -51,6 +51,37 @@
> >  /* from BKL pushdown */
> >  DEFINE_MUTEX(drm_global_mutex);
> >  
> > +bool drm_dev_needs_global_mutex(struct drm_device *dev)
> > +{
> > +	/*
> > +	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
> > +	 * bother. They also still need BKL locking for their ioctls, so better
> > +	 * safe than sorry.
> > +	 */
> > +	if (drm_core_check_feature(dev, DRIVER_LEGACY))
> > +		return true;
> > +
> > +	/*
> > +	 * The deprecated ->load callback must be called after the driver is
> > +	 * already registered. This means such drivers rely on the BKL to make
> > +	 * sure an open can't proceed until the driver is actually fully set up.
> > +	 * Similar hilarity holds for the unload callback.
> > +	 */
> > +	if (dev->driver->load || dev->driver->unload)
> > +		return true;
> > +
> > +	/*
> > +	 * Drivers with the lastclose callback assume that it's synchronized
> > +	 * against concurrent opens, which again needs the BKL. The proper fix
> > +	 * is to use the drm_client infrastructure with proper locking for each
> > +	 * client.
> > +	 */
> > +	if (dev->driver->lastclose)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  /**
> >   * DOC: file operations
> >   *
> > @@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
> >  	if (IS_ERR(minor))
> >  		return PTR_ERR(minor);
> >  
> > -	mutex_unlock(&drm_global_mutex);
> > -
> >  	dev = minor->dev;
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> > +
> >  	if (!atomic_fetch_inc(&dev->open_count))
> >  		need_setup = 1;
> >  
> > @@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
> >  		}
> >  	}
> >  
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  
> >  	return 0;
> >  
> >  err_undo:
> >  	atomic_dec(&dev->open_count);
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  	drm_minor_release(minor);
> >  	return retcode;
> >  }
> > @@ -444,6 +478,7 @@ int drm_release(struct inode *inode, struct file *filp)
> >  	struct drm_minor *minor = file_priv->minor;
> >  	struct drm_device *dev = minor->dev;
> >  
> > +	if (drm_dev_needs_global_mutex(dev))
> >  	mutex_lock(&drm_global_mutex);
> >  
> >  	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
> > @@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
> >  	if (atomic_dec_and_test(&dev->open_count))
> >  		drm_lastclose(dev);
> >  
> > -	mutex_unlock(&drm_global_mutex);
> > +	if (drm_dev_needs_global_mutex(dev))
> > +		mutex_unlock(&drm_global_mutex);
> >  
> >  	drm_minor_release(minor);
> >  
> > diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> > index 6937bf923f05..aeec2e68d772 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -41,6 +41,7 @@ struct drm_printer;
> >  
> >  /* drm_file.c */
> >  extern struct mutex drm_global_mutex;
> > +bool drm_dev_needs_global_mutex(struct drm_device *dev);
> >  struct drm_file *drm_file_alloc(struct drm_minor *minor);
> >  void drm_file_free(struct drm_file *file);
> >  void drm_lastclose(struct drm_device *dev);
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > 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

Re: [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Chris Wilson]

Quoting Daniel Vetter (2020-01-29 08:24:10)
> @@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
>         if (IS_ERR(minor))
>                 return PTR_ERR(minor);
>  
> -       mutex_unlock(&drm_global_mutex);
> -
>         dev = minor->dev;
> +       if (drm_dev_needs_global_mutex(dev))
> +               mutex_unlock(&drm_global_mutex);

Too soon, too soon.

> +
>         if (!atomic_fetch_inc(&dev->open_count))
>                 need_setup = 1;
>  
> @@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
>                 }
>         }
>  
> -       mutex_unlock(&drm_global_mutex);
> +       if (drm_dev_needs_global_mutex(dev))
> +               mutex_unlock(&drm_global_mutex);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 5/5] drm: Nerv drm_global_mutex BKL for good drivers

[Chris Wilson]

Quoting Daniel Vetter (2020-01-29 08:24:10)
> @@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
>         if (IS_ERR(minor))
>                 return PTR_ERR(minor);
>  
> -       mutex_unlock(&drm_global_mutex);
> -
>         dev = minor->dev;
> +       if (drm_dev_needs_global_mutex(dev))
> +               mutex_unlock(&drm_global_mutex);

Too soon, too soon.

> +
>         if (!atomic_fetch_inc(&dev->open_count))
>                 need_setup = 1;
>  
> @@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
>                 }
>         }
>  
> -       mutex_unlock(&drm_global_mutex);
> +       if (drm_dev_needs_global_mutex(dev))
> +               mutex_unlock(&drm_global_mutex);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm: Nerf drm_global_mutex BKL for good drivers

[Daniel Vetter]

This catches the majority of drivers (unfortunately not if we take
users into account, because all the big drivers have at least a
lastclose hook).

With the prep patches out of the way all drm state is fully protected
and either prevents or can deal with the races from dropping the BKL
around open/close. The only thing left to audit are the various driver
hooks - by keeping the BKL around if any of them are set we have a
very simple cop-out!

Note that one of the biggest prep pieces to get here was making
dev->open_count atomic, which was done in

commit 7e13ad896484a0165a68197a2e64091ea28c9602
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jan 24 13:01:07 2020 +0000

    drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count

v2:
- Rebase and fix locking in drm_open() (Chris)
- Indentation fix in drm_release
- Typo fix in the commit message (Sam)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c      |  6 +++--
 drivers/gpu/drm/drm_file.c     | 48 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_internal.h |  1 +
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 05bdf0b9d2b3..9fcd6ab3c154 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	struct drm_driver *driver = dev->driver;
 	int ret;
 
-	mutex_lock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
 
 	if (dev->driver->load) {
 		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
@@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 out_unlock:
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	return ret;
 }
 EXPORT_SYMBOL(drm_dev_register);
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 80d556402ab4..c4c704e01961 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -51,6 +51,37 @@
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
 
+bool drm_dev_needs_global_mutex(struct drm_device *dev)
+{
+	/*
+	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
+	 * bother. They also still need BKL locking for their ioctls, so better
+	 * safe than sorry.
+	 */
+	if (drm_core_check_feature(dev, DRIVER_LEGACY))
+		return true;
+
+	/*
+	 * The deprecated ->load callback must be called after the driver is
+	 * already registered. This means such drivers rely on the BKL to make
+	 * sure an open can't proceed until the driver is actually fully set up.
+	 * Similar hilarity holds for the unload callback.
+	 */
+	if (dev->driver->load || dev->driver->unload)
+		return true;
+
+	/*
+	 * Drivers with the lastclose callback assume that it's synchronized
+	 * against concurrent opens, which again needs the BKL. The proper fix
+	 * is to use the drm_client infrastructure with proper locking for each
+	 * client.
+	 */
+	if (dev->driver->lastclose)
+		return true;
+
+	return false;
+}
+
 /**
  * DOC: file operations
  *
@@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
-	mutex_lock(&drm_global_mutex);
-
 	dev = minor->dev;
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
+
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
 
@@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
 		}
 	}
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
@@ -444,7 +478,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	struct drm_minor *minor = file_priv->minor;
 	struct drm_device *dev = minor->dev;
 
-	mutex_lock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
 
 	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
 
@@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	if (atomic_dec_and_test(&dev->open_count))
 		drm_lastclose(dev);
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	drm_minor_release(minor);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 6937bf923f05..aeec2e68d772 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -41,6 +41,7 @@ struct drm_printer;
 
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
+bool drm_dev_needs_global_mutex(struct drm_device *dev);
 struct drm_file *drm_file_alloc(struct drm_minor *minor);
 void drm_file_free(struct drm_file *file);
 void drm_lastclose(struct drm_device *dev);
-- 
2.24.1

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

[Intel-gfx] [PATCH] drm: Nerf drm_global_mutex BKL for good drivers

[Daniel Vetter]

This catches the majority of drivers (unfortunately not if we take
users into account, because all the big drivers have at least a
lastclose hook).

With the prep patches out of the way all drm state is fully protected
and either prevents or can deal with the races from dropping the BKL
around open/close. The only thing left to audit are the various driver
hooks - by keeping the BKL around if any of them are set we have a
very simple cop-out!

Note that one of the biggest prep pieces to get here was making
dev->open_count atomic, which was done in

commit 7e13ad896484a0165a68197a2e64091ea28c9602
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jan 24 13:01:07 2020 +0000

    drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count

v2:
- Rebase and fix locking in drm_open() (Chris)
- Indentation fix in drm_release
- Typo fix in the commit message (Sam)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c      |  6 +++--
 drivers/gpu/drm/drm_file.c     | 48 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_internal.h |  1 +
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 05bdf0b9d2b3..9fcd6ab3c154 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -946,7 +946,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	struct drm_driver *driver = dev->driver;
 	int ret;
 
-	mutex_lock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
 
 	if (dev->driver->load) {
 		if (!drm_core_check_feature(dev, DRIVER_LEGACY))
@@ -992,7 +993,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 out_unlock:
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	return ret;
 }
 EXPORT_SYMBOL(drm_dev_register);
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 80d556402ab4..c4c704e01961 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -51,6 +51,37 @@
 /* from BKL pushdown */
 DEFINE_MUTEX(drm_global_mutex);
 
+bool drm_dev_needs_global_mutex(struct drm_device *dev)
+{
+	/*
+	 * Legacy drivers rely on all kinds of BKL locking semantics, don't
+	 * bother. They also still need BKL locking for their ioctls, so better
+	 * safe than sorry.
+	 */
+	if (drm_core_check_feature(dev, DRIVER_LEGACY))
+		return true;
+
+	/*
+	 * The deprecated ->load callback must be called after the driver is
+	 * already registered. This means such drivers rely on the BKL to make
+	 * sure an open can't proceed until the driver is actually fully set up.
+	 * Similar hilarity holds for the unload callback.
+	 */
+	if (dev->driver->load || dev->driver->unload)
+		return true;
+
+	/*
+	 * Drivers with the lastclose callback assume that it's synchronized
+	 * against concurrent opens, which again needs the BKL. The proper fix
+	 * is to use the drm_client infrastructure with proper locking for each
+	 * client.
+	 */
+	if (dev->driver->lastclose)
+		return true;
+
+	return false;
+}
+
 /**
  * DOC: file operations
  *
@@ -378,9 +409,10 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
-	mutex_lock(&drm_global_mutex);
-
 	dev = minor->dev;
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
+
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
 
@@ -398,13 +430,15 @@ int drm_open(struct inode *inode, struct file *filp)
 		}
 	}
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
@@ -444,7 +478,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	struct drm_minor *minor = file_priv->minor;
 	struct drm_device *dev = minor->dev;
 
-	mutex_lock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_lock(&drm_global_mutex);
 
 	DRM_DEBUG("open_count = %d\n", atomic_read(&dev->open_count));
 
@@ -453,7 +488,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	if (atomic_dec_and_test(&dev->open_count))
 		drm_lastclose(dev);
 
-	mutex_unlock(&drm_global_mutex);
+	if (drm_dev_needs_global_mutex(dev))
+		mutex_unlock(&drm_global_mutex);
 
 	drm_minor_release(minor);
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 6937bf923f05..aeec2e68d772 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -41,6 +41,7 @@ struct drm_printer;
 
 /* drm_file.c */
 extern struct mutex drm_global_mutex;
+bool drm_dev_needs_global_mutex(struct drm_device *dev);
 struct drm_file *drm_file_alloc(struct drm_minor *minor);
 void drm_file_free(struct drm_file *file);
 void drm_lastclose(struct drm_device *dev);
-- 
2.24.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for disable drm_global_mutex for most drivers

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers
URL   : https://patchwork.freedesktop.org/series/72711/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4c86ec07f811 drm: Complain if drivers still use the ->load callback
-:48: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
42ee1c03e770 drm/fbdev-helper: don't force restores
-:70: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 29 lines checked
ca5ca224d1e1 drm/client: Rename _force to _locked
-:97: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 60 lines checked
7c9238faf93b drm: Push drm_global_mutex locking in drm_open
-:87: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 54 lines checked
bffddad1dd65 drm: Nerv drm_global_mutex BKL for good drivers
-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 7e13ad896484 ("drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count")'
#19: 
commit 7e13ad896484a0165a68197a2e64091ea28c9602

-:130: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 8)
#130: FILE: drivers/gpu/drm/drm_file.c:481:
+	if (drm_dev_needs_global_mutex(dev))
 	mutex_lock(&drm_global_mutex);

-:155: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 1 errors, 2 warnings, 0 checks, 107 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for disable drm_global_mutex for most drivers

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers
URL   : https://patchwork.freedesktop.org/series/72711/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7838 -> Patchwork_16311
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16311 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16311, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16311:

### IGT changes ###

#### Possible regressions ####

  * igt@core_auth@basic-auth:
    - fi-kbl-r:           [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-r/igt@core_auth@basic-auth.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-r/igt@core_auth@basic-auth.html
    - fi-bwr-2160:        NOTRUN -> [DMESG-WARN][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-bwr-2160/igt@core_auth@basic-auth.html
    - fi-bdw-5557u:       [PASS][4] -> [DMESG-WARN][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-bdw-5557u/igt@core_auth@basic-auth.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-bdw-5557u/igt@core_auth@basic-auth.html
    - fi-skl-guc:         [PASS][6] -> [DMESG-WARN][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-skl-guc/igt@core_auth@basic-auth.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-skl-guc/igt@core_auth@basic-auth.html
    - fi-kbl-8809g:       [PASS][8] -> [DMESG-WARN][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-8809g/igt@core_auth@basic-auth.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-8809g/igt@core_auth@basic-auth.html
    - fi-kbl-guc:         [PASS][10] -> [DMESG-WARN][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-guc/igt@core_auth@basic-auth.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-guc/igt@core_auth@basic-auth.html
    - fi-icl-dsi:         [PASS][12] -> [DMESG-WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-dsi/igt@core_auth@basic-auth.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-icl-dsi/igt@core_auth@basic-auth.html
    - fi-kbl-7500u:       [PASS][14] -> [DMESG-WARN][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-7500u/igt@core_auth@basic-auth.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-7500u/igt@core_auth@basic-auth.html
    - fi-icl-u2:          [PASS][16] -> [DMESG-WARN][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-u2/igt@core_auth@basic-auth.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-icl-u2/igt@core_auth@basic-auth.html
    - fi-hsw-4770:        [PASS][18] -> [DMESG-WARN][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-hsw-4770/igt@core_auth@basic-auth.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-hsw-4770/igt@core_auth@basic-auth.html
    - fi-byt-j1900:       [PASS][20] -> [DMESG-WARN][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-byt-j1900/igt@core_auth@basic-auth.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-byt-j1900/igt@core_auth@basic-auth.html
    - fi-cfl-8700k:       [PASS][22] -> [DMESG-WARN][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cfl-8700k/igt@core_auth@basic-auth.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cfl-8700k/igt@core_auth@basic-auth.html
    - fi-whl-u:           [PASS][24] -> [DMESG-WARN][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-whl-u/igt@core_auth@basic-auth.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-whl-u/igt@core_auth@basic-auth.html
    - fi-pnv-d510:        [PASS][26] -> [DMESG-WARN][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-pnv-d510/igt@core_auth@basic-auth.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-pnv-d510/igt@core_auth@basic-auth.html
    - fi-snb-2520m:       [PASS][28] -> [DMESG-WARN][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-snb-2520m/igt@core_auth@basic-auth.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-snb-2520m/igt@core_auth@basic-auth.html
    - fi-cml-u2:          [PASS][30] -> [DMESG-WARN][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cml-u2/igt@core_auth@basic-auth.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cml-u2/igt@core_auth@basic-auth.html
    - fi-skl-6700k2:      [PASS][32] -> [DMESG-WARN][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-skl-6700k2/igt@core_auth@basic-auth.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-skl-6700k2/igt@core_auth@basic-auth.html
    - fi-bxt-dsi:         [PASS][34] -> [DMESG-WARN][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-bxt-dsi/igt@core_auth@basic-auth.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-bxt-dsi/igt@core_auth@basic-auth.html
    - fi-byt-n2820:       NOTRUN -> [DMESG-WARN][36]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-byt-n2820/igt@core_auth@basic-auth.html
    - fi-cfl-guc:         [PASS][37] -> [DMESG-WARN][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cfl-guc/igt@core_auth@basic-auth.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cfl-guc/igt@core_auth@basic-auth.html
    - fi-skl-6770hq:      [PASS][39] -> [DMESG-WARN][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-skl-6770hq/igt@core_auth@basic-auth.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-skl-6770hq/igt@core_auth@basic-auth.html
    - fi-icl-guc:         [PASS][41] -> [DMESG-WARN][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-guc/igt@core_auth@basic-auth.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-icl-guc/igt@core_auth@basic-auth.html
    - fi-elk-e7500:       [PASS][43] -> [DMESG-WARN][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-elk-e7500/igt@core_auth@basic-auth.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-elk-e7500/igt@core_auth@basic-auth.html
    - fi-ilk-650:         [PASS][45] -> [DMESG-WARN][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-ilk-650/igt@core_auth@basic-auth.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-ilk-650/igt@core_auth@basic-auth.html
    - fi-kbl-soraka:      [PASS][47] -> [DMESG-WARN][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-soraka/igt@core_auth@basic-auth.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-soraka/igt@core_auth@basic-auth.html
    - fi-cml-s:           [PASS][49] -> [DMESG-WARN][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cml-s/igt@core_auth@basic-auth.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cml-s/igt@core_auth@basic-auth.html
    - fi-hsw-peppy:       [PASS][51] -> [DMESG-WARN][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-hsw-peppy/igt@core_auth@basic-auth.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-hsw-peppy/igt@core_auth@basic-auth.html
    - fi-icl-u3:          [PASS][53] -> [DMESG-WARN][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-u3/igt@core_auth@basic-auth.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-icl-u3/igt@core_auth@basic-auth.html
    - fi-hsw-4770r:       [PASS][55] -> [DMESG-WARN][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-hsw-4770r/igt@core_auth@basic-auth.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-hsw-4770r/igt@core_auth@basic-auth.html
    - fi-kbl-x1275:       [PASS][57] -> [DMESG-WARN][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-x1275/igt@core_auth@basic-auth.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-x1275/igt@core_auth@basic-auth.html
    - fi-icl-y:           [PASS][59] -> [DMESG-WARN][60]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-icl-y/igt@core_auth@basic-auth.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-icl-y/igt@core_auth@basic-auth.html
    - fi-gdg-551:         [PASS][61] -> [DMESG-WARN][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-gdg-551/igt@core_auth@basic-auth.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-gdg-551/igt@core_auth@basic-auth.html
    - fi-glk-dsi:         [PASS][63] -> [DMESG-WARN][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-glk-dsi/igt@core_auth@basic-auth.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-glk-dsi/igt@core_auth@basic-auth.html
    - fi-cfl-8109u:       [PASS][65] -> [DMESG-WARN][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cfl-8109u/igt@core_auth@basic-auth.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cfl-8109u/igt@core_auth@basic-auth.html
    - fi-blb-e6850:       [PASS][67] -> [DMESG-WARN][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-blb-e6850/igt@core_auth@basic-auth.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-blb-e6850/igt@core_auth@basic-auth.html

  * igt@runner@aborted:
    - fi-ilk-650:         NOTRUN -> [FAIL][69]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-ilk-650/igt@runner@aborted.html
    - fi-pnv-d510:        NOTRUN -> [FAIL][70]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-pnv-d510/igt@runner@aborted.html
    - fi-kbl-x1275:       NOTRUN -> [FAIL][71]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-x1275/igt@runner@aborted.html
    - fi-cfl-8700k:       NOTRUN -> [FAIL][72]
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cfl-8700k/igt@runner@aborted.html
    - fi-hsw-4770r:       NOTRUN -> [FAIL][73]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-hsw-4770r/igt@runner@aborted.html
    - fi-cfl-8109u:       NOTRUN -> [FAIL][74]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cfl-8109u/igt@runner@aborted.html
    - fi-hsw-peppy:       NOTRUN -> [FAIL][75]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-hsw-peppy/igt@runner@aborted.html
    - fi-gdg-551:         NOTRUN -> [FAIL][76]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-gdg-551/igt@runner@aborted.html
    - fi-kbl-8809g:       NOTRUN -> [FAIL][77]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-8809g/igt@runner@aborted.html
    - fi-snb-2520m:       NOTRUN -> [FAIL][78]
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-snb-2520m/igt@runner@aborted.html
    - fi-kbl-r:           NOTRUN -> [FAIL][79]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-r/igt@runner@aborted.html
    - fi-bdw-5557u:       NOTRUN -> [FAIL][80]
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-bdw-5557u/igt@runner@aborted.html
    - fi-byt-n2820:       NOTRUN -> [FAIL][81]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-byt-n2820/igt@runner@aborted.html
    - fi-hsw-4770:        NOTRUN -> [FAIL][82]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-hsw-4770/igt@runner@aborted.html
    - fi-kbl-7500u:       NOTRUN -> [FAIL][83]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-7500u/igt@runner@aborted.html
    - fi-kbl-guc:         NOTRUN -> [FAIL][84]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-guc/igt@runner@aborted.html
    - fi-whl-u:           NOTRUN -> [FAIL][85]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-whl-u/igt@runner@aborted.html
    - fi-bxt-dsi:         NOTRUN -> [FAIL][86]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-bxt-dsi/igt@runner@aborted.html
    - fi-elk-e7500:       NOTRUN -> [FAIL][87]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-elk-e7500/igt@runner@aborted.html
    - fi-cfl-guc:         NOTRUN -> [FAIL][88]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cfl-guc/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][89]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-blb-e6850/igt@runner@aborted.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-kbl-soraka:      [FAIL][90] ([fdo#109383] / [fdo#111012]) -> [FAIL][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-kbl-soraka/igt@runner@aborted.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-kbl-soraka/igt@runner@aborted.html
    - fi-byt-j1900:       [FAIL][92] ([i915#816]) -> [FAIL][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-byt-j1900/igt@runner@aborted.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-byt-j1900/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@core_auth@basic-auth:
    - {fi-tgl-u}:         [PASS][94] -> [DMESG-WARN][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-tgl-u/igt@core_auth@basic-auth.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-tgl-u/igt@core_auth@basic-auth.html
    - {fi-ehl-1}:         [PASS][96] -> [DMESG-WARN][97]
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-ehl-1/igt@core_auth@basic-auth.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-ehl-1/igt@core_auth@basic-auth.html

  * igt@runner@aborted:
    - {fi-ehl-1}:         NOTRUN -> [FAIL][98]
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-ehl-1/igt@runner@aborted.html
    - {fi-tgl-u}:         NOTRUN -> [FAIL][99]
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-tgl-u/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_16311 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_auth@basic-auth:
    - fi-tgl-y:           [PASS][100] -> [DMESG-WARN][101] ([CI#94])
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-tgl-y/igt@core_auth@basic-auth.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-tgl-y/igt@core_auth@basic-auth.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-cml-s:           [FAIL][102] ([fdo#111012] / [fdo#111764] / [i915#577]) -> [FAIL][103] ([i915#577])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7838/fi-cml-s/igt@runner@aborted.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/fi-cml-s/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#109383]: https://bugs.freedesktop.org/show_bug.cgi?id=109383
  [fdo#111012]: https://bugs.freedesktop.org/show_bug.cgi?id=111012
  [fdo#111764]: https://bugs.freedesktop.org/show_bug.cgi?id=111764
  [i915#577]: https://gitlab.freedesktop.org/drm/intel/issues/577
  [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816


Participating hosts (50 -> 42)
------------------------------

  Additional (2): fi-byt-n2820 fi-bwr-2160 
  Missing    (10): fi-ilk-m540 fi-bdw-samus fi-bsw-n3050 fi-hsw-4200u fi-ivb-3770 fi-skl-lmem fi-kbl-7560u fi-byt-clapper fi-skl-6600u fi-snb-2600 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7838 -> Patchwork_16311

  CI-20190529: 20190529
  CI_DRM_7838: d3d96beea538c8de906a1c4d7e6793a47d17a471 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5404: 4147bab8e3dcaf11bd657b5fb4c109708e94e60c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16311: bffddad1dd65b0f70ac76ec748f1c61a6d987744 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bffddad1dd65 drm: Nerv drm_global_mutex BKL for good drivers
7c9238faf93b drm: Push drm_global_mutex locking in drm_open
ca5ca224d1e1 drm/client: Rename _force to _locked
42ee1c03e770 drm/fbdev-helper: don't force restores
4c86ec07f811 drm: Complain if drivers still use the ->load callback

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16311/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for disable drm_global_mutex for most drivers (rev3)

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers (rev3)
URL   : https://patchwork.freedesktop.org/series/72711/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5b5c3976292b drm: Complain if drivers still use the ->load callback
-:48: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
769f93d67513 drm/fbdev-helper: don't force restores
-:70: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 29 lines checked
819b18c6d96b drm/client: Rename _force to _locked
-:97: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 60 lines checked
e55cc9d8da91 drm: Push drm_global_mutex locking in drm_open
-:95: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 54 lines checked
ad79bc5f5388 drm: Nerf drm_global_mutex BKL for good drivers
-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 7e13ad896484 ("drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count")'
#19: 
commit 7e13ad896484a0165a68197a2e64091ea28c9602

-:162: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 1 errors, 1 warnings, 0 checks, 109 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for disable drm_global_mutex for most drivers (rev3)

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers (rev3)
URL   : https://patchwork.freedesktop.org/series/72711/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7847 -> Patchwork_16349
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16349 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16349, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16349:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_execlists:
    - fi-cml-u2:          [PASS][1] -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-cml-u2/igt@i915_selftest@live_execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-cml-u2/igt@i915_selftest@live_execlists.html

  
Known issues
------------

  Here are the changes found in Patchwork_16349 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [PASS][3] -> [DMESG-FAIL][4] ([i915#553] / [i915#725])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-n2820:       [TIMEOUT][5] ([fdo#112271] / [i915#1084] / [i915#816]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-byt-n2820/igt@gem_close_race@basic-threads.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-byt-n2820/igt@gem_close_race@basic-threads.html

  * igt@i915_selftest@live_execlists:
    - fi-icl-y:           [DMESG-FAIL][7] ([fdo#108569]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-icl-y/igt@i915_selftest@live_execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-icl-y/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-guc:         [INCOMPLETE][9] ([fdo#106070] / [i915#424]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-cfl-guc/igt@i915_selftest@live_gem_contexts.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-cfl-guc/igt@i915_selftest@live_gem_contexts.html

  
#### Warnings ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770:        [DMESG-FAIL][11] ([i915#725]) -> [DMESG-FAIL][12] ([i915#553] / [i915#725])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-hsw-4770/igt@i915_selftest@live_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-hsw-4770/igt@i915_selftest@live_blt.html

  * igt@runner@aborted:
    - fi-byt-n2820:       [FAIL][13] ([i915#816]) -> [FAIL][14] ([i915#999])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7847/fi-byt-n2820/igt@runner@aborted.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/fi-byt-n2820/igt@runner@aborted.html

  
  [fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#1084]: https://gitlab.freedesktop.org/drm/intel/issues/1084
  [i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816
  [i915#999]: https://gitlab.freedesktop.org/drm/intel/issues/999


Participating hosts (47 -> 40)
------------------------------

  Additional (4): fi-bdw-gvtdvm fi-gdg-551 fi-elk-e7500 fi-kbl-7500u 
  Missing    (11): fi-icl-1065g7 fi-bdw-samus fi-bsw-n3050 fi-hsw-4200u fi-bsw-cyan fi-bwr-2160 fi-skl-lmem fi-blb-e6850 fi-tgl-y fi-skl-6600u fi-kbl-r 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7847 -> Patchwork_16349

  CI-20190529: 20190529
  CI_DRM_7847: 2515f8cc5d56f8791232dc6b077a370658d4cecf @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5407: a9d69f51dadbcbc53527671f87572d05c3370cba @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16349: ad79bc5f5388f9d9888c7bd9b86c5eed776c6b00 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ad79bc5f5388 drm: Nerf drm_global_mutex BKL for good drivers
e55cc9d8da91 drm: Push drm_global_mutex locking in drm_open
819b18c6d96b drm/client: Rename _force to _locked
769f93d67513 drm/fbdev-helper: don't force restores
5b5c3976292b drm: Complain if drivers still use the ->load callback

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16349/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for disable drm_global_mutex for most drivers (rev4)

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers (rev4)
URL   : https://patchwork.freedesktop.org/series/72711/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ec6b1085531d drm: Complain if drivers still use the ->load callback
-:48: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
58509b1c6e52 drm/fbdev-helper: don't force restores
-:70: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 29 lines checked
edabfe0698c0 drm/client: Rename _force to _locked
-:97: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 60 lines checked
850beb19ef64 drm: Push drm_global_mutex locking in drm_open
-:95: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 0 errors, 1 warnings, 0 checks, 54 lines checked
0aa851bfac6d drm: Nerf drm_global_mutex BKL for good drivers
-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 7e13ad896484 ("drm: Avoid drm_global_mutex for simple inc/dec of dev->open_count")'
#19: 
commit 7e13ad896484a0165a68197a2e64091ea28c9602

-:162: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'

total: 1 errors, 1 warnings, 0 checks, 109 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for disable drm_global_mutex for most drivers (rev4)

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers (rev4)
URL   : https://patchwork.freedesktop.org/series/72711/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7853 -> Patchwork_16357
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/index.html

Known issues
------------

  Here are the changes found in Patchwork_16357 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [PASS][1] -> [DMESG-FAIL][2] ([i915#725])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-j1900:       [TIMEOUT][3] ([fdo#112271] / [i915#1084] / [i915#816]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/fi-byt-j1900/igt@gem_close_race@basic-threads.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/fi-byt-j1900/igt@gem_close_race@basic-threads.html
    - fi-byt-n2820:       [TIMEOUT][5] ([fdo#112271] / [i915#1084] / [i915#816]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/fi-byt-n2820/igt@gem_close_race@basic-threads.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/fi-byt-n2820/igt@gem_close_race@basic-threads.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-cml-u2:          [DMESG-WARN][7] ([IGT#4]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/fi-cml-u2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#111096] / [i915#323]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-kbl-guc:         [FAIL][11] ([i915#579]) -> [SKIP][12] ([fdo#109271])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html

  
  [IGT#4]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/4
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#1084]: https://gitlab.freedesktop.org/drm/intel/issues/1084
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816


Participating hosts (47 -> 45)
------------------------------

  Additional (4): fi-skl-6700k2 fi-cfl-8109u fi-skl-6600u fi-snb-2600 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7853 -> Patchwork_16357

  CI-20190529: 20190529
  CI_DRM_7853: 1df04205c16923e525efe9c26d6e98612d38c9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5409: 93aefe6baa3fabf8c0cabe83e185f7b8f8d8753d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16357: 0aa851bfac6dd1e5818bab37e3a722e736d276ca @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0aa851bfac6d drm: Nerf drm_global_mutex BKL for good drivers
850beb19ef64 drm: Push drm_global_mutex locking in drm_open
edabfe0698c0 drm/client: Rename _force to _locked
58509b1c6e52 drm/fbdev-helper: don't force restores
ec6b1085531d drm: Complain if drivers still use the ->load callback

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for disable drm_global_mutex for most drivers (rev4)

[Patchwork]

== Series Details ==

Series: disable drm_global_mutex for most drivers (rev4)
URL   : https://patchwork.freedesktop.org/series/72711/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7853_full -> Patchwork_16357_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_16357_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_16357_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_16357_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_fbcon_fbt@psr:
    - shard-tglb:         [PASS][1] -> [FAIL][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-tglb5/igt@kms_fbcon_fbt@psr.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-tglb6/igt@kms_fbcon_fbt@psr.html
    - shard-skl:          [PASS][3] -> [FAIL][4] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl4/igt@kms_fbcon_fbt@psr.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl10/igt@kms_fbcon_fbt@psr.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-iclb:         [PASS][5] -> [FAIL][6] +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb7/igt@kms_fbcon_fbt@psr-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb5/igt@kms_fbcon_fbt@psr-suspend.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@gem_ctx_persistence@legacy-engines-mixed-process@blt}:
    - shard-skl:          [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl7/igt@gem_ctx_persistence@legacy-engines-mixed-process@blt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl5/igt@gem_ctx_persistence@legacy-engines-mixed-process@blt.html

  
Known issues
------------

  Here are the changes found in Patchwork_16357_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@hang:
    - shard-iclb:         [PASS][9] -> [TIMEOUT][10] ([fdo#112271])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb1/igt@gem_exec_balancer@hang.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb2/igt@gem_exec_balancer@hang.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112146]) +7 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb7/igt@gem_exec_schedule@in-order-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb4/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@preempt-queue-bsd2:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109276]) +10 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd2.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb6/igt@gem_exec_schedule@preempt-queue-bsd2.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([i915#644])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-glk4/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-glk9/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_pwrite@big-cpu-random:
    - shard-apl:          [PASS][17] -> [INCOMPLETE][18] ([CI#80] / [fdo#103927])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-apl1/igt@gem_pwrite@big-cpu-random.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-apl6/igt@gem_pwrite@big-cpu-random.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-apl2/igt@gem_workarounds@suspend-resume-context.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-apl4/igt@gem_workarounds@suspend-resume-context.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([i915#180]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-kbl3/igt@gem_workarounds@suspend-resume-fd.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-kbl1/igt@gem_workarounds@suspend-resume-fd.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([IGT#5])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([i915#79])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@plain-flip-ts-check-interruptible:
    - shard-skl:          [PASS][27] -> [FAIL][28] ([i915#34])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl7/igt@kms_flip@plain-flip-ts-check-interruptible.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl6/igt@kms_flip@plain-flip-ts-check-interruptible.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][29] -> [FAIL][30] ([fdo#108145] / [i915#265])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +3 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb7/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-skl:          [PASS][33] -> [FAIL][34] ([i915#31])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl1/igt@kms_setmode@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl9/igt@kms_setmode@basic.html

  * igt@perf_pmu@busy-no-semaphores-vcs1:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#112080]) +10 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb2/igt@perf_pmu@busy-no-semaphores-vcs1.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb3/igt@perf_pmu@busy-no-semaphores-vcs1.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][37] ([i915#180]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-apl6/igt@gem_ctx_isolation@rcs0-s3.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-apl6/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][39] ([fdo#110841]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb5/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [SKIP][41] ([fdo#112080]) -> [PASS][42] +10 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb6/igt@gem_exec_parallel@vcs1-fds.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb4/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@pi-distinct-iova-bsd:
    - shard-iclb:         [SKIP][43] ([i915#677]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb4/igt@gem_exec_schedule@pi-distinct-iova-bsd.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb8/igt@gem_exec_schedule@pi-distinct-iova-bsd.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [SKIP][45] ([fdo#112146]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb4/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb8/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_partial_pwrite_pread@writes-after-reads:
    - shard-hsw:          [FAIL][47] ([i915#694]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-hsw1/igt@gem_partial_pwrite_pread@writes-after-reads.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-hsw1/igt@gem_partial_pwrite_pread@writes-after-reads.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][49] ([i915#69]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl10/igt@gem_softpin@noreloc-s3.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl6/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [FAIL][51] ([i915#454]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb6/igt@i915_pm_dc@dc6-psr.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb2/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rps@reset:
    - shard-tglb:         [FAIL][53] ([i915#413]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-tglb1/igt@i915_pm_rps@reset.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-tglb5/igt@i915_pm_rps@reset.html

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-xtiled:
    - shard-glk:          [FAIL][55] ([i915#52] / [i915#54]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-glk4/igt@kms_draw_crc@draw-method-xrgb8888-blt-xtiled.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-glk1/igt@kms_draw_crc@draw-method-xrgb8888-blt-xtiled.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +8 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu:
    - shard-tglb:         [SKIP][59] ([i915#668]) -> [PASS][60] +9 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-cpu.html

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-skl:          [DMESG-WARN][61] ([i915#109]) -> [PASS][62] +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-skl4/igt@kms_plane@pixel-format-pipe-a-planes.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-skl4/igt@kms_plane@pixel-format-pipe-a-planes.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][63] ([fdo#109642] / [fdo#111068]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb1/igt@kms_psr2_su@frontbuffer.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [SKIP][65] ([fdo#109441]) -> [PASS][66] +2 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb1/igt@kms_psr@psr2_cursor_blt.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [SKIP][67] ([fdo#109276]) -> [PASS][68] +19 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb5/igt@prime_vgem@fence-wait-bsd2.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv-switch:
    - shard-iclb:         [FAIL][69] ([IGT#28]) -> [SKIP][70] ([fdo#112080])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv-switch.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-iclb3/igt@gem_ctx_isolation@vcs1-nonpriv-switch.html

  * igt@gem_eio@in-flight-contexts-immediate:
    - shard-apl:          [TIMEOUT][71] ([fdo#112271]) -> [INCOMPLETE][72] ([CI#80] / [fdo#103927] / [i915#1098])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-apl8/igt@gem_eio@in-flight-contexts-immediate.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-apl2/igt@gem_eio@in-flight-contexts-immediate.html

  * igt@gem_tiled_blits@normal:
    - shard-hsw:          [FAIL][73] ([i915#818]) -> [FAIL][74] ([i915#694])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-hsw5/igt@gem_tiled_blits@normal.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-hsw5/igt@gem_tiled_blits@normal.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [FAIL][75] ([i915#454]) -> [SKIP][76] ([i915#468])
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-tglb6/igt@i915_pm_dc@dc6-dpms.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [INCOMPLETE][77] ([fdo#103665]) -> [DMESG-WARN][78] ([i915#180])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7853/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [CI#80]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/80
  [IGT#28]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/28
  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112271]: https://bugs.freedesktop.org/show_bug.cgi?id=112271
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#1098]: https://gitlab.freedesktop.org/drm/intel/issues/1098
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#413]: https://gitlab.freedesktop.org/drm/intel/issues/413
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#818]: https://gitlab.freedesktop.org/drm/intel/issues/818


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7853 -> Patchwork_16357

  CI-20190529: 20190529
  CI_DRM_7853: 1df04205c16923e525efe9c26d6e98612d38c9b3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5409: 93aefe6baa3fabf8c0cabe83e185f7b8f8d8753d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16357: 0aa851bfac6dd1e5818bab37e3a722e736d276ca @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16357/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 0/5] disable drm_global_mutex for most drivers, take 2

[Daniel Vetter]

CI didn't like my test-with tag :-/

Test-with: 20200128112549.172135-1-daniel.vetter@ffwll.ch

Daniel Vetter (5):
  drm: Complain if drivers still use the ->load callback
  drm/fbdev-helper: don't force restores
  drm/client: Rename _force to _locked
  drm: Push drm_global_mutex locking in drm_open
  drm: Nerf drm_global_mutex BKL for good drivers

 drivers/gpu/drm/drm_client_modeset.c | 12 +++++---
 drivers/gpu/drm/drm_drv.c            | 26 +++++++++-------
 drivers/gpu/drm/drm_fb_helper.c      | 16 ++--------
 drivers/gpu/drm/drm_file.c           | 46 ++++++++++++++++++++++++++--
 drivers/gpu/drm/drm_internal.h       |  1 +
 include/drm/drm_client.h             |  7 ++++-
 include/drm/drm_drv.h                |  3 ++
 7 files changed, 79 insertions(+), 32 deletions(-)

-- 
2.24.1

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

[PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Daniel Vetter]

We want to only take the BKL on crap drivers, but to know whether
we have a crap driver we first need to look it up. Split this shuffle
out from the main BKL-disabling patch, for more clarity. Historical
aside: When the kernel-wide BKL was removed, it was replaced by
drm_global_mutex within the scope of the drm subsystem hence why these
two things are (almost) interchangeable as concepts here.

Since the minors are refcounted drm_minor_acquire is purely internal
and this does not have a driver visible effect.

v2: Push the locking even further into drm_open(), suggested by Chris.
This gives us more symmetry with drm_release(), and maybe a futuer
avenue where we make drm_global_mutex locking (partially) opt-in like
with drm_release_noglobal().

v3:
- Actually push this stuff correctly, don't unlock twice (Chris)
- Fix typo on commit message, plus explain why BKL = drm_global_mutex
  (Sam)

Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c  | 14 +++++---------
 drivers/gpu/drm/drm_file.c |  6 ++++++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8deff75b484c..05bdf0b9d2b3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1085,17 +1085,14 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 
 	DRM_DEBUG("\n");
 
-	mutex_lock(&drm_global_mutex);
 	minor = drm_minor_acquire(iminor(inode));
-	if (IS_ERR(minor)) {
-		err = PTR_ERR(minor);
-		goto out_unlock;
-	}
+	if (IS_ERR(minor))
+		return PTR_ERR(minor);
 
 	new_fops = fops_get(minor->dev->driver->fops);
 	if (!new_fops) {
 		err = -ENODEV;
-		goto out_release;
+		goto out;
 	}
 
 	replace_fops(filp, new_fops);
@@ -1104,10 +1101,9 @@ static int drm_stub_open(struct inode *inode, struct file *filp)
 	else
 		err = 0;
 
-out_release:
+out:
 	drm_minor_release(minor);
-out_unlock:
-	mutex_unlock(&drm_global_mutex);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 1075b3a8b5b1..80d556402ab4 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -378,6 +378,8 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (IS_ERR(minor))
 		return PTR_ERR(minor);
 
+	mutex_lock(&drm_global_mutex);
+
 	dev = minor->dev;
 	if (!atomic_fetch_inc(&dev->open_count))
 		need_setup = 1;
@@ -395,10 +397,14 @@ int drm_open(struct inode *inode, struct file *filp)
 			goto err_undo;
 		}
 	}
+
+	mutex_unlock(&drm_global_mutex);
+
 	return 0;
 
 err_undo:
 	atomic_dec(&dev->open_count);
+	mutex_unlock(&drm_global_mutex);
 	drm_minor_release(minor);
 	return retcode;
 }
-- 
2.24.1

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

Re: [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open

[Chris Wilson]

Quoting Daniel Vetter (2020-02-04 15:01:45)
> We want to only take the BKL on crap drivers, but to know whether
> we have a crap driver we first need to look it up. Split this shuffle
> out from the main BKL-disabling patch, for more clarity. Historical
> aside: When the kernel-wide BKL was removed, it was replaced by
> drm_global_mutex within the scope of the drm subsystem hence why these
> two things are (almost) interchangeable as concepts here.
> 
> Since the minors are refcounted drm_minor_acquire is purely internal
> and this does not have a driver visible effect.
> 
> v2: Push the locking even further into drm_open(), suggested by Chris.
> This gives us more symmetry with drm_release(), and maybe a futuer
> avenue where we make drm_global_mutex locking (partially) opt-in like
> with drm_release_noglobal().
> 
> v3:
> - Actually push this stuff correctly, don't unlock twice (Chris)
> - Fix typo on commit message, plus explain why BKL = drm_global_mutex
>   (Sam)
> 
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel