All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Sam Ravnborg <sam@ravnborg.org>
Subject: [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open
Date: Tue,  4 Feb 2020 16:01:45 +0100	[thread overview]
Message-ID: <20200204150146.2006481-5-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <20200204150146.2006481-1-daniel.vetter@ffwll.ch>

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

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Sam Ravnborg <sam@ravnborg.org>
Subject: [Intel-gfx] [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open
Date: Tue,  4 Feb 2020 16:01:45 +0100	[thread overview]
Message-ID: <20200204150146.2006481-5-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <20200204150146.2006481-1-daniel.vetter@ffwll.ch>

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

  parent reply	other threads:[~2020-02-04 15:02 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-04 15:01 [PATCH 0/5] disable drm_global_mutex for most drivers, take 2 Daniel Vetter
2020-02-04 15:01 ` [Intel-gfx] " Daniel Vetter
2020-02-04 15:01 ` [PATCH 1/5] drm: Complain if drivers still use the ->load callback Daniel Vetter
2020-02-04 15:01   ` [Intel-gfx] " Daniel Vetter
2020-02-04 15:01 ` [PATCH 2/5] drm/fbdev-helper: don't force restores Daniel Vetter
2020-02-04 15:01   ` [Intel-gfx] " Daniel Vetter
2020-02-04 15:01 ` [PATCH 3/5] drm/client: Rename _force to _locked Daniel Vetter
2020-02-04 15:01   ` [Intel-gfx] " Daniel Vetter
2020-02-04 15:01 ` Daniel Vetter [this message]
2020-02-04 15:01   ` [Intel-gfx] [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open Daniel Vetter
2020-02-10  8:09   ` Chris Wilson
2020-02-10  8:09     ` [Intel-gfx] " Chris Wilson
2020-02-04 15:01 ` [PATCH 5/5] drm: Nerf drm_global_mutex BKL for good drivers Daniel Vetter
2020-02-04 15:01   ` [Intel-gfx] " Daniel Vetter
2020-02-10  8:09   ` Chris Wilson
2020-02-10  8:09     ` [Intel-gfx] " Chris Wilson
2020-02-05  2:58 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for disable drm_global_mutex for most drivers (rev5) Patchwork
2020-02-06 19:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for disable drm_global_mutex for most drivers (rev6) Patchwork
2020-02-06 20:03 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for disable drm_global_mutex for most drivers (rev7) Patchwork
2020-02-07  8:22 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-02-07 13:37 ` [PATCH 0/5] disable drm_global_mutex for most drivers, take 2 Thomas Zimmermann
2020-02-07 13:37   ` [Intel-gfx] " Thomas Zimmermann
2020-02-09 17:13 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for disable drm_global_mutex for most drivers (rev7) Patchwork
2020-02-10  9:47 ` [PATCH 0/5] disable drm_global_mutex for most drivers, take 2 Thomas Zimmermann
2020-02-10  9:47   ` [Intel-gfx] " Thomas Zimmermann
2020-02-11 11:20   ` Daniel Vetter
2020-02-11 11:20     ` [Intel-gfx] " Daniel Vetter
2020-02-10 10:43 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for disable drm_global_mutex for most drivers (rev7) Patchwork
2020-02-10 12:20 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2020-01-29  8:24 [PATCH 0/5] disable drm_global_mutex for most drivers Daniel Vetter
2020-01-29  8:24 ` [PATCH 4/5] drm: Push drm_global_mutex locking in drm_open Daniel Vetter
2020-01-29 16:45   ` Sam Ravnborg
2020-01-29 17:05     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200204150146.2006481-5-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=sam@ravnborg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.