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
next prev 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: linkBe 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.