* INFO: possible circular locking dependency detected [v2.6.37-rc1-152-g6c88078]
[not found] <AANLkTikZ2cGGVnVai108VSQrL9yHSa5myKMEDZOgkDCe@mail.gmail.com>
@ 2010-11-08 0:53 ` Uwe Helm
2010-11-08 1:34 ` [PATCH] drm/i915: Avoid might_fault during pwrite whilst holding our mutex Chris Wilson
0 siblings, 1 reply; 3+ messages in thread
From: Uwe Helm @ 2010-11-08 0:53 UTC (permalink / raw)
To: linux-kernel
Hi,
I've been seeing these lately, Xorg version is 1.9.0 with latest git kernel.
=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.37-rc1-uwe1+ #4
-------------------------------------------------------
Xorg/1401 is trying to acquire lock:
(&mm->mmap_sem){++++++}, at: [<c01e4ddb>] might_fault+0x4b/0xa0
but task is already holding lock:
(&dev->struct_mutex){+.+.+.}, at: [<f869c3ac>]
i915_mutex_lock_interruptible+0x3c/0x60 [i915]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&dev->struct_mutex){+.+.+.}:
[<c01738ec>] lock_acquire+0x7c/0x120
[<c04d3560>] mutex_lock_nested+0x60/0x330
[<f810c7aa>] drm_gem_mmap+0x3a/0x140 [drm]
[<c01ec202>] mmap_region+0x2e2/0x430
[<c01ec59f>] do_mmap_pgoff+0x24f/0x2f0
[<c01ec6bb>] sys_mmap_pgoff+0x7b/0x110
[<c0102f97>] sysenter_do_call+0x12/0x36
-> #0 (&mm->mmap_sem){++++++}:
[<c0173549>] __lock_acquire+0x14a9/0x17d0
[<c01738ec>] lock_acquire+0x7c/0x120
[<c01e4e0c>] might_fault+0x7c/0xa0
[<c032dfad>] _copy_from_user+0x3d/0x130
[<f869cdec>] i915_gem_pwrite_ioctl+0x18c/0xb20 [i915]
[<f810afd2>] drm_ioctl+0x1e2/0x440 [drm]
[<c0216dcd>] do_vfs_ioctl+0x9d/0x6c0
[<c0217457>] sys_ioctl+0x67/0x80
[<c0102f97>] sysenter_do_call+0x12/0x36
other info that might help us debug this:
1 lock held by Xorg/1401:
#0: (&dev->struct_mutex){+.+.+.}, at: [<f869c3ac>]
i915_mutex_lock_interruptible+0x3c/0x60 [i915]
stack backtrace:
Pid: 1401, comm: Xorg Not tainted 2.6.37-rc1-uwe1+ #4
Call Trace:
[<c04d12de>] ? printk+0x1d/0x1f
[<c01714f2>] print_circular_bug+0xc2/0xd0
[<c0173549>] __lock_acquire+0x14a9/0x17d0
[<c04d82c9>] ? sub_preempt_count+0x9/0x50
[<c01738ec>] lock_acquire+0x7c/0x120
[<c01e4ddb>] ? might_fault+0x4b/0xa0
[<c01e4e0c>] might_fault+0x7c/0xa0
[<c01e4ddb>] ? might_fault+0x4b/0xa0
[<c032dfad>] _copy_from_user+0x3d/0x130
[<f869cdec>] i915_gem_pwrite_ioctl+0x18c/0xb20 [i915]
[<c01e4ddb>] ? might_fault+0x4b/0xa0
[<f810afd2>] drm_ioctl+0x1e2/0x440 [drm]
[<f869cc60>] ? i915_gem_pwrite_ioctl+0x0/0xb20 [i915]
[<c02f36d3>] ? smk_access+0xb3/0x1f0
[<c02f3914>] ? smk_curacc+0x74/0xa0
[<f810adf0>] ? drm_ioctl+0x0/0x440 [drm]
[<c0216dcd>] do_vfs_ioctl+0x9d/0x6c0
[<c0217457>] sys_ioctl+0x67/0x80
[<c0102f97>] sysenter_do_call+0x12/0x36
thanks,
Uwe
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] drm/i915: Avoid might_fault during pwrite whilst holding our mutex
2010-11-08 0:53 ` INFO: possible circular locking dependency detected [v2.6.37-rc1-152-g6c88078] Uwe Helm
@ 2010-11-08 1:34 ` Chris Wilson
2010-11-08 2:45 ` Uwe Helm
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2010-11-08 1:34 UTC (permalink / raw)
To: Uwe Helm; +Cc: linux-kernel, Chris Wilson
... and so prevent a potential circular reference:
[ INFO: possible circular locking dependency detected ]
2.6.37-rc1-uwe1+ #4
-------------------------------------------------------
Xorg/1401 is trying to acquire lock:
(&mm->mmap_sem){++++++}, at: [<c01e4ddb>] might_fault+0x4b/0xa0
but task is already holding lock:
(&dev->struct_mutex){+.+.+.}, at: [<f869c3ac>]
i915_mutex_lock_interruptible+0x3c/0x60 [i915]
which lock already depends on the new lock.
When the locking around the pwrite ioctl was simplified, I did not spot
that the phys path never took any locks and so we introduced this
potential circular reference.
Reported-by: Uwe Helm <uwe.helm@googlemail.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++++++---------
1 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 984eb6e..eba9b16 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4878,17 +4878,24 @@ i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object *obj,
struct drm_file *file_priv)
{
struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
- void *obj_addr;
- int ret;
- char __user *user_data;
+ void *vaddr = obj_priv->phys_obj->handle->vaddr + args->offset;
+ char __user *user_data = (char __user *) (uintptr_t) args->data_ptr;
- user_data = (char __user *) (uintptr_t) args->data_ptr;
- obj_addr = obj_priv->phys_obj->handle->vaddr + args->offset;
+ DRM_DEBUG_DRIVER("vaddr %p, %lld\n", vaddr, args->size);
- DRM_DEBUG_DRIVER("obj_addr %p, %lld\n", obj_addr, args->size);
- ret = copy_from_user(obj_addr, user_data, args->size);
- if (ret)
- return -EFAULT;
+ if (__copy_from_user_inatomic_nocache(vaddr, user_data, args->size)) {
+ unsigned long unwritten;
+
+ /* The physical object once assigned is fixed for the lifetime
+ * of the obj, so we can safely drop the lock and continue
+ * to access vaddr.
+ */
+ mutex_unlock(&dev->struct_mutex);
+ unwritten = copy_from_user(vaddr, user_data, args->size);
+ mutex_lock(&dev->struct_mutex);
+ if (unwritten)
+ return -EFAULT;
+ }
drm_agp_chipset_flush(dev);
return 0;
--
1.7.2.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Avoid might_fault during pwrite whilst holding our mutex
2010-11-08 1:34 ` [PATCH] drm/i915: Avoid might_fault during pwrite whilst holding our mutex Chris Wilson
@ 2010-11-08 2:45 ` Uwe Helm
0 siblings, 0 replies; 3+ messages in thread
From: Uwe Helm @ 2010-11-08 2:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: linux-kernel
On Sun, Nov 7, 2010 at 5:34 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> ... and so prevent a potential circular reference:
>
> [ INFO: possible circular locking dependency detected ]
> 2.6.37-rc1-uwe1+ #4
> -------------------------------------------------------
> Xorg/1401 is trying to acquire lock:
> (&mm->mmap_sem){++++++}, at: [<c01e4ddb>] might_fault+0x4b/0xa0
>
> but task is already holding lock:
> (&dev->struct_mutex){+.+.+.}, at: [<f869c3ac>]
> i915_mutex_lock_interruptible+0x3c/0x60 [i915]
>
> which lock already depends on the new lock.
>
> When the locking around the pwrite ioctl was simplified, I did not spot
> that the phys path never took any locks and so we introduced this
> potential circular reference.
>
> Reported-by: Uwe Helm <uwe.helm@googlemail.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++++++---------
> 1 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 984eb6e..eba9b16 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4878,17 +4878,24 @@ i915_gem_phys_pwrite(struct drm_device *dev, struct drm_gem_object *obj,
> struct drm_file *file_priv)
> {
> struct drm_i915_gem_object *obj_priv = to_intel_bo(obj);
> - void *obj_addr;
> - int ret;
> - char __user *user_data;
> + void *vaddr = obj_priv->phys_obj->handle->vaddr + args->offset;
> + char __user *user_data = (char __user *) (uintptr_t) args->data_ptr;
>
> - user_data = (char __user *) (uintptr_t) args->data_ptr;
> - obj_addr = obj_priv->phys_obj->handle->vaddr + args->offset;
> + DRM_DEBUG_DRIVER("vaddr %p, %lld\n", vaddr, args->size);
>
> - DRM_DEBUG_DRIVER("obj_addr %p, %lld\n", obj_addr, args->size);
> - ret = copy_from_user(obj_addr, user_data, args->size);
> - if (ret)
> - return -EFAULT;
> + if (__copy_from_user_inatomic_nocache(vaddr, user_data, args->size)) {
> + unsigned long unwritten;
> +
> + /* The physical object once assigned is fixed for the lifetime
> + * of the obj, so we can safely drop the lock and continue
> + * to access vaddr.
> + */
> + mutex_unlock(&dev->struct_mutex);
> + unwritten = copy_from_user(vaddr, user_data, args->size);
> + mutex_lock(&dev->struct_mutex);
> + if (unwritten)
> + return -EFAULT;
> + }
>
> drm_agp_chipset_flush(dev);
> return 0;
> --
> 1.7.2.3
>
>
works, thank you.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-11-08 2:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <AANLkTikZ2cGGVnVai108VSQrL9yHSa5myKMEDZOgkDCe@mail.gmail.com>
2010-11-08 0:53 ` INFO: possible circular locking dependency detected [v2.6.37-rc1-152-g6c88078] Uwe Helm
2010-11-08 1:34 ` [PATCH] drm/i915: Avoid might_fault during pwrite whilst holding our mutex Chris Wilson
2010-11-08 2:45 ` Uwe Helm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).