Add a flag NVOBJ_FLAG_CREAT_EXCL, which will make sure that only 1 engctx will be created. This removes the need of having multiple
Fixes the following lockdep splat:
============================================= [ INFO: possible recursive locking detected ] 3.8.0-rc6-ninja+ #1 Not tainted --------------------------------------------- modprobe/585 is trying to acquire lock: (&subdev->mutex){+.+.+.}, at: [<ffffffffa016c323>] nouveau_instobj_create_+0x43/0x90 [nouveau]
but task is already holding lock: (&subdev->mutex){+.+.+.}, at: [<ffffffffa017672d>] nv50_disp_data_ctor+0x5d/0xd0 [nouveau]
other info that might help us debug this: Possible unsafe locking scenario:
CPU0 ---- lock(&subdev->mutex); lock(&subdev->mutex);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by modprobe/585: #0: (&__lockdep_no_validate__){......}, at: [<ffffffff813075f3>] __driver_attach+0x53/0xb0 #1: (&__lockdep_no_validate__){......}, at: [<ffffffff81307601>] __driver_attach+0x61/0xb0 #2: (drm_global_mutex){+.+.+.}, at: [<ffffffff812ee59c>] drm_get_pci_dev+0xbc/0x2b0 #3: (&subdev->mutex){+.+.+.}, at: [<ffffffffa017672d>] nv50_disp_data_ctor+0x5d/0xd0 [nouveau]
stack backtrace: Pid: 585, comm: modprobe Not tainted 3.8.0-rc6-expert+ #1 Call Trace: [<ffffffff8108fde2>] validate_chain.isra.33+0xd72/0x10d0 [<ffffffff8105fa08>] ? __kernel_text_address+0x58/0x80 [<ffffffff8100575d>] ? print_context_stack+0x5d/0xd0 [<ffffffff81090bc1>] __lock_acquire+0x3a1/0xb60 [<ffffffff8108d504>] ? __lock_is_held+0x54/0x80 [<ffffffff8109184a>] lock_acquire+0x5a/0x70 [<ffffffffa016c323>] ? nouveau_instobj_create_+0x43/0x90 [nouveau] [<ffffffff81558739>] mutex_lock_nested+0x69/0x340 [<ffffffffa016c323>] ? nouveau_instobj_create_+0x43/0x90 [nouveau] [<ffffffffa0152370>] ? nouveau_object_create_+0x60/0xa0 [nouveau] [<ffffffffa016c323>] nouveau_instobj_create_+0x43/0x90 [nouveau] [<ffffffffa016cf8c>] nv50_instobj_ctor+0x4c/0xf0 [nouveau] [<ffffffffa0152163>] nouveau_object_ctor+0x33/0xc0 [nouveau] [<ffffffffa016cd51>] nv50_instmem_alloc+0x21/0x30 [nouveau] [<ffffffffa0150917>] nouveau_gpuobj_create_+0x247/0x2f0 [nouveau] [<ffffffff8155b35a>] ? _raw_spin_unlock_irqrestore+0x3a/0x70 [<ffffffff810921fd>] ? trace_hardirqs_on_caller+0x10d/0x1a0 [<ffffffffa014f4bc>] nouveau_engctx_create_+0x25c/0x2a0 [nouveau] [<ffffffffa0176791>] nv50_disp_data_ctor+0xc1/0xd0 [nouveau] [<ffffffffa0153722>] ? nouveau_subdev_reset+0x52/0x60 [nouveau] [<ffffffffa0152163>] nouveau_object_ctor+0x33/0xc0 [nouveau] [<ffffffffa0152a42>] nouveau_object_new+0x112/0x240 [nouveau] [<ffffffffa01f4b1d>] nv50_display_create+0x18d/0x860 [nouveau] [<ffffffff8105cb5d>] ? __cancel_work_timer+0x6d/0xc0 [<ffffffffa01db8eb>] nouveau_display_create+0x3cb/0x670 [nouveau] [<ffffffffa01cb1bf>] nouveau_drm_load+0x26f/0x590 [nouveau] [<ffffffff81304c99>] ? device_register+0x19/0x20 [<ffffffff812efe91>] ? drm_sysfs_device_add+0x81/0xb0 [<ffffffff812ee65e>] drm_get_pci_dev+0x17e/0x2b0 [<ffffffff81245e56>] ? __pci_set_master+0x26/0x80 [<ffffffffa01cab2a>] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [<ffffffff8124a386>] local_pci_probe+0x46/0x80 [<ffffffff8124ac11>] pci_device_probe+0x101/0x110 [<ffffffff813073d6>] driver_probe_device+0x76/0x240 [<ffffffff81307643>] __driver_attach+0xa3/0xb0 [<ffffffff813075a0>] ? driver_probe_device+0x240/0x240 [<ffffffff8130564d>] bus_for_each_dev+0x4d/0x90 [<ffffffff81306f39>] driver_attach+0x19/0x20 [<ffffffff81306af0>] bus_add_driver+0x1a0/0x270 [<ffffffffa023d000>] ? 0xffffffffa023cfff [<ffffffff81307cd2>] driver_register+0x72/0x170 [<ffffffffa023d000>] ? 0xffffffffa023cfff [<ffffffff8124ad0f>] __pci_register_driver+0x5f/0x70 [<ffffffff812ee8a5>] drm_pci_init+0x115/0x130 [<ffffffffa023d000>] ? 0xffffffffa023cfff [<ffffffffa023d000>] ? 0xffffffffa023cfff [<ffffffffa023d04d>] nouveau_drm_init+0x4d/0x1000 [nouveau] [<ffffffff810002da>] do_one_initcall+0x11a/0x170 [<ffffffff8109d044>] load_module+0xe84/0x1470 [<ffffffff81098c30>] ? in_lock_functions+0x20/0x20 [<ffffffff8122c22e>] ? trace_hardirqs_on_thunk+0x3a/0x3f [<ffffffff8109d6e7>] sys_init_module+0xb7/0xe0 [<ffffffff8155c156>] system_call_fastpath+0x1a/0x1f
Reported-by: Arend van Spriel arend@broadcom.com Reported-by: Peter Hurley peter@hurleysoftware.com Reported-by: Daniel J Blueman daniel@quora.org Signed-off-by: Maarten Lankhorst maarten.lankhorst@canonical.com --- XXX: Add Cc: stable@kernel.org if it applies to 3.7 ?
diff --git a/drivers/gpu/drm/nouveau/core/core/engctx.c b/drivers/gpu/drm/nouveau/core/core/engctx.c index 84c71fa..4a0ab2b 100644 --- a/drivers/gpu/drm/nouveau/core/core/engctx.c +++ b/drivers/gpu/drm/nouveau/core/core/engctx.c @@ -31,12 +31,15 @@ #include <subdev/vm.h>
static inline int -nouveau_engctx_exists(struct nouveau_object *parent, +nouveau_engctx_exists(struct nouveau_object *parent, u32 flags, struct nouveau_engine *engine, void **pobject) { struct nouveau_engctx *engctx; struct nouveau_object *parctx;
+ if ((flags & NVOBJ_FLAG_CREAT_EXCL) && !list_empty(&engine->contexts)) + return -EBUSY; + list_for_each_entry(engctx, &engine->contexts, head) { parctx = nv_pclass(nv_object(engctx), NV_PARENT_CLASS); if (parctx == parent) { @@ -67,7 +70,7 @@ nouveau_engctx_create_(struct nouveau_object *parent, * and reference it instead of creating a new one */ spin_lock_irqsave(&engine->lock, save); - ret = nouveau_engctx_exists(parent, engine, pobject); + ret = nouveau_engctx_exists(parent, flags, engine, pobject); spin_unlock_irqrestore(&engine->lock, save); if (ret) return ret; @@ -94,7 +97,7 @@ nouveau_engctx_create_(struct nouveau_object *parent, * it's not possible to allocate the object with it held. */ spin_lock_irqsave(&engine->lock, save); - ret = nouveau_engctx_exists(parent, engine, pobject); + ret = nouveau_engctx_exists(parent, flags, engine, pobject); if (ret) { spin_unlock_irqrestore(&engine->lock, save); nouveau_object_ref(NULL, &engctx); diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c index ca1a7d7..1d3dcd0 100644 --- a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c +++ b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c @@ -695,9 +695,8 @@ nv50_disp_data_ctor(struct nouveau_object *parent, struct nouveau_oclass *oclass, void *data, u32 size, struct nouveau_object **pobject) { - struct nv50_disp_priv *priv = (void *)engine; struct nouveau_engctx *ectx; - int ret = -EBUSY; + int ret;
/* no context needed for channel objects... */ if (nv_mclass(parent) != NV_DEVICE_CLASS) { @@ -707,14 +706,10 @@ nv50_disp_data_ctor(struct nouveau_object *parent, }
/* allocate display hardware to client */ - mutex_lock(&nv_subdev(priv)->mutex); - if (list_empty(&nv_engine(priv)->contexts)) { - ret = nouveau_engctx_create(parent, engine, oclass, NULL, - 0x10000, 0x10000, - NVOBJ_FLAG_HEAP, &ectx); - *pobject = nv_object(ectx); - } - mutex_unlock(&nv_subdev(priv)->mutex); + ret = nouveau_engctx_create(parent, engine, oclass, NULL, 0x10000, 0x10000, + NVOBJ_FLAG_HEAP | NVOBJ_FLAG_CREAT_EXCL, &ectx); + + *pobject = nv_object(ectx); return ret; }
diff --git a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h index b3b9ce4..9d47d51 100644 --- a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h +++ b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h @@ -12,6 +12,7 @@ struct nouveau_vm; #define NVOBJ_FLAG_ZERO_ALLOC 0x00000001 #define NVOBJ_FLAG_ZERO_FREE 0x00000002 #define NVOBJ_FLAG_HEAP 0x00000004 +#define NVOBJ_FLAG_CREAT_EXCL 0x00000008
struct nouveau_gpuobj { struct nouveau_object base;